diff --git a/CHANGELOG.md b/CHANGELOG.md index 59403745c..423fd3fad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - The fish manual, tutorial and FAQ are now available in `man` format as `fish-doc`, `fish-tutorial` and `fish-faq` respectively (#5521). - Local values for `fish_complete_path` and `fish_function_path` are now ignored; only their global values are respected. - Empty universal variables may now be exported (#5992). +- Exported universal variables are no longer imported into the global scope, preventing shadowing. This makes it easier to change such variables for all fish sessions and avoids breakage when the value is a list of multiple elements (#5258). - A bug where local variables would not be exported to functions has been fixed (#6153). - A bug where `string split` would be drop empty strings if the output was only empty strings has been fixed (#5987). - `switch` now allows arguments that expand to nothing, like empty variables (#5677). diff --git a/src/env.cpp b/src/env.cpp index 7bf5f3523..38b590fa1 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -405,6 +405,19 @@ void env_init(const struct config_paths_t *paths /* or NULL */) { callback_data_list_t callbacks; s_universal_variables->initialize(callbacks); env_universal_callbacks(&env_stack_t::principal(), callbacks); + + // Do not import variables that have the same name and value as + // an exported universal variable. See issues #5258 and #5348. + for (const auto &kv : uvars()->get_table()) { + const wcstring &name = kv.first; + const env_var_t &uvar = kv.second; + if (!uvar.exports()) continue; + // Look for a global exported variable with the same name. + maybe_t global = vars.globals().get(name, ENV_GLOBAL | ENV_EXPORT); + if (global && uvar.as_string() == global->as_string()) { + vars.globals().remove(name, ENV_GLOBAL | ENV_EXPORT); + } + } } static int set_umask(const wcstring_list_t &list_val) { diff --git a/src/env_universal_common.h b/src/env_universal_common.h index c9553edfc..3268b1c54 100644 --- a/src/env_universal_common.h +++ b/src/env_universal_common.h @@ -111,6 +111,9 @@ class env_universal_t { // Gets variable names. wcstring_list_t get_names(bool show_exported, bool show_unexported) const; + /// Get a view on the universal variable table. + const var_table_t &get_table() const { return vars; } + /// Loads variables at the correct path, optionally migrating from a legacy path. bool initialize(callback_data_list_t &callbacks); diff --git a/tests/checks/set.fish b/tests/checks/set.fish index cd692bf17..db6855dbf 100644 --- a/tests/checks/set.fish +++ b/tests/checks/set.fish @@ -462,4 +462,16 @@ begin # CHECK: inner end +# Skip importing universal variables (#5258) +while set -q EDITOR; set -e EDITOR; end +set -Ux EDITOR emacs -nw +# CHECK: $EDITOR: not set in global scope +# CHECK: $EDITOR: set in universal scope, exported, with 2 elements +$FISH -c 'set -S EDITOR' | string match -r -e 'global|universal' + +# When the variable has been changed outside of fish we accept it. +# CHECK: $EDITOR: set in global scope, exported, with 1 elements +# CHECK: $EDITOR: set in universal scope, exported, with 2 elements +sh -c "EDITOR='vim -g' $FISH -c "'\'set -S EDITOR\'' | string match -r -e 'global|universal' + true