diff --git a/builtin_set.cpp b/builtin_set.cpp index aae87550d..e74cc6d61 100644 --- a/builtin_set.cpp +++ b/builtin_set.cpp @@ -69,7 +69,7 @@ static int my_env_set(const wchar_t *key, const wcstring_list_t &val, int scope) /* Don't bother validating (or complaining about) values that are already present */ wcstring_list_t existing_values; - const env_var_t existing_variable = env_get_string(key); + const env_var_t existing_variable = env_get_string(key, scope); if (! existing_variable.missing_or_empty()) tokenize_variable_array(existing_variable, existing_values); @@ -360,7 +360,7 @@ static void print_variables(int include_values, int esc, bool shorten_ok, int sc if (include_values) { - env_var_t value = env_get_string(key); + env_var_t value = env_get_string(key, scope); if (!value.missing()) { int shorten = 0; @@ -606,7 +606,7 @@ static int builtin_set(parser_t &parser, wchar_t **argv) wcstring_list_t result; size_t j; - env_var_t dest_str = env_get_string(dest); + env_var_t dest_str = env_get_string(dest, scope); if (! dest_str.missing()) tokenize_variable_array(dest_str, result); @@ -696,14 +696,6 @@ static int builtin_set(parser_t &parser, wchar_t **argv) return 1; } - if (slice && erase && (scope != ENV_USER)) - { - free(dest); - append_format(stderr_buffer, _(L"%ls: Can not specify scope when erasing array slice\n"), argv[0]); - builtin_print_help(parser, argv[0], stderr_buffer); - return 1; - } - /* set assignment can work in two modes, either using slices or using the whole array. We detect which mode is used here. @@ -718,7 +710,7 @@ static int builtin_set(parser_t &parser, wchar_t **argv) std::vector indexes; wcstring_list_t result; - const env_var_t dest_str = env_get_string(dest); + const env_var_t dest_str = env_get_string(dest, scope); if (! dest_str.missing()) tokenize_variable_array(dest_str, result); diff --git a/doc_src/set.txt b/doc_src/set.txt index f72d90187..6602e860f 100644 --- a/doc_src/set.txt +++ b/doc_src/set.txt @@ -65,11 +65,7 @@ to the scoping rules for variables: In query mode, the scope to be examined can be specified. In erase mode, if variable indices are specified, only the specified -slices of the array variable will be erased. When erasing an entire -variable (i.e. no slicing), the scope of the variable to be erased can -be specified. That way, a global variable can be erased even if a -local variable with the same name exists. Scope can not be specified -when erasing a slice of an array. The innermost scope is always used. +slices of the array variable will be erased. \c set requires all options to come before any other arguments. For example, set flags -l will have diff --git a/env.cpp b/env.cpp index 77ee75aa5..278e2af4e 100644 --- a/env.cpp +++ b/env.cpp @@ -936,82 +936,107 @@ const wchar_t *env_var_t::c_str(void) const return wcstring::c_str(); } -env_var_t env_get_string(const wcstring &key) +env_var_t env_get_string(const wcstring &key, int mode) { - /* Big hack...we only allow getting the history on the main thread. Note that history_t may ask for an environment variable, so don't take the lock here (we don't need it) */ - const bool is_main = is_main_thread(); - if (key == L"history" && is_main) - { - env_var_t result; + const bool has_scope = mode & (ENV_LOCAL | ENV_GLOBAL | ENV_UNIVERSAL); + const bool search_local = !has_scope || (mode & ENV_LOCAL); + const bool search_global = !has_scope || (mode & ENV_GLOBAL); + const bool search_universal = !has_scope || (mode & ENV_UNIVERSAL); - history_t *history = reader_get_history(); - if (! history) + const bool search_exported = (mode & ENV_EXPORT) || !(mode & ENV_UNEXPORT); + const bool search_unexported = (mode & ENV_UNEXPORT) || !(mode & ENV_EXPORT); + + /* Make the assumption that electric keys can't be shadowed elsewhere, since we currently block that in env_set() */ + if (is_electric(key)) + { + if (!search_global) return env_var_t::missing_var(); + /* Big hack...we only allow getting the history on the main thread. Note that history_t may ask for an environment variable, so don't take the lock here (we don't need it) */ + if (key == L"history" && is_main_thread()) { - history = &history_t::history_with_name(L"fish"); - } - if (history) - history->get_string_representation(result, ARRAY_SEP_STR); - return result; - } - else if (key == L"COLUMNS") - { - return to_string(common_get_width()); - } - else if (key == L"LINES") - { - return to_string(common_get_height()); - } - else if (key == L"status") - { - return to_string(proc_get_last_status()); - } - else if (key == L"umask") - { - return format_string(L"0%0.3o", get_umask()); - } - else - { - { - /* Lock around a local region */ - scoped_lock lock(env_lock); + env_var_t result; - env_node_t *env = top; - wcstring result; - - while (env != NULL) + history_t *history = reader_get_history(); + if (! history) { - const var_entry_t *entry = env->find_entry(key); - if (entry != NULL) - { - if (entry->val == ENV_NULL) - { - return env_var_t::missing_var(); - } - else - { - return entry->val; - } - } + history = &history_t::history_with_name(L"fish"); + } + if (history) + history->get_string_representation(result, ARRAY_SEP_STR); + return result; + } + else if (key == L"COLUMNS") + { + return to_string(common_get_width()); + } + else if (key == L"LINES") + { + return to_string(common_get_height()); + } + else if (key == L"status") + { + return to_string(proc_get_last_status()); + } + else if (key == L"umask") + { + return format_string(L"0%0.3o", get_umask()); + } + // we should never get here unless the electric var list is out of sync + } + if (search_local || search_global) { + /* Lock around a local region */ + scoped_lock lock(env_lock); + + env_node_t *env = search_local ? top : global_env; + wcstring result; + + while (env != NULL) + { + const var_entry_t *entry = env->find_entry(key); + if (entry != NULL && (entry->exportv ? search_exported : search_unexported)) + { + if (entry->val == ENV_NULL) + { + return env_var_t::missing_var(); + } + else + { + return entry->val; + } + } + + if (has_scope) + { + if (!search_global || env == global_env) break; + env = global_env; + } + else + { env = env->next_scope_to_search(); } } + } - /* Another big hack - only do a universal barrier on the main thread (since it can change variable values) - Make sure we do this outside the env_lock because it may itself call env_get_string */ - if (is_main && ! get_proc_had_barrier()) - { - set_proc_had_barrier(true); - env_universal_barrier(); - } + if (!search_universal) return env_var_t::missing_var(); - env_var_t env_var = uvars() ? uvars()->get(key) : env_var_t::missing_var(); - if (env_var == ENV_NULL) + /* Another big hack - only do a universal barrier on the main thread (since it can change variable values) + Make sure we do this outside the env_lock because it may itself call env_get_string */ + if (is_main_thread() && ! get_proc_had_barrier()) + { + set_proc_had_barrier(true); + env_universal_barrier(); + } + + if (uvars()) + { + env_var_t env_var = uvars()->get(key); + if (env_var == ENV_NULL || !(uvars()->get_export(key) ? search_exported : search_unexported)) { env_var = env_var_t::missing_var(); } return env_var; } + return env_var_t::missing_var(); } bool env_exist(const wchar_t *key, int mode) diff --git a/env.h b/env.h index a0931c87f..605fc453a 100644 --- a/env.h +++ b/env.h @@ -169,8 +169,13 @@ public: }; -/** Gets the variable with the specified name, or env_var_t::missing_var if it does not exist. */ -env_var_t env_get_string(const wcstring &key); +/** + Gets the variable with the specified name, or env_var_t::missing_var if it does not exist. + + \param key The name of the variable to get + \param mode An optional scope to search in. All scopes are searched if unset +*/ +env_var_t env_get_string(const wcstring &key, int mode = 0); /** Returns true if the specified key exists. This can't be reliably done