From 2b3bd29588c56a71e4fb1c8e63456747a8ab1777 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Tue, 30 Dec 2025 08:38:06 +0100 Subject: [PATCH] Fix infinite repaint when setting magic variables in prompt Commit 7996637db55 (Make fish immediately show color changes again, 2025-12-01) repaints unnecessarily when a local unexported color variable changes. Also, it repaints when the change comes from fish_prompt, causing an easy infinite loop. Same when changing TERM, COLORTERM and others. This feature is relevant when using a color-theme aware theme, so try to keep it. Repaint only on global/universal changes. Also ignore changes if already repainting fish prompt. This change may be at odds with concurrent execution (parser should not care about whether we are repainting) but that's intentional because of 1. time constraints and 2. I'm not sure what the solution will look like; we could use the event infrastructure. But a lot of existing variable listeners don't use that. Extract a context object we pass whenever we mutate the environment; While at it, use it to pass EnvMode::USER, to reduce EnvMode responsibilities. Fixes #12233 --- CHANGELOG.rst | 2 + src/bin/fish.rs | 10 +- src/builtins/abbr.rs | 4 +- src/builtins/argparse.rs | 48 ++++++---- src/builtins/cd.rs | 7 +- src/builtins/commandline.rs | 2 +- src/builtins/fg.rs | 3 +- src/builtins/read.rs | 30 +++--- src/builtins/set.rs | 10 +- src/builtins/source.rs | 2 +- src/builtins/string/match.rs | 5 +- src/common.rs | 2 +- src/complete.rs | 22 +++-- src/env/environment.rs | 168 ++++++++++++++++++++------------- src/env/environment_impl.rs | 28 ++++-- src/env/var.rs | 41 ++++++-- src/env_dispatch.rs | 89 ++++++++++------- src/exec.rs | 41 ++++---- src/expand.rs | 5 +- src/highlight/highlight.rs | 29 ++---- src/history/history.rs | 14 +-- src/parse_execution.rs | 21 +++-- src/parser.rs | 60 ++++++++++-- src/path.rs | 10 +- src/reader/reader.rs | 29 +++--- src/termsize.rs | 10 +- tests/checks/tmux-repaint.fish | 49 ++++++++++ 27 files changed, 497 insertions(+), 244 deletions(-) create mode 100644 tests/checks/tmux-repaint.fish diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7e7194561..d46fdfe7a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,6 +6,8 @@ This release fixes the following problems identified in 4.3.0: - Pre-built macOS packages failed to start due to a ``Malformed Mach-O file`` error (:issue:`12224`). - ``extra_functionsdir`` (usually ``vendor_functions.d``) and friends were not used (:issue:`12226`). - Sample config file ``~/.config/fish/config.fish/`` and config directories ``~/.config/fish/conf.d/``, ``~/.config/fish/completions/`` and ``~/.config/fish/functions/`` were recreated on every startup instead of only the first time fish runs on a system (:issue:`12230`). +- Spurious echo of ``^[[I`` in some scenarios (:issue:`12232`). +- Infinite prompt redraw loop on some prompts (:issue:`12233`). - The removal of pre-built HTML docs from tarballs revealed that cross compilation is broken because we use ``${CMAKE_BINARY_DIR}/fish_indent`` for building HTML docs. As a workaround, the new CMake build option ``FISH_INDENT_FOR_BUILDING_DOCS`` can be set to the path of a runnable ``fish_indent`` binary. diff --git a/src/bin/fish.rs b/src/bin/fish.rs index c145ef7df..11f504cc9 100644 --- a/src/bin/fish.rs +++ b/src/bin/fish.rs @@ -47,7 +47,7 @@ parse_constants::{ParseErrorList, ParseTreeFlags}, parse_tree::ParsedSource, parse_util::parse_util_detect_errors_in_ast, - parser::{BlockType, CancelBehavior, Parser}, + parser::{BlockType, CancelBehavior, Parser, ParserEnvSetMode}, path::path_get_config, prelude::*, printf, @@ -498,9 +498,9 @@ fn throwing_main() -> i32 { if is_interactive_session() && opts.no_config && !opts.no_exec { // If we have no config, we default to the default key bindings. - parser.vars().set_one( + parser.set_one( L!("fish_key_bindings"), - EnvMode::UNEXPORT, + ParserEnvSetMode::new(EnvMode::UNEXPORT), L!("fish_default_key_bindings").to_owned(), ); if function::exists(L!("fish_default_key_bindings"), parser) { @@ -547,7 +547,7 @@ fn throwing_main() -> i32 { let list = &args[my_optind..]; parser.set_var( L!("argv"), - EnvMode::default(), + ParserEnvSetMode::default(), list.iter().map(|s| s.to_owned()).collect(), ); res = run_command_list(parser, &opts.batch_cmds); @@ -582,7 +582,7 @@ fn throwing_main() -> i32 { let list = &args[my_optind..]; parser.set_var( L!("argv"), - EnvMode::default(), + ParserEnvSetMode::default(), list.iter().map(|s| s.to_owned()).collect(), ); let rel_filename = &args[my_optind - 1]; diff --git a/src/builtins/abbr.rs b/src/builtins/abbr.rs index 3ae9be40a..1a60f2cb2 100644 --- a/src/builtins/abbr.rs +++ b/src/builtins/abbr.rs @@ -2,6 +2,7 @@ use crate::abbrs::{self, Abbreviation, Position}; use crate::common::{EscapeStringStyle, escape, escape_string, valid_func_name}; use crate::env::{EnvMode, EnvStackSetResult}; +use crate::parser::ParserEnvSetMode; use crate::re::{regex_make_anchored, to_boxed_chars}; use pcre2::utf32::{Regex, RegexBuilder}; @@ -460,7 +461,8 @@ fn abbr_erase(opts: &Options, parser: &Parser) -> BuiltinResult { let esc_src = escape(arg); if !esc_src.is_empty() { let var_name = WString::from_str("_fish_abbr_") + esc_src.as_utfstr(); - let ret = parser.vars().remove(&var_name, EnvMode::UNIVERSAL); + let ret = + parser.remove_var(&var_name, ParserEnvSetMode::new(EnvMode::UNIVERSAL)); if ret == EnvStackSetResult::Ok { result = Ok(SUCCESS) diff --git a/src/builtins/argparse.rs b/src/builtins/argparse.rs index 02fb8f129..cfcbbbc43 100644 --- a/src/builtins/argparse.rs +++ b/src/builtins/argparse.rs @@ -2,8 +2,9 @@ use super::prelude::*; -use crate::env::{EnvMode, EnvStack}; +use crate::env::{EnvMode, EnvSetMode, EnvStack}; use crate::exec::exec_subshell; +use crate::parser::ParserEnvSetMode; use crate::wutil::fish_iswalnum; const VAR_NAME_PREFIX: &wstr = L!("_flag_"); @@ -699,24 +700,31 @@ fn validate_arg<'opts>( return Ok(SUCCESS); } - let vars = parser.vars(); - vars.push(true /* new_scope */); + parser.vars().push(true /* new_scope */); - let env_mode = EnvMode::LOCAL | EnvMode::EXPORT; - vars.set_one(L!("_argparse_cmd"), env_mode, opts_name.to_owned()); + let local_exported_mode = ParserEnvSetMode::new(EnvMode::LOCAL | EnvMode::EXPORT); + parser.set_one( + L!("_argparse_cmd"), + local_exported_mode, + opts_name.to_owned(), + ); let flag_name = WString::from(VAR_NAME_PREFIX) + "name"; if is_long_flag { - vars.set_one(&flag_name, env_mode, opt_spec.long_flag.to_owned()); - } else { - vars.set_one( + parser.set_one( &flag_name, - env_mode, + local_exported_mode, + opt_spec.long_flag.to_owned(), + ); + } else { + parser.set_one( + &flag_name, + local_exported_mode, WString::from_chars(vec![opt_spec.short_flag]), ); } - vars.set_one( + parser.set_one( &(WString::from(VAR_NAME_PREFIX) + "value"), - env_mode, + local_exported_mode, woptarg.to_owned(), ); @@ -733,7 +741,7 @@ fn validate_arg<'opts>( streams.err.append(&output); streams.err.append_char('\n'); } - vars.pop(); + parser.vars().pop(parser.is_repainting()); retval.map(|()| SUCCESS) } @@ -1138,7 +1146,7 @@ fn check_min_max_args_constraints( } /// Put the result of parsing the supplied args into the caller environment as local vars. -fn set_argparse_result_vars(vars: &EnvStack, opts: ArgParseCmdOpts) { +fn set_argparse_result_vars(vars: &EnvStack, local_mode: EnvSetMode, opts: ArgParseCmdOpts) { for opt_spec in opts.options.values() { if opt_spec.num_seen == 0 { continue; @@ -1147,7 +1155,7 @@ fn set_argparse_result_vars(vars: &EnvStack, opts: ArgParseCmdOpts) { if opt_spec.short_flag_valid { let mut var_name = WString::from(VAR_NAME_PREFIX); var_name.push(opt_spec.short_flag); - vars.set(&var_name, EnvMode::LOCAL, opt_spec.vals.clone()); + vars.set(&var_name, local_mode, opt_spec.vals.clone()); } if !opt_spec.long_flag.is_empty() { @@ -1158,14 +1166,14 @@ fn set_argparse_result_vars(vars: &EnvStack, opts: ArgParseCmdOpts) { .chars() .map(|c| if fish_iswalnum(c) { c } else { '_' }); let var_name_long: WString = VAR_NAME_PREFIX.chars().chain(long_flag).collect(); - vars.set(&var_name_long, EnvMode::LOCAL, opt_spec.vals.clone()); + vars.set(&var_name_long, local_mode, opt_spec.vals.clone()); } } let args = opts.args.into_iter().map(|s| s.into_owned()).collect(); - vars.set(L!("argv"), EnvMode::LOCAL, args); + vars.set(L!("argv"), local_mode, args); let args_opts = opts.args_opts.into_iter().map(|s| s.into_owned()).collect(); - vars.set(L!("argv_opts"), EnvMode::LOCAL, args_opts); + vars.set(L!("argv_opts"), local_mode, args_opts); } /// The argparse builtin. This is explicitly not compatible with the BSD or GNU version of this @@ -1213,7 +1221,11 @@ pub fn argparse(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> check_min_max_args_constraints(&opts, streams)?; - set_argparse_result_vars(parser.vars(), opts); + set_argparse_result_vars( + parser.vars(), + parser.convert_env_set_mode(ParserEnvSetMode::new(EnvMode::LOCAL)), + opts, + ); Ok(SUCCESS) } diff --git a/src/builtins/cd.rs b/src/builtins/cd.rs index f057d32fc..9cf92a31f 100644 --- a/src/builtins/cd.rs +++ b/src/builtins/cd.rs @@ -4,6 +4,7 @@ use crate::{ env::{EnvMode, Environment}, fds::{BEST_O_SEARCH, wopen_dir}, + parser::ParserEnvSetMode, path::path_apply_cdpath, wutil::{normalize_path, wperror, wreadlink}, }; @@ -127,7 +128,11 @@ pub fn cd(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> Built // Stash the fd for the cwd in the parser. parser.libdata_mut().cwd_fd = Some(dir_fd); - parser.set_var_and_fire(L!("PWD"), EnvMode::EXPORT | EnvMode::GLOBAL, vec![norm_dir]); + parser.set_var_and_fire( + L!("PWD"), + ParserEnvSetMode::new(EnvMode::EXPORT | EnvMode::GLOBAL), + vec![norm_dir], + ); return Ok(SUCCESS); } diff --git a/src/builtins/commandline.rs b/src/builtins/commandline.rs index 5c434ea72..23dbb34b4 100644 --- a/src/builtins/commandline.rs +++ b/src/builtins/commandline.rs @@ -399,7 +399,7 @@ pub fn commandline(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) // Don't enqueue a repaint if we're currently in the middle of one, // because that's an infinite loop. if matches!(cmd, RL::RepaintMode | RL::ForceRepaint | RL::Repaint) - && parser.libdata().is_repaint + && parser.is_repainting() { continue; } diff --git a/src/builtins/fg.rs b/src/builtins/fg.rs index 644b38de9..e33e86131 100644 --- a/src/builtins/fg.rs +++ b/src/builtins/fg.rs @@ -1,6 +1,7 @@ //! Implementation of the fg builtin. use crate::fds::make_fd_blocking; +use crate::parser::ParserEnvSetMode; use crate::reader::{reader_save_screen_state, reader_write_title}; use crate::tokenizer::tok_command; use crate::wutil::perror; @@ -123,7 +124,7 @@ pub fn fg(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Built // Provide value for `status current-command` parser.libdata_mut().status_vars.command = ft.clone(); // Also provide a value for the deprecated fish 2.0 $_ variable - parser.set_var_and_fire(L!("_"), EnvMode::EXPORT, vec![ft]); + parser.set_var_and_fire(L!("_"), ParserEnvSetMode::new(EnvMode::EXPORT), vec![ft]); // Provide value for `status current-commandline` parser.libdata_mut().status_vars.commandline = job.command().to_owned(); } diff --git a/src/builtins/read.rs b/src/builtins/read.rs index 6d0eede72..38099d777 100644 --- a/src/builtins/read.rs +++ b/src/builtins/read.rs @@ -16,6 +16,7 @@ use crate::input_common::decode_one_codepoint_utf8; use crate::nix::isatty; use crate::parse_execution::varname_error; +use crate::parser::ParserEnvSetMode; use crate::reader::ReaderConfig; use crate::reader::commandline_set_buffer; use crate::reader::{reader_pop, reader_push, reader_readline, set_shell_modes_temporarily}; @@ -42,7 +43,7 @@ pub(crate) enum TokenOutputMode { #[derive(Default)] struct Options { print_help: bool, - place: EnvMode, + place: ParserEnvSetMode, prompt: Option, prompt_str: Option, right_prompt: WString, @@ -63,7 +64,7 @@ struct Options { impl Options { fn new() -> Self { Options { - place: EnvMode::USER, + place: ParserEnvSetMode::user(EnvMode::empty()), ..Default::default() } } @@ -129,10 +130,10 @@ fn parse_cmd_opts( return Err(STATUS_INVALID_ARGS); } 'f' => { - opts.place |= EnvMode::FUNCTION; + opts.place.mode |= EnvMode::FUNCTION; } 'g' => { - opts.place |= EnvMode::GLOBAL; + opts.place.mode |= EnvMode::GLOBAL; } 'h' => { opts.print_help = true; @@ -141,7 +142,7 @@ fn parse_cmd_opts( opts.one_line = true; } 'l' => { - opts.place |= EnvMode::LOCAL; + opts.place.mode |= EnvMode::LOCAL; } 'n' => { opts.nchars = match fish_wcstoi(w.woptarg.unwrap()) { @@ -205,13 +206,13 @@ fn parse_cmd_opts( opts.token_mode = Some(new_mode); } 'U' => { - opts.place |= EnvMode::UNIVERSAL; + opts.place.mode |= EnvMode::UNIVERSAL; } 'u' => { - opts.place |= EnvMode::UNEXPORT; + opts.place.mode |= EnvMode::UNEXPORT; } 'x' => { - opts.place |= EnvMode::EXPORT; + opts.place.mode |= EnvMode::EXPORT; } 'z' => { opts.split_null = true; @@ -483,7 +484,7 @@ fn validate_read_args( opts.prompt = Some(DEFAULT_READ_PROMPT.to_owned()); } - if opts.place.contains(EnvMode::UNEXPORT) && opts.place.contains(EnvMode::EXPORT) { + if opts.place.mode.contains(EnvMode::UNEXPORT) && opts.place.mode.contains(EnvMode::EXPORT) { streams .err .append(&wgettext_fmt!(BUILTIN_ERR_EXPUNEXP, cmd)); @@ -491,7 +492,14 @@ fn validate_read_args( return Err(STATUS_INVALID_ARGS); } - if opts.place.intersection(EnvMode::ANY_SCOPE).iter().count() > 1 { + if opts + .place + .mode + .intersection(EnvMode::ANY_SCOPE) + .iter() + .count() + > 1 + { streams.err.append(&wgettext_fmt!(BUILTIN_ERR_GLOCAL, cmd)); builtin_print_error_trailer(parser, streams.err, cmd); return Err(STATUS_INVALID_ARGS); @@ -614,7 +622,7 @@ pub fn read(parser: &Parser, streams: &mut IoStreams, argv: &mut [&wstr]) -> Bui let vars_left = |var_ptr: usize| argc - var_ptr; let clear_remaining_vars = |var_ptr: &mut usize| { while vars_left(*var_ptr) != 0 { - parser.vars().set_empty(argv[*var_ptr], opts.place); + parser.set_empty(argv[*var_ptr], opts.place); *var_ptr += 1; } }; diff --git a/src/builtins/set.rs b/src/builtins/set.rs index c88152fb8..50fa07206 100644 --- a/src/builtins/set.rs +++ b/src/builtins/set.rs @@ -16,6 +16,7 @@ use crate::history::History; use crate::history::history_session_id; use crate::parse_execution::varname_error; +use crate::parser::ParserEnvSetMode; use crate::{ env::{EnvMode, EnvVar, Environment}, wutil::wcstoi::wcstoi_partial, @@ -78,7 +79,7 @@ fn default() -> Self { impl Options { fn env_mode(&self) -> EnvMode { - let mut scope = EnvMode::USER; + let mut scope = EnvMode::empty(); for (is_mode, mode) in [ (self.local, EnvMode::LOCAL), (self.function, EnvMode::FUNCTION), @@ -372,10 +373,11 @@ fn env_set_reporting_errors( streams: &mut IoStreams, parser: &Parser, ) -> EnvStackSetResult { + let mode = ParserEnvSetMode::user(mode); let retval = if opts.no_event { - parser.set_var(key, mode | EnvMode::USER, list) + parser.set_var(key, mode, list) } else { - parser.set_var_and_fire(key, mode | EnvMode::USER, list) + parser.set_var_and_fire(key, mode, list) }; // If this returned OK, the parser already fired the event. handle_env_return(retval, cmd, key, streams); @@ -791,7 +793,7 @@ fn erase( let retval; if split.indexes.is_empty() { // unset the var - retval = parser.vars().remove(split.varname, mode); + retval = parser.remove_var(split.varname, ParserEnvSetMode::new(mode)); // When a non-existent-variable is unset, return NotFound as $status // but do not emit any errors at the console as a compromise between user // friendliness and correctness. diff --git a/src/builtins/source.rs b/src/builtins/source.rs index b15e60596..24be29fa7 100644 --- a/src/builtins/source.rs +++ b/src/builtins/source.rs @@ -87,7 +87,7 @@ pub fn source(parser: &Parser, streams: &mut IoStreams, args: &mut [&wstr]) -> B // points to the end of argv. Otherwise we want to skip the file name to get to the args if any. let remaining_args = &args[optind + if argc == optind { 0 } else { 1 }..]; let argv_list = remaining_args.iter().map(|&arg| arg.to_owned()).collect(); - parser.vars().set_argv(argv_list); + parser.vars().set_argv(argv_list, parser.is_repainting()); let retval = reader_read(parser, fd, streams.io_chain); diff --git a/src/builtins/string/match.rs b/src/builtins/string/match.rs index 49c6af8e8..29650640c 100644 --- a/src/builtins/string/match.rs +++ b/src/builtins/string/match.rs @@ -3,9 +3,10 @@ use std::num::NonZeroUsize; use super::*; -use crate::env::{EnvMode, EnvVar, EnvVarFlags}; +use crate::env::{EnvVar, EnvVarFlags}; use crate::flog::flog; use crate::parse_util::parse_util_unescape_wildcards; +use crate::parser::ParserEnvSetMode; use crate::wildcard::{ANY_STRING, wildcard_match}; #[derive(Default)] @@ -146,7 +147,7 @@ fn handle( }) = matcher { for (name, vals) in first_match_captures.into_iter() { - parser.set_var(&WString::from(name), EnvMode::default(), vals); + parser.set_var(&WString::from(name), ParserEnvSetMode::default(), vals); } } diff --git a/src/common.rs b/src/common.rs index 2fee4c4d4..6296138b9 100644 --- a/src/common.rs +++ b/src/common.rs @@ -1951,7 +1951,7 @@ macro_rules! env_stack_set_from_env { if let Some(var) = std::env::var_os($var_name) { $vars.set_one( L!($var_name), - $crate::env::EnvMode::GLOBAL, + $crate::env::EnvSetMode::new_at_early_startup($crate::env::EnvMode::GLOBAL), $crate::common::bytes2wcstring(var.as_bytes()), ); } diff --git a/src/complete.rs b/src/complete.rs index f4b5437d5..526f1c806 100644 --- a/src/complete.rs +++ b/src/complete.rs @@ -32,7 +32,7 @@ parse_util::{ parse_util_cmdsubst_extent, parse_util_process_extent, parse_util_unescape_wildcards, }, - parser::{Block, Parser}, + parser::{Block, Parser, ParserEnvSetMode}, parser_keywords::parser_keywords_is_subcommand, path::{path_get_path, path_try_get_path}, prelude::*, @@ -1914,7 +1914,11 @@ fn apply_var_assignments>( } else { Vec::new() }; - parser.set_var(variable_name, EnvMode::LOCAL | EnvMode::EXPORT, vals); + parser.set_var( + variable_name, + ParserEnvSetMode::new(EnvMode::LOCAL | EnvMode::EXPORT), + vals, + ); if self.ctx.check_cancel() { break; } @@ -2628,11 +2632,12 @@ mod tests { sort_and_prioritize, }; use crate::abbrs::{self, Abbreviation, with_abbrs_mut}; - use crate::env::{EnvMode, Environment}; + use crate::env::{EnvMode, EnvSetMode, Environment}; use crate::io::IoChain; use crate::operation_context::{ EXPANSION_LIMIT_BACKGROUND, EXPANSION_LIMIT_DEFAULT, OperationContext, no_cancel, }; + use crate::parser::ParserEnvSetMode; use crate::prelude::*; use crate::reader::completion_apply_to_command_line; use crate::tests::prelude::*; @@ -3219,9 +3224,9 @@ macro_rules! perform_one_completion_cd_test { // This is to ensure tilde expansion is handled. See the `cd ~/test_autosuggest_suggest_specia` // test below. // Fake out the home directory - parser.vars().set_one( + parser.set_one( L!("HOME"), - EnvMode::LOCAL | EnvMode::EXPORT, + ParserEnvSetMode::new(EnvMode::LOCAL | EnvMode::EXPORT), L!("test/test-home").to_owned(), ); std::fs::create_dir_all("test/test-home/test_autosuggest_suggest_special/").unwrap(); @@ -3331,9 +3336,10 @@ macro_rules! perform_one_completion_cd_test { perform_one_completion_cd_test!("cd ~absolutelynosuchus", "er/"); perform_one_completion_cd_test!("cd ~absolutelynosuchuser/", "path1/"); - parser - .vars() - .remove(L!("HOME"), EnvMode::LOCAL | EnvMode::EXPORT); + parser.vars().remove( + L!("HOME"), + EnvSetMode::new(EnvMode::LOCAL | EnvMode::EXPORT, false), + ); parser.popd(); } diff --git a/src/env/environment.rs b/src/env/environment.rs index b47f45c7c..e8864c232 100644 --- a/src/env/environment.rs +++ b/src/env/environment.rs @@ -7,8 +7,8 @@ use crate::builtins::shared::{BuiltinResult, SUCCESS}; use crate::common::{UnescapeStringStyle, bytes2wcstring, unescape_string, wcs2zstring}; use crate::env::config_paths::ConfigPaths; -use crate::env::{EnvMode, EnvVar, Statuses}; -use crate::env_dispatch::{env_dispatch_init, env_dispatch_var_change}; +use crate::env::{EnvMode, EnvSetMode, EnvVar, Statuses}; +use crate::env_dispatch::{VarChangeMilieu, env_dispatch_init, env_dispatch_var_change}; use crate::event::Event; use crate::flog::flog; use crate::global_safety::RelaxedAtomicBool; @@ -212,7 +212,7 @@ pub fn set_last_statuses(&self, statuses: Statuses) { } /// Sets the variable with the specified name to the given values. - pub fn set(&self, key: &wstr, mode: EnvMode, mut vals: Vec) -> EnvStackSetResult { + pub fn set(&self, key: &wstr, mode: EnvSetMode, mut vals: Vec) -> EnvStackSetResult { // Historical behavior. if vals.len() == 1 && (key == "PWD" || key == "HOME") { path_make_canonical(vals.first_mut().unwrap()); @@ -238,7 +238,14 @@ pub fn set(&self, key: &wstr, mode: EnvMode, mut vals: Vec) -> EnvStack // Dispatch changes if we modified the global state or have 'dispatches_var_changes' set. // Important to not hold the lock here. if ret.global_modified || self.dispatches_var_changes { - env_dispatch_var_change(key, self); + env_dispatch_var_change( + VarChangeMilieu { + is_repainting: mode.is_repainting, + global_or_universal: ret.global_modified || ret.uvar_modified, + }, + key, + self, + ); } } // Mark if we modified a uvar. @@ -249,12 +256,12 @@ pub fn set(&self, key: &wstr, mode: EnvMode, mut vals: Vec) -> EnvStack } /// Sets the variable with the specified name to a single value. - pub fn set_one(&self, key: &wstr, mode: EnvMode, val: WString) -> EnvStackSetResult { + pub fn set_one(&self, key: &wstr, mode: EnvSetMode, val: WString) -> EnvStackSetResult { self.set(key, mode, vec![val]) } /// Sets the variable with the specified name to no values. - pub fn set_empty(&self, key: &wstr, mode: EnvMode) -> EnvStackSetResult { + pub fn set_empty(&self, key: &wstr, mode: EnvSetMode) -> EnvStackSetResult { self.set(key, mode, Vec::new()) } @@ -269,24 +276,32 @@ pub fn set_pwd_from_getcwd(&self) { ) ); } - self.set_one(L!("PWD"), EnvMode::EXPORT | EnvMode::GLOBAL, cwd); + let global_exported_mode = + EnvSetMode::new_at_early_startup(EnvMode::GLOBAL | EnvMode::EXPORT); + self.set_one(L!("PWD"), global_exported_mode, cwd); } /// Remove environment variable. /// /// \param key The name of the variable to remove - /// \param mode should be ENV_USER if this is a remove request from the user, 0 otherwise. If - /// this is a user request, read-only variables can not be removed. The mode may also specify - /// the scope of the variable that should be erased. + /// \param mode If this is a user request, read-only variables can not be removed. The mode + /// may also specify the scope of the variable that should be erased. /// /// Return the set result. - pub fn remove(&self, key: &wstr, mode: EnvMode) -> EnvStackSetResult { + pub fn remove(&self, key: &wstr, mode: EnvSetMode) -> EnvStackSetResult { let ret = self.lock().remove(key, mode); #[allow(clippy::collapsible_if)] if ret.status == EnvStackSetResult::Ok { if ret.global_modified || self.dispatches_var_changes { // Important to not hold the lock here. - env_dispatch_var_change(key, self); + env_dispatch_var_change( + VarChangeMilieu { + is_repainting: mode.is_repainting, + global_or_universal: ret.global_modified || ret.uvar_modified, + }, + key, + self, + ); } } if ret.uvar_modified { @@ -307,14 +322,21 @@ pub fn push(&self, new_scope: bool) { } /// Pop the variable stack. Used for implementing local variables for functions and for-loops. - pub fn pop(&self) { + pub fn pop(&self, is_repainting: bool) { assert!(self.can_push_pop, "push/pop not allowed on global stack"); let popped = self.lock().pop(); if self.dispatches_var_changes { // TODO: we would like to coalesce locale changes, so that we only re-initialize // once. for key in popped { - env_dispatch_var_change(&key, self); + env_dispatch_var_change( + VarChangeMilieu { + is_repainting, + global_or_universal: false, + }, + &key, + self, + ); } } } @@ -335,7 +357,7 @@ pub fn snapshot(&self) -> EnvDyn { /// If `always` is set, perform synchronization even if there's no pending changes from this /// instance (that is, look for changes from other fish instances). /// Return a list of events for changed variables. - pub fn universal_sync(&self, always: bool) -> Vec { + pub fn universal_sync(&self, always: bool, is_repainting: bool) -> Vec { if UVAR_SCOPE_IS_GLOBAL.load() { return Vec::new(); } @@ -353,7 +375,14 @@ pub fn universal_sync(&self, always: bool) -> Vec { if let Some(callbacks) = callbacks { for callback in callbacks { let name = callback.key; - env_dispatch_var_change(&name, self); + env_dispatch_var_change( + VarChangeMilieu { + is_repainting, + global_or_universal: true, + }, + &name, + self, + ); let evt = if callback.val.is_none() { Event::variable_erase(name) } else { @@ -378,8 +407,12 @@ pub fn globals() -> &'static EnvStack { }) } - pub fn set_argv(&self, argv: Vec) { - self.set(L!("argv"), EnvMode::LOCAL, argv); + pub fn set_argv(&self, argv: Vec, is_repainting: bool) { + self.set( + L!("argv"), + EnvSetMode::new(EnvMode::LOCAL, is_repainting), + argv, + ); } } @@ -478,7 +511,7 @@ pub fn get_home() -> Option { } /// Set up the USER and HOME variable. -fn setup_user(vars: &EnvStack) { +fn setup_user(global_exported_mode: EnvSetMode, vars: &EnvStack) { let uid: uid_t = geteuid(); let user_var = vars.get_unless_empty(L!("USER")); @@ -508,11 +541,11 @@ fn setup_user(vars: &EnvStack) { let s = unsafe { CStr::from_ptr(userinfo.pw_dir) }; vars.set_one( L!("HOME"), - EnvMode::GLOBAL | EnvMode::EXPORT, + global_exported_mode, bytes2wcstring(s.to_bytes()), ); } else { - vars.set_empty(L!("HOME"), EnvMode::GLOBAL | EnvMode::EXPORT); + vars.set_empty(L!("HOME"), global_exported_mode); } } return; @@ -535,7 +568,7 @@ fn setup_user(vars: &EnvStack) { let userinfo = unsafe { userinfo.assume_init() }; let s = unsafe { CStr::from_ptr(userinfo.pw_name) }; let uname = bytes2wcstring(s.to_bytes()); - vars.set_one(L!("USER"), EnvMode::GLOBAL | EnvMode::EXPORT, uname); + vars.set_one(L!("USER"), global_exported_mode, uname); // Only change $HOME if it's empty, so we allow e.g. `HOME=(mktemp -d)`. // This is okay with common `su` and `sudo` because they set $HOME. if vars.get_unless_empty(L!("HOME")).is_none() { @@ -543,18 +576,18 @@ fn setup_user(vars: &EnvStack) { let s = unsafe { CStr::from_ptr(userinfo.pw_dir) }; vars.set_one( L!("HOME"), - EnvMode::GLOBAL | EnvMode::EXPORT, + global_exported_mode, bytes2wcstring(s.to_bytes()), ); } else { // We cannot get $HOME. This triggers warnings for history and config.fish already, // so it isn't necessary to warn here as well. - vars.set_empty(L!("HOME"), EnvMode::GLOBAL | EnvMode::EXPORT); + vars.set_empty(L!("HOME"), global_exported_mode); } } } else if vars.get_unless_empty(L!("HOME")).is_none() { // If $USER is empty as well (which we tried to set above), we can't get $HOME. - vars.set_empty(L!("HOME"), EnvMode::GLOBAL | EnvMode::EXPORT); + vars.set_empty(L!("HOME"), global_exported_mode); } } @@ -581,15 +614,11 @@ fn setup_user(vars: &EnvStack) { }); /// Make sure the PATH variable contains something. -fn setup_path() { +fn setup_path(global_exported_mode: EnvSetMode) { let vars = EnvStack::globals(); let path = vars.get_unless_empty(L!("PATH")); if path.is_none() { - vars.set( - L!("PATH"), - EnvMode::GLOBAL | EnvMode::EXPORT, - FALLBACK_PATH.to_vec(), - ); + vars.set(L!("PATH"), global_exported_mode, FALLBACK_PATH.to_vec()); } } @@ -600,6 +629,9 @@ fn setup_path() { pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool) { let vars = EnvStack::globals(); + let global_mode = EnvSetMode::new_at_early_startup(EnvMode::GLOBAL); + let global_exported_mode = EnvSetMode::new_at_early_startup(EnvMode::GLOBAL | EnvMode::EXPORT); + let env_iter: Vec<_> = std::env::vars_os() .map(|(k, v)| (bytes2wcstring(k.as_bytes()), bytes2wcstring(v.as_bytes()))) .collect(); @@ -619,7 +651,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool // a value we previously (due to user error) exported will cause impossibly // difficult to debug PATH problems. if key != "fish_user_paths" { - vars.set(&key, EnvMode::EXPORT | EnvMode::GLOBAL, vec![val.clone()]); + vars.set(&key, global_exported_mode, vec![val.clone()]); } } inherited_vars.insert(key, val); @@ -631,14 +663,14 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool // Set $USER, $HOME and $EUID // This involves going to passwd and stuff. - vars.set_one(L!("EUID"), EnvMode::GLOBAL, geteuid().to_wstring()); - setup_user(vars); + vars.set_one(L!("EUID"), global_mode, geteuid().to_wstring()); + setup_user(global_exported_mode, vars); if let Some(paths) = paths { let set_path = |key: &wstr, maybe_path: Option<&PathBuf>| { vars.set( key, - EnvMode::GLOBAL, + global_mode, maybe_path .map(|path| vec![bytes2wcstring(path.as_os_str().as_bytes())]) .unwrap_or_default(), @@ -656,45 +688,49 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool let user_config_dir = path_get_config(); vars.set_one( FISH_CONFIG_DIR, - EnvMode::GLOBAL, + global_mode, user_config_dir.unwrap_or_default(), ); let user_data_dir = path_get_data(); vars.set_one( FISH_USER_DATA_DIR, - EnvMode::GLOBAL, + global_mode, user_data_dir.unwrap_or_default(), ); let user_cache_dir = path_get_cache(); vars.set_one( FISH_CACHE_DIR, - EnvMode::GLOBAL, + global_mode, user_cache_dir.unwrap_or_default(), ); // Set up a default PATH - setup_path(); + setup_path(global_exported_mode); // Set up $IFS - this used to be in share/config.fish, but really breaks if it isn't done. - vars.set_one(L!("IFS"), EnvMode::GLOBAL, "\n \t".into()); + vars.set_one(L!("IFS"), global_mode, "\n \t".into()); // Ensure this var is present even before an interactive command is run so that if it is used // in a function like `fish_prompt` or `fish_right_prompt` it is defined at the time the first // prompt is written. - vars.set_one(L!("CMD_DURATION"), EnvMode::UNEXPORT, "0".into()); + vars.set_one( + L!("CMD_DURATION"), + EnvSetMode::new_at_early_startup(EnvMode::UNEXPORT), + "0".into(), + ); // Set up the version variable. let version = bytes2wcstring(crate::BUILD_VERSION.as_bytes()); - vars.set_one(L!("version"), EnvMode::GLOBAL, version.clone()); - vars.set_one(L!("FISH_VERSION"), EnvMode::GLOBAL, version); + vars.set_one(L!("version"), global_mode, version.clone()); + vars.set_one(L!("FISH_VERSION"), global_mode, version); // Set the $fish_pid variable. - vars.set_one(L!("fish_pid"), EnvMode::GLOBAL, getpid().to_wstring()); + vars.set_one(L!("fish_pid"), global_mode, getpid().to_wstring()); // Set the $hostname variable let hostname: WString = get_hostname_identifier().unwrap_or("fish".into()); - vars.set_one(L!("hostname"), EnvMode::GLOBAL, hostname); + vars.set_one(L!("hostname"), global_mode, hostname); // Set up SHLVL variable. Not we can't use vars.get() because SHLVL is read-only, and therefore // was not inherited from the environment. @@ -709,13 +745,13 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool } else { L!("1").to_owned() }; - vars.set_one(L!("SHLVL"), EnvMode::GLOBAL | EnvMode::EXPORT, nshlvl_str); + vars.set_one(L!("SHLVL"), global_exported_mode, nshlvl_str); } else { // If we're not interactive, simply pass the value along. if let Some(shlvl_var) = std::env::var_os("SHLVL") { vars.set_one( L!("SHLVL"), - EnvMode::GLOBAL | EnvMode::EXPORT, + global_exported_mode, bytes2wcstring(shlvl_var.as_os_str().as_bytes()), ); } @@ -736,7 +772,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool && incoming_pwd.char_at(0) == '/' && paths_are_same_file(&incoming_pwd, L!(".")) { - vars.set_one(L!("PWD"), EnvMode::EXPORT | EnvMode::GLOBAL, incoming_pwd); + vars.set_one(L!("PWD"), global_exported_mode, incoming_pwd); } else { vars.set_pwd_from_getcwd(); } @@ -744,18 +780,14 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool // Initialize termsize variables. let termsize = termsize::SHARED_CONTAINER.initialize(vars as &dyn Environment); if vars.get_unless_empty(L!("COLUMNS")).is_none() { - vars.set_one( - L!("COLUMNS"), - EnvMode::GLOBAL, - termsize.width().to_wstring(), - ); + vars.set_one(L!("COLUMNS"), global_mode, termsize.width().to_wstring()); } if vars.get_unless_empty(L!("LINES")).is_none() { - vars.set_one(L!("LINES"), EnvMode::GLOBAL, termsize.height().to_wstring()); + vars.set_one(L!("LINES"), global_mode, termsize.height().to_wstring()); } // Set fish_bind_mode to "default". - vars.set_one(FISH_BIND_MODE_VAR, EnvMode::GLOBAL, "default".into()); + vars.set_one(FISH_BIND_MODE_VAR, global_mode, "default".into()); // Allow changes to variables to produce events. env_dispatch_init(vars); @@ -776,7 +808,14 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool // Set up universal variables using the default path. let callbacks = uvars().initialize().unwrap_or_default(); for callback in callbacks { - env_dispatch_var_change(&callback.key, vars); + env_dispatch_var_change( + VarChangeMilieu { + is_repainting: false, + global_or_universal: true, + }, + &callback.key, + vars, + ); } // Do not import variables that have the same name and value as @@ -798,7 +837,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool to_skip }; for name in &globals_to_skip { - EnvStack::globals().remove(name, EnvMode::GLOBAL | EnvMode::EXPORT); + EnvStack::globals().remove(name, global_exported_mode); } // Import any abbreviations from uvars. @@ -830,6 +869,7 @@ pub fn env_init(paths: Option<&ConfigPaths>, do_uvars: bool, default_paths: bool #[cfg(test)] mod tests { use super::{EnvMode, EnvStack, Environment}; + use crate::env::EnvSetMode; use crate::prelude::*; use crate::tests::prelude::*; @@ -845,19 +885,19 @@ fn test_env_snapshot() { let before_pwd = vars.get(L!("PWD")).unwrap().as_string(); vars.set_one( L!("test_env_snapshot_var"), - EnvMode::default(), + EnvSetMode::default(), L!("before").to_owned(), ); let snapshot = vars.snapshot(); - vars.set_one(L!("PWD"), EnvMode::default(), L!("/newdir").to_owned()); + vars.set_one(L!("PWD"), EnvSetMode::default(), L!("/newdir").to_owned()); vars.set_one( L!("test_env_snapshot_var"), - EnvMode::default(), + EnvSetMode::default(), L!("after").to_owned(), ); vars.set_one( L!("test_env_snapshot_var_2"), - EnvMode::default(), + EnvSetMode::default(), L!("after").to_owned(), ); @@ -886,7 +926,7 @@ fn test_env_snapshot() { // snapshots see global var changes except for perproc like PWD vars.set_one( L!("test_env_snapshot_var_3"), - EnvMode::GLOBAL, + EnvSetMode::new(EnvMode::GLOBAL, false), L!("reallyglobal").to_owned(), ); assert_eq!( @@ -901,7 +941,7 @@ fn test_env_snapshot() { L!("reallyglobal") ); - vars.pop(); + vars.pop(false); parser.popd(); } @@ -915,6 +955,6 @@ fn test_no_global_push() { #[test] #[should_panic] fn test_no_global_pop() { - EnvStack::globals().pop(); + EnvStack::globals().pop(false); } } diff --git a/src/env/environment_impl.rs b/src/env/environment_impl.rs index 2f68e59ef..336f129b5 100644 --- a/src/env/environment_impl.rs +++ b/src/env/environment_impl.rs @@ -1,6 +1,6 @@ use crate::common::wcs2zstring; use crate::env::{ - ELECTRIC_VARIABLES, ElectricVar, EnvMode, EnvStackSetResult, EnvVar, EnvVarFlags, + ELECTRIC_VARIABLES, ElectricVar, EnvMode, EnvSetMode, EnvStackSetResult, EnvVar, EnvVarFlags, PATH_ARRAY_SEP, Statuses, VarTable, is_read_only, }; use crate::env_universal_common::EnvUniversal; @@ -116,9 +116,19 @@ struct Query { pub user: bool, } +impl From for Query { + fn from(mode: EnvMode) -> Self { + Self::new(mode, false) + } +} +impl From for Query { + fn from(mode: EnvSetMode) -> Self { + Self::new(mode.mode, mode.user) + } +} impl Query { /// Creates a `Query` from env mode flags. - fn new(mode: EnvMode) -> Self { + fn new(mode: EnvMode, user: bool) -> Self { let has_scope = mode.intersects(EnvMode::ANY_SCOPE); let has_export_unexport = mode.intersects(EnvMode::EXPORT | EnvMode::UNEXPORT); Query { @@ -137,7 +147,7 @@ fn new(mode: EnvMode) -> Self { pathvar: mode.contains(EnvMode::PATHVAR), unpathvar: mode.contains(EnvMode::UNPATHVAR), - user: mode.contains(EnvMode::USER), + user, } } @@ -458,7 +468,7 @@ fn try_get_universal(&self, key: &wstr) -> Option { } pub fn getf(&self, key: &wstr, mode: EnvMode) -> Option { - let query = Query::new(mode); + let query = Query::from(mode); let mut result: Option = None; // Computed variables are effectively global and can't be shadowed. if query.global { @@ -488,7 +498,7 @@ pub fn getf(&self, key: &wstr, mode: EnvMode) -> Option { } pub fn get_names(&self, flags: EnvMode) -> Vec { - let query = Query::new(flags); + let query = Query::from(flags); let mut names: HashSet = HashSet::new(); // Helper to add the names of variables from `envs` to names, respecting show_exported and @@ -720,8 +730,8 @@ pub fn new() -> EnvMutex { } /// Set a variable under the name `key`, using the given `mode`, setting its value to `val`. - pub fn set(&mut self, key: &wstr, mode: EnvMode, mut val: Vec) -> ModResult { - let query = Query::new(mode); + pub fn set(&mut self, key: &wstr, mode: EnvSetMode, mut val: Vec) -> ModResult { + let query = Query::from(mode); // Handle electric and read-only variables. if let Some(ret) = self.try_set_electric(key, &query, &mut val) { return ModResult::new(ret); @@ -802,8 +812,8 @@ pub fn set(&mut self, key: &wstr, mode: EnvMode, mut val: Vec) -> ModRe } /// Remove a variable under the name `key`. - pub fn remove(&mut self, key: &wstr, mode: EnvMode) -> ModResult { - let query = Query::new(mode); + pub fn remove(&mut self, key: &wstr, mode: EnvSetMode) -> ModResult { + let query = Query::from(mode); // Users can't remove read-only keys. if query.user && is_read_only(key) { return ModResult::new(EnvStackSetResult::Scope); diff --git a/src/env/var.rs b/src/env/var.rs index e100a407a..23bb7885f 100644 --- a/src/env/var.rs +++ b/src/env/var.rs @@ -31,11 +31,6 @@ pub struct EnvMode: u16 { const PATHVAR = 1 << 6; /// Flag to unmark a variable as a path variable. const UNPATHVAR = 1 << 7; - /// Flag for variable update request from the user. All variable changes that are made directly - /// by the user, such as those from the `read` and `set` builtin must have this flag set. It - /// serves one purpose: to indicate that an error should be returned if the user is attempting - /// to modify a var that should not be modified by direct user action; e.g., a read-only var. - const USER = 1 << 8; } } @@ -52,6 +47,35 @@ fn from(val: EnvMode) -> Self { } } +#[derive(Copy, Clone, Default)] +pub struct EnvSetMode { + pub mode: EnvMode, + + /// Flag for variable update request from the user. All variable changes that are made directly + /// by the user, such as those from the `read` and `set` builtin must have this flag set. It + /// serves to indicate that an error should be returned if the user is attempting to modify + /// a var that should not be modified by direct user action; e.g., a read-only var. + pub user: bool, + + pub is_repainting: bool, +} + +impl EnvSetMode { + pub fn new(mode: EnvMode, is_repainting: bool) -> Self { + Self::new_with(mode, false, is_repainting) + } + pub fn new_with(mode: EnvMode, user: bool, is_repainting: bool) -> Self { + Self { + mode, + user, + is_repainting, + } + } + pub fn new_at_early_startup(mode: EnvMode) -> Self { + Self::new_with(mode, false, false) + } +} + /// A collection of status and pipestatus. #[derive(Clone, Debug)] pub struct Statuses { @@ -294,6 +318,7 @@ pub fn is_read_only(name: &wstr) -> bool { #[cfg(test)] mod tests { use super::{EnvMode, EnvVar, EnvVarFlags}; + use crate::env::EnvSetMode; use crate::env::environment::{EnvStack, Environment}; use crate::prelude::*; use crate::tests::prelude::*; @@ -306,7 +331,11 @@ mod tests { fn return_timezone_hour(tstamp: SystemTime, timezone: &wstr) -> libc::c_int { let vars = EnvStack::globals().create_child(true /* dispatches_var_changes */); - vars.set_one(L!("TZ"), EnvMode::EXPORT, timezone.to_owned()); + vars.set_one( + L!("TZ"), + EnvSetMode::new(EnvMode::EXPORT, false), + timezone.to_owned(), + ); let _var = vars.get(L!("TZ")); diff --git a/src/env_dispatch.rs b/src/env_dispatch.rs index 80f38618c..b6f01aa34 100644 --- a/src/env_dispatch.rs +++ b/src/env_dispatch.rs @@ -48,8 +48,14 @@ once_cell::sync::Lazy::new(|| { let mut table = VarDispatchTable::default(); + macro_rules! vars { + ( $f:ident ) => { + |vars: &EnvStack, _suppress_repaint: bool| $f(vars) + }; + } + for name in LOCALE_VARIABLES { - table.add_anon(name, handle_locale_change); + table.add_anon(name, vars!(handle_locale_change)); } for name in CURSES_VARIABLES { @@ -60,43 +66,49 @@ table.add_anon(L!("COLORTERM"), handle_fish_term_change); table.add_anon(L!("fish_term256"), handle_fish_term_change); table.add_anon(L!("fish_term24bit"), handle_fish_term_change); - table.add_anon(L!("fish_escape_delay_ms"), update_wait_on_escape_ms); + table.add_anon(L!("fish_escape_delay_ms"), vars!(update_wait_on_escape_ms)); table.add_anon( L!("fish_sequence_key_delay_ms"), - update_wait_on_sequence_key_ms, + vars!(update_wait_on_sequence_key_ms), ); - table.add_anon(L!("fish_emoji_width"), guess_emoji_width); - table.add_anon(L!("fish_ambiguous_width"), handle_change_ambiguous_width); - table.add_anon(L!("LINES"), handle_term_size_change); - table.add_anon(L!("COLUMNS"), handle_term_size_change); - table.add_anon(L!("fish_complete_path"), handle_complete_path_change); - table.add_anon(L!("fish_function_path"), handle_function_path_change); - table.add_anon(L!("fish_read_limit"), handle_read_limit_change); - table.add_anon(L!("fish_history"), handle_fish_history_change); + table.add_anon(L!("fish_emoji_width"), vars!(guess_emoji_width)); + table.add_anon( + L!("fish_ambiguous_width"), + vars!(handle_change_ambiguous_width), + ); + table.add_anon(L!("LINES"), vars!(handle_term_size_change)); + table.add_anon(L!("COLUMNS"), vars!(handle_term_size_change)); + table.add_anon(L!("fish_complete_path"), vars!(handle_complete_path_change)); + table.add_anon(L!("fish_function_path"), vars!(handle_function_path_change)); + table.add_anon(L!("fish_read_limit"), vars!(handle_read_limit_change)); + table.add_anon(L!("fish_history"), vars!(handle_fish_history_change)); table.add_anon( L!("fish_autosuggestion_enabled"), - handle_autosuggestion_change, + vars!(handle_autosuggestion_change), + ); + table.add_anon( + L!("fish_transient_prompt"), + vars!(handle_transient_prompt_change), ); - table.add_anon(L!("fish_transient_prompt"), handle_transient_prompt_change); table.add_anon( L!("fish_use_posix_spawn"), - handle_fish_use_posix_spawn_change, + vars!(handle_fish_use_posix_spawn_change), ); - table.add_anon(L!("fish_trace"), handle_fish_trace); + table.add_anon(L!("fish_trace"), vars!(handle_fish_trace)); table.add_anon( L!("fish_cursor_selection_mode"), - handle_fish_cursor_selection_mode_change, + vars!(handle_fish_cursor_selection_mode_change), ); table.add_anon( L!("fish_cursor_end_mode"), - handle_fish_cursor_end_mode_change, + vars!(handle_fish_cursor_end_mode_change), ); table }); type NamedEnvCallback = fn(name: &wstr, env: &EnvStack); -type AnonEnvCallback = fn(env: &EnvStack); +type AnonEnvCallback = fn(env: &EnvStack, suppress_repaint: bool); enum EnvCallback { Named(NamedEnvCallback), @@ -121,10 +133,10 @@ pub fn add_anon(&mut self, name: &'static wstr, callback: AnonEnvCallback) { assert!(prev.is_none(), "Already observing {}", name); } - pub fn dispatch(&self, key: &wstr, vars: &EnvStack) { + pub fn dispatch(&self, key: &wstr, vars: &EnvStack, suppress_repaint: bool) { match self.table.get(key) { Some(EnvCallback::Named(named)) => (named)(key, vars), - Some(EnvCallback::Anon(anon)) => (anon)(vars), + Some(EnvCallback::Anon(anon)) => (anon)(vars, suppress_repaint), None => (), } } @@ -207,29 +219,40 @@ pub fn guess_emoji_width(vars: &EnvStack) { } } +pub struct VarChangeMilieu { + pub is_repainting: bool, + pub global_or_universal: bool, +} + /// React to modifying the given variable. -pub fn env_dispatch_var_change(key: &wstr, vars: &EnvStack) { +pub fn env_dispatch_var_change(milieu: VarChangeMilieu, key: &wstr, vars: &EnvStack) { use once_cell::sync::Lazy; + let suppress_repaint = milieu.is_repainting || !milieu.global_or_universal; + // We want to ignore variable changes until the dispatch table is explicitly initialized. if let Some(dispatch_table) = Lazy::get(&VAR_DISPATCH_TABLE) { - dispatch_table.dispatch(key, vars); + dispatch_table.dispatch(key, vars, suppress_repaint); } // TODO(MSRV>=1.88): if-let - if let Some(data) = reader_current_data() { - if string_prefixes_string(L!("fish_color_"), key) || { - // TODO Don't re-exec prompt when only pager color changed. - string_prefixes_string(L!("fish_pager_color_"), key) - } { - data.schedule_prompt_repaint(); + if !suppress_repaint { + if let Some(data) = reader_current_data() { + if string_prefixes_string(L!("fish_color_"), key) || { + // TODO Don't re-exec prompt when only pager color changed. + string_prefixes_string(L!("fish_pager_color_"), key) + } { + data.schedule_prompt_repaint(); + } } } } -fn handle_fish_term_change(vars: &EnvStack) { +fn handle_fish_term_change(vars: &EnvStack, suppress_repaint: bool) { update_fish_color_support(vars); - reader_schedule_prompt_repaint(); + if !suppress_repaint { + reader_schedule_prompt_repaint(); + } } fn handle_change_ambiguous_width(vars: &EnvStack) { @@ -311,11 +334,13 @@ fn handle_locale_change(vars: &EnvStack) { init_locale(vars); } -fn handle_term_change(vars: &EnvStack) { +fn handle_term_change(vars: &EnvStack, suppress_repaint: bool) { guess_emoji_width(vars); init_terminal(vars); read_terminfo_database(vars); - reader_schedule_prompt_repaint(); + if !suppress_repaint { + reader_schedule_prompt_repaint(); + } } fn handle_fish_use_posix_spawn_change(vars: &EnvStack) { diff --git a/src/exec.rs b/src/exec.rs index 2a23f69d9..8e00f0d93 100644 --- a/src/exec.rs +++ b/src/exec.rs @@ -11,7 +11,7 @@ ScopeGuard, bytes2wcstring, exit_without_destructors, truncate_at_nul, wcs2bytes, wcs2zstring, write_loop, }; -use crate::env::{EnvMode, EnvStack, Environment, READ_BYTE_LIMIT, Statuses}; +use crate::env::{EnvMode, EnvSetMode, EnvStack, Environment, READ_BYTE_LIMIT, Statuses}; #[cfg(have_posix_spawn)] use crate::env_dispatch::use_posix_spawn; use crate::fds::make_fd_blocking; @@ -32,7 +32,7 @@ }; use crate::nix::{getpid, isatty}; use crate::null_terminated_array::OwningNullTerminatedArray; -use crate::parser::{Block, BlockId, BlockType, EvalRes, Parser}; +use crate::parser::{Block, BlockId, BlockType, EvalRes, Parser, ParserEnvSetMode}; use crate::prelude::*; use crate::proc::Pid; use crate::proc::{ @@ -98,12 +98,12 @@ pub fn exec_job(parser: &Parser, job: &Job, block_io: IoChain) -> bool { for assignment in &job.processes()[0].variable_assignments { parser.set_var( &assignment.variable_name, - EnvMode::LOCAL | EnvMode::EXPORT, + ParserEnvSetMode::new(EnvMode::LOCAL | EnvMode::EXPORT), assignment.values.clone(), ); } - internal_exec(parser.vars(), job, block_io); + internal_exec(parser.vars(), parser.is_repainting(), job, block_io); // internal_exec only returns if it failed to set up redirections. // In case of an successful exec, this code is not reached. let status = if job.flags().negate { 0 } else { 1 }; @@ -234,11 +234,13 @@ pub fn exec_job(parser: &Parser, job: &Job, block_io: IoChain) -> bool { // a pgroup, so error out before setting last_pid. if !job.is_foreground() { if let Some(last_pid) = job.get_last_pid() { - parser - .vars() - .set_one(L!("last_pid"), EnvMode::GLOBAL, last_pid.to_wstring()); + parser.set_one( + L!("last_pid"), + ParserEnvSetMode::new(EnvMode::GLOBAL), + last_pid.to_wstring(), + ); } else { - parser.vars().set_empty(L!("last_pid"), EnvMode::GLOBAL); + parser.set_empty(L!("last_pid"), ParserEnvSetMode::new(EnvMode::GLOBAL)); } } @@ -479,7 +481,7 @@ fn can_use_posix_spawn_for_job(job: &Job, dup2s: &Dup2List) -> bool { !wants_terminal } -fn internal_exec(vars: &EnvStack, j: &Job, block_io: IoChain) { +fn internal_exec(vars: &EnvStack, is_repainting: bool, j: &Job, block_io: IoChain) { // Do a regular launch - but without forking first... let mut all_ios = block_io; if !all_ios.append_from_specs(j.processes()[0].redirection_specs(), &vars.get_pwd_slash()) { @@ -508,7 +510,8 @@ fn internal_exec(vars: &EnvStack, j: &Job, block_io: IoChain) { { // Decrement SHLVL as we're removing ourselves from the shell "stack". if is_interactive_session() { - let shlvl_var = vars.getf(L!("SHLVL"), EnvMode::GLOBAL | EnvMode::EXPORT); + let global_exported_mode = EnvMode::GLOBAL | EnvMode::EXPORT; + let shlvl_var = vars.getf(L!("SHLVL"), global_exported_mode); let mut shlvl_str = L!("0").to_owned(); if let Some(shlvl_var) = shlvl_var { if let Ok(shlvl) = fish_wcstol(&shlvl_var.as_string()) { @@ -517,7 +520,11 @@ fn internal_exec(vars: &EnvStack, j: &Job, block_io: IoChain) { } } } - vars.set_one(L!("SHLVL"), EnvMode::GLOBAL | EnvMode::EXPORT, shlvl_str); + vars.set_one( + L!("SHLVL"), + EnvSetMode::new(global_exported_mode, is_repainting), + shlvl_str, + ); } // launch_process _never_ returns. @@ -957,15 +964,17 @@ fn function_prepare_environment( // 2. inherited variables // 3. argv + let mode = parser.convert_env_set_mode(ParserEnvSetMode::user(EnvMode::LOCAL)); + let mut overwrite_argv = false; for (idx, named_arg) in props.named_arguments.iter().enumerate() { if named_arg == L!("argv") { overwrite_argv = true }; if idx < argv.len() { - vars.set_one(named_arg, EnvMode::LOCAL | EnvMode::USER, argv[idx].clone()); + vars.set_one(named_arg, mode, argv[idx].clone()); } else { - vars.set_empty(named_arg, EnvMode::LOCAL | EnvMode::USER); + vars.set_empty(named_arg, mode); } } @@ -973,11 +982,11 @@ fn function_prepare_environment( if key == L!("argv") { overwrite_argv = true }; - vars.set(key, EnvMode::LOCAL | EnvMode::USER, value.clone()); + vars.set(key, mode, value.clone()); } if !overwrite_argv { - vars.set_argv(argv); + vars.set_argv(argv, mode.is_repainting); } fb } @@ -1310,7 +1319,7 @@ fn exec_process_in_job( for assignment in &p.variable_assignments { parser.set_var( &assignment.variable_name, - EnvMode::LOCAL | EnvMode::EXPORT, + ParserEnvSetMode::new(EnvMode::LOCAL | EnvMode::EXPORT), assignment.values.clone(), ); } diff --git a/src/expand.rs b/src/expand.rs index 0c2722aee..7b6cb647d 100644 --- a/src/expand.rs +++ b/src/expand.rs @@ -1597,6 +1597,7 @@ mod tests { use crate::expand::{ExpandResultCode, expand_to_receiver}; use crate::operation_context::{EXPANSION_LIMIT_DEFAULT, no_cancel}; use crate::parse_constants::ParseErrorList; + use crate::parser::ParserEnvSetMode; use crate::tests::prelude::*; use crate::wildcard::ANY_STRING; use crate::{ @@ -1957,7 +1958,7 @@ fn test_expand_overflow() { let parser = TestParser::new(); parser.vars().push(true); - let set = parser.vars().set(L!("bigvar"), EnvMode::LOCAL, vals); + let set = parser.set_var(L!("bigvar"), ParserEnvSetMode::new(EnvMode::LOCAL), vals); assert_eq!(set, EnvStackSetResult::Ok); let mut errors = ParseErrorList::new(); @@ -1977,7 +1978,7 @@ fn test_expand_overflow() { assert_ne!(errors, vec![]); assert_eq!(res, ExpandResultCode::error); - parser.vars().pop(); + parser.vars().pop(false); } #[test] diff --git a/src/highlight/highlight.rs b/src/highlight/highlight.rs index ad7f93abb..d80c9c904 100644 --- a/src/highlight/highlight.rs +++ b/src/highlight/highlight.rs @@ -1274,7 +1274,7 @@ pub struct HighlightSpec { mod tests { use super::{HighlightColorResolver, HighlightRole, HighlightSpec, highlight_shell}; use crate::common::ScopeGuard; - use crate::env::{EnvMode, Environment}; + use crate::env::{EnvMode, EnvSetMode, Environment}; use crate::future_feature_flags::{self, FeatureFlag}; use crate::highlight::parse_text_face_for_highlight; use crate::operation_context::{EXPANSION_LIMIT_BACKGROUND, OperationContext}; @@ -1386,26 +1386,15 @@ macro_rules! validate { // Verify variables and wildcards in commands using /bin/cat. let vars = parser.vars(); - vars.set_one( - L!("CDPATH"), - EnvMode::LOCAL, - L!("./cdpath-entry").to_owned(), - ); + let local_mode = EnvSetMode::new_at_early_startup(EnvMode::LOCAL); + vars.set_one(L!("CDPATH"), local_mode, L!("./cdpath-entry").to_owned()); - vars.set_one( - L!("VARIABLE_IN_COMMAND"), - EnvMode::LOCAL, - L!("a").to_owned(), - ); - vars.set_one( - L!("VARIABLE_IN_COMMAND2"), - EnvMode::LOCAL, - L!("at").to_owned(), - ); + vars.set_one(L!("VARIABLE_IN_COMMAND"), local_mode, L!("a").to_owned()); + vars.set_one(L!("VARIABLE_IN_COMMAND2"), local_mode, L!("at").to_owned()); let _cleanup = ScopeGuard::new((), |_| { - vars.remove(L!("VARIABLE_IN_COMMAND"), EnvMode::default()); - vars.remove(L!("VARIABLE_IN_COMMAND2"), EnvMode::default()); + vars.remove(L!("VARIABLE_IN_COMMAND"), EnvSetMode::default()); + vars.remove(L!("VARIABLE_IN_COMMAND2"), EnvSetMode::default()); }); validate!( @@ -1798,7 +1787,7 @@ fn test_trailing_spaces_after_command() { // First, set up fish_color_command to include underline vars.set_one( L!("fish_color_command"), - EnvMode::LOCAL, + EnvSetMode::new_at_early_startup(EnvMode::LOCAL), L!("--underline").to_owned(), ); @@ -1850,7 +1839,7 @@ fn test_resolve_role() { let vars = parser.vars(); let set = |var: &wstr, value: Vec| { - vars.set(var, EnvMode::LOCAL, value); + vars.set(var, EnvSetMode::new(EnvMode::LOCAL, false), value); }; set(L!("fish_color_normal"), vec![L!("normal").into()]); set( diff --git a/src/history/history.rs b/src/history/history.rs index ab860135d..fee3f3ac9 100644 --- a/src/history/history.rs +++ b/src/history/history.rs @@ -16,7 +16,7 @@ use crate::{ common::cstr2wcstring, - env::EnvVar, + env::{EnvSetMode, EnvVar}, fs::{ LOCKED_FILE_MODE, LockedFile, LockingMode, PotentialUpdate, WriteMethod, lock_and_load, rewrite_via_temporary_file, @@ -1760,8 +1760,9 @@ pub fn all_paths_are_valid>( /// Sets private mode on. Once in private mode, it cannot be turned off. pub fn start_private_mode(vars: &EnvStack) { - vars.set_one(L!("fish_history"), EnvMode::GLOBAL, L!("").to_owned()); - vars.set_one(L!("fish_private_mode"), EnvMode::GLOBAL, L!("1").to_owned()); + let global_mode = EnvSetMode::new_at_early_startup(EnvMode::GLOBAL); + vars.set_one(L!("fish_history"), global_mode, L!("").to_owned()); + vars.set_one(L!("fish_private_mode"), global_mode, L!("1").to_owned()); } /// Queries private mode status. @@ -1777,7 +1778,7 @@ mod tests { }; use crate::common::ESCAPE_TEST_CHAR; use crate::common::{ScopeGuard, bytes2wcstring, wcs2bytes, wcs2osstring}; - use crate::env::{EnvMode, EnvStack}; + use crate::env::{EnvMode, EnvSetMode, EnvStack}; use crate::fs::{LockedFile, WriteMethod}; use crate::path::path_get_data; use crate::prelude::*; @@ -2270,8 +2271,9 @@ fn test_history_path_detection() { let wdir_path = WString::from(tmpdir.path().to_str().unwrap()); let test_vars = EnvStack::new(); - test_vars.set_one(L!("PWD"), EnvMode::GLOBAL, wdir_path.clone()); - test_vars.set_one(L!("HOME"), EnvMode::GLOBAL, wdir_path.clone()); + let global_mode = EnvSetMode::new(EnvMode::GLOBAL, false); + test_vars.set_one(L!("PWD"), global_mode, wdir_path.clone()); + test_vars.set_one(L!("HOME"), global_mode, wdir_path.clone()); let history = History::with_name(L!("path_detection")); history.clear(); diff --git a/src/parse_execution.rs b/src/parse_execution.rs index 4bb80d127..5b4b26fc5 100644 --- a/src/parse_execution.rs +++ b/src/parse_execution.rs @@ -35,7 +35,9 @@ MaybeParentheses::CommandSubstitution, parse_util_locate_cmdsubst_range, parse_util_unescape_wildcards, }; -use crate::parser::{Block, BlockData, BlockId, BlockType, LoopStatus, Parser, ProfileItem}; +use crate::parser::{ + Block, BlockData, BlockId, BlockType, LoopStatus, Parser, ParserEnvSetMode, ProfileItem, +}; use crate::parser_keywords::parser_keywords_is_subcommand; use crate::path::{path_as_implicit_cd, path_try_get_path}; use crate::prelude::*; @@ -648,8 +650,11 @@ fn apply_variable_assignments( vals.clone(), )); } - ctx.parser() - .set_var_and_fire(variable_name, EnvMode::LOCAL | EnvMode::EXPORT, vals); + ctx.parser().set_var_and_fire( + variable_name, + ParserEnvSetMode::new(EnvMode::LOCAL | EnvMode::EXPORT), + vals, + ); } EndExecutionReason::Ok } @@ -928,7 +933,7 @@ fn run_for_statement( let retval = ctx.parser().set_var( &for_var_name, - EnvMode::LOCAL | EnvMode::USER, + ParserEnvSetMode::user(EnvMode::LOCAL), var.map_or(vec![], |var| var.as_list().to_owned()), ); assert!(retval == EnvStackSetResult::Ok); @@ -946,9 +951,11 @@ fn run_for_statement( break; } - let retval = ctx - .parser() - .set_var(&for_var_name, EnvMode::USER, vec![val]); + let retval = ctx.parser().set_var( + &for_var_name, + ParserEnvSetMode::user(EnvMode::empty()), + vec![val], + ); assert!( retval == EnvStackSetResult::Ok, "for loop variable should have been successfully set" diff --git a/src/parser.rs b/src/parser.rs index b17f87473..d421daac6 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -8,7 +8,8 @@ }; use crate::complete::CompletionList; use crate::env::{ - EnvMode, EnvStack, EnvStackSetResult, Environment, FISH_TERMINAL_COLOR_THEME_VAR, Statuses, + EnvMode, EnvSetMode, EnvStack, EnvStackSetResult, Environment, FISH_TERMINAL_COLOR_THEME_VAR, + Statuses, }; use crate::event::{self, Event}; use crate::expand::{ @@ -437,6 +438,21 @@ pub struct Parser { pub blocking_query_timeout: RefCell>, } +#[derive(Copy, Clone, Default)] +pub struct ParserEnvSetMode { + pub mode: EnvMode, + pub user: bool, +} + +impl ParserEnvSetMode { + pub fn new(mode: EnvMode) -> Self { + Self { mode, user: false } + } + pub fn user(mode: EnvMode) -> Self { + Self { mode, user: true } + } +} + impl Parser { /// Create a parser. pub fn new(variables: EnvStack, cancel_behavior: CancelBehavior) -> Parser { @@ -945,7 +961,7 @@ pub fn set_last_statuses(&self, s: Statuses) { pub fn set_var_and_fire( &self, key: &wstr, - mode: EnvMode, + mode: ParserEnvSetMode, vals: Vec, ) -> EnvStackSetResult { let res = self.set_var(key, mode, vals); @@ -955,17 +971,49 @@ pub fn set_var_and_fire( res } + pub fn is_repainting(&self) -> bool { + self.libdata().is_repaint + } + + pub fn convert_env_set_mode(&self, mode: ParserEnvSetMode) -> EnvSetMode { + EnvSetMode::new_with(mode.mode, mode.user, self.is_repainting()) + } + /// Cover of vars().set(), without firing events - pub fn set_var(&self, key: &wstr, mode: EnvMode, vals: Vec) -> EnvStackSetResult { + pub fn set_var( + &self, + key: &wstr, + mode: ParserEnvSetMode, + vals: Vec, + ) -> EnvStackSetResult { + let mode = self.convert_env_set_mode(mode); self.vars().set(key, mode, vals) } + /// Cover of vars().set_one(), without firing events + pub fn set_one(&self, key: &wstr, mode: ParserEnvSetMode, val: WString) -> EnvStackSetResult { + let mode = self.convert_env_set_mode(mode); + self.vars().set_one(key, mode, val) + } + + /// Cover of vars().set_empty(), without firing events + pub fn set_empty(&self, key: &wstr, mode: ParserEnvSetMode) -> EnvStackSetResult { + let mode = self.convert_env_set_mode(mode); + self.vars().set_empty(key, mode) + } + + /// Cover of vars().remove(), without firing events + pub fn remove_var(&self, key: &wstr, mode: ParserEnvSetMode) -> EnvStackSetResult { + let mode = self.convert_env_set_mode(mode); + self.vars().remove(key, mode) + } + /// Update any universal variables and send event handlers. /// If `always` is set, then do it even if we have no pending changes (that is, look for /// changes from other fish instances); otherwise only sync if this instance has changed uvars. pub fn sync_uvars_and_fire(&self, always: bool) { if self.syncs_uvars.load() { - let evts = self.vars().universal_sync(always); + let evts = self.vars().universal_sync(always, self.is_repainting()); for evt in evts { event::fire(self, evt); } @@ -994,7 +1042,7 @@ pub fn pop_block(&self, expected: BlockId) { block_list.pop().unwrap() }; if block.wants_pop_env() { - self.vars().pop(); + self.vars().pop(self.is_repainting()); } } @@ -1247,7 +1295,7 @@ pub fn set_color_theme(&self, background_color: Option<&xterm_color::Color>) { ); self.set_var_and_fire( FISH_TERMINAL_COLOR_THEME_VAR, - EnvMode::GLOBAL, + ParserEnvSetMode::new(EnvMode::GLOBAL), vec![color_theme.to_owned()], ); } diff --git a/src/path.rs b/src/path.rs index d893885ed..e07451a60 100644 --- a/src/path.rs +++ b/src/path.rs @@ -3,7 +3,7 @@ //! path-related issues. use crate::common::{wcs2osstring, wcs2zstring}; -use crate::env::{EnvMode, EnvStack, Environment, FALLBACK_PATH}; +use crate::env::{EnvMode, EnvSetMode, EnvStack, Environment, FALLBACK_PATH}; use crate::expand::{HOME_DIRECTORY, expand_tilde}; use crate::flog::{flog, flogf}; use crate::prelude::*; @@ -134,15 +134,13 @@ fn maybe_issue_path_warning( vars: &EnvStack, ) { let warning_var_name = L!("_FISH_WARNED_").to_owned() + which_dir; - if vars - .getf(&warning_var_name, EnvMode::GLOBAL | EnvMode::EXPORT) - .is_some() - { + let global_exported_mode = EnvMode::GLOBAL | EnvMode::EXPORT; + if vars.getf(&warning_var_name, global_exported_mode).is_some() { return; } vars.set_one( &warning_var_name, - EnvMode::GLOBAL | EnvMode::EXPORT, + EnvSetMode::new_at_early_startup(global_exported_mode), L!("1").to_owned(), ); diff --git a/src/reader/reader.rs b/src/reader/reader.rs index 8d96dc052..a3808b59e 100644 --- a/src/reader/reader.rs +++ b/src/reader/reader.rs @@ -113,6 +113,7 @@ parse_util_get_line_from_offset, parse_util_get_offset, parse_util_get_offset_from_line, parse_util_lineno, parse_util_locate_cmdsubst_range, parse_util_token_extent, }; +use crate::parser::ParserEnvSetMode; use crate::parser::{BlockType, EvalRes, Parser}; use crate::prelude::*; use crate::proc::{ @@ -392,9 +393,11 @@ pub fn reader_push<'a>(parser: &'a Parser, history_name: &wstr, conf: ReaderConf // Provide value for `status current-command` parser.libdata_mut().status_vars.command = L!("fish").to_owned(); // Also provide a value for the deprecated fish 2.0 $_ variable - parser - .vars() - .set_one(L!("_"), EnvMode::GLOBAL, L!("fish").to_owned()); + parser.set_one( + L!("_"), + ParserEnvSetMode::new(EnvMode::GLOBAL), + L!("fish").to_owned(), + ); let old = parser .blocking_query_timeout .replace(input_data.blocking_query_timeout); @@ -5988,9 +5991,11 @@ fn reader_run_command(parser: &Parser, cmd: &wstr) -> EvalRes { parser.libdata_mut().status_vars.command = ft.to_owned(); parser.libdata_mut().status_vars.commandline = cmd.to_owned(); // Also provide a value for the deprecated fish 2.0 $_ variable - parser - .vars() - .set_one(L!("_"), EnvMode::GLOBAL, ft.to_owned()); + parser.set_one( + L!("_"), + ParserEnvSetMode::new(EnvMode::GLOBAL), + ft.to_owned(), + ); } reader_write_title(cmd, parser, true); @@ -6008,9 +6013,9 @@ fn reader_run_command(parser: &Parser, cmd: &wstr) -> EvalRes { if !ft.is_empty() { let time_after = Instant::now(); let duration = time_after.duration_since(time_before); - parser.vars().set_one( + parser.set_one( ENV_CMD_DURATION, - EnvMode::UNEXPORT, + ParserEnvSetMode::new(EnvMode::UNEXPORT), duration.as_millis().to_wstring(), ); } @@ -6020,9 +6025,11 @@ fn reader_run_command(parser: &Parser, cmd: &wstr) -> EvalRes { // Provide value for `status current-command` parser.libdata_mut().status_vars.command = get_program_name().to_owned(); // Also provide a value for the deprecated fish 2.0 $_ variable - parser - .vars() - .set_one(L!("_"), EnvMode::GLOBAL, get_program_name().to_owned()); + parser.set_one( + L!("_"), + ParserEnvSetMode::new(EnvMode::GLOBAL), + get_program_name().to_owned(), + ); // Provide value for `status current-commandline` parser.libdata_mut().status_vars.commandline = L!("").to_owned(); diff --git a/src/termsize.rs b/src/termsize.rs index 7ecadebfb..fcea3fd22 100644 --- a/src/termsize.rs +++ b/src/termsize.rs @@ -2,7 +2,7 @@ use crate::common::assert_sync; use crate::env::{EnvMode, EnvVar, Environment}; use crate::flog::flog; -use crate::parser::Parser; +use crate::parser::{Parser, ParserEnvSetMode}; use crate::prelude::*; use crate::wutil::fish_wcstoi; use std::mem::MaybeUninit; @@ -200,12 +200,12 @@ fn set_columns_lines_vars(&self, val: Termsize, parser: &Parser) { let saved = self.setting_env_vars.swap(true, Ordering::Relaxed); parser.set_var_and_fire( L!("COLUMNS"), - EnvMode::GLOBAL, + ParserEnvSetMode::new(EnvMode::GLOBAL), vec![val.width().to_wstring()], ); parser.set_var_and_fire( L!("LINES"), - EnvMode::GLOBAL, + ParserEnvSetMode::new(EnvMode::GLOBAL), vec![val.height().to_wstring()], ); self.setting_env_vars.store(saved, Ordering::Relaxed); @@ -262,7 +262,7 @@ pub fn safe_termsize_invalidate_tty() { #[cfg(test)] mod tests { - use crate::env::{EnvMode, Environment}; + use crate::env::{EnvMode, EnvSetMode, Environment}; use crate::termsize::*; use crate::tests::prelude::*; use std::sync::Mutex; @@ -272,7 +272,7 @@ mod tests { #[serial] fn test_termsize() { let _cleanup = test_init(); - let env_global = EnvMode::GLOBAL; + let env_global = EnvSetMode::new(EnvMode::GLOBAL, false); let parser = TestParser::new(); let vars = parser.vars(); diff --git a/tests/checks/tmux-repaint.fish b/tests/checks/tmux-repaint.fish new file mode 100644 index 000000000..14057b64e --- /dev/null +++ b/tests/checks/tmux-repaint.fish @@ -0,0 +1,49 @@ +#RUN: %fish %s +#REQUIRES: command -v tmux + +isolated-tmux-start -C ' + function fish_prompt + set -g counter (math $counter + 1) + set -g fish_color_status red + set -g fish_pager_color_background --background=white + echo "$counter> " + set -ga TERM . + set -ga TERMINFO . + set -ga TERMINFO_DIRS . + set -ga COLORTERM . + set -ga fish_term256 . + set -ga fish_term24bit . + end + set -eg fish_color_param + bind ctrl-g,A "{ set -l fish_color_command 111 }" + bind ctrl-g,B "set -l fish_color_command 222" + bind ctrl-g,C "set -e fish_color_command" + bind ctrl-g,D "set -eg fish_color_param" + bind ctrl-g,E "set -g fish_color_command 333" + bind ctrl-g,F "set -U fish_color_param 444" + bind ctrl-g,G "set -eg fish_color_command" +' + +isolated-tmux capture-pane -p +# The weird global assignments in fish prompt cause an initial repaint. +# CHECK: 2> + +isolated-tmux send-keys C-g A C-g B C-g C C-g D +tmux-sleep +isolated-tmux capture-pane -p +# CHECK: 2> + +isolated-tmux send-keys C-g E +tmux-sleep +isolated-tmux capture-pane -p +# CHECK: 3> + +isolated-tmux send-keys C-g F +tmux-sleep +isolated-tmux capture-pane -p +# CHECK: 4> + +isolated-tmux send-keys C-g G +tmux-sleep +isolated-tmux capture-pane -p +# CHECK: 5>