From f02526919580f972d901761c5c6e65fed0d1b8fb Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Tue, 30 Jan 2018 12:36:50 -0800 Subject: [PATCH] env_var_t to forget its name Store properties associated with the name via flags instead --- src/builtin_cd.cpp | 2 +- src/builtin_set.cpp | 3 +-- src/env.cpp | 34 ++++++++++++++++++++-------------- src/env.h | 36 ++++++++++++++++++++++++++---------- src/env_universal_common.cpp | 18 +++++++++--------- src/fish_tests.cpp | 2 +- src/highlight.cpp | 8 +++----- 7 files changed, 61 insertions(+), 42 deletions(-) diff --git a/src/builtin_cd.cpp b/src/builtin_cd.cpp index 542019a5d..b86981e5c 100644 --- a/src/builtin_cd.cpp +++ b/src/builtin_cd.cpp @@ -37,7 +37,7 @@ int builtin_cd(parser_t &parser, io_streams_t &streams, wchar_t **argv) { wcstring dir; if (argv[optind]) { - dir_in = env_var_t(L"", argv[optind]); // unamed var + dir_in = env_var_t(wcstring(argv[optind]), 0); // unamed var } else { auto maybe_dir_in = env_get(L"HOME"); if (maybe_dir_in.missing_or_empty()) { diff --git a/src/builtin_set.cpp b/src/builtin_set.cpp index fd2b2beb7..ac064aaea 100644 --- a/src/builtin_set.cpp +++ b/src/builtin_set.cpp @@ -532,8 +532,7 @@ static void show_scope(const wchar_t *var_name, int scope, io_streams_t &streams return; } - - const wchar_t *exportv = var->exportv ? _(L"exported") : _(L"unexported"); + const wchar_t *exportv = var->exports() ? _(L"exported") : _(L"unexported"); wcstring_list_t vals = var->as_list(); streams.out.append_format(_(L"$%ls: set in %ls scope, %ls, with %d elements\n"), var_name, scope_name, exportv, vals.size()); diff --git a/src/env.cpp b/src/env.cpp index 8ef67dc63..03bca8782 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -218,7 +218,7 @@ void var_stack_t::push(bool new_scope) { // "--no-scope-shadowing". if (new_scope && top_node != this->global_env) { for (const auto &var : top_node->env) { - if (var.second.exportv) node->env.insert(var); + if (var.second.exports()) node->env.insert(var); } } @@ -265,7 +265,7 @@ void var_stack_t::pop() { for (const auto &entry_pair : old_top->env) { const env_var_t &var = entry_pair.second; - if (var.exportv) { + if (var.exports()) { this->mark_changed_exported(); break; } @@ -323,8 +323,6 @@ static bool is_read_only(const wcstring &key) { return env_read_only.find(key) != env_read_only.end(); } -bool env_var_t::read_only() const { return is_read_only(name); } - /// Table of variables whose value is dynamically calculated, such as umask, status, etc. static const_string_set_t env_electric; @@ -1090,7 +1088,7 @@ static int env_set_internal(const wcstring &key, env_mode_flags_t var_mode, wcst var_table_t::const_iterator result = preexisting_node->env.find(key); assert(result != preexisting_node->env.end()); const env_var_t &var = result->second; - if (var.exportv) { + if (var.exports()) { preexisting_entry_exportv = true; has_changed_new = true; } @@ -1137,7 +1135,7 @@ static int env_set_internal(const wcstring &key, env_mode_flags_t var_mode, wcst // Set the entry in the node. Note that operator[] accesses the existing entry, or // creates a new one. env_var_t &var = node->env[key]; - if (var.exportv) { + if (var.exports()) { // This variable already existed, and was exported. has_changed_new = true; } @@ -1146,11 +1144,11 @@ static int env_set_internal(const wcstring &key, env_mode_flags_t var_mode, wcst if (var_mode & ENV_EXPORT) { // The new variable is exported. - var.exportv = true; + var.set_exports(true); node->exportv = true; has_changed_new = true; } else { - var.exportv = false; + var.set_exports(false); // Set the node's exported when it changes something about exports // (also when it redefines a variable to not be exported). node->exportv = has_changed_old != has_changed_new; @@ -1201,7 +1199,7 @@ static bool try_remove(env_node_t *n, const wchar_t *key, int var_mode) { var_table_t::iterator result = n->env.find(key); if (result != n->env.end()) { - if (result->second.exportv) { + if (result->second.exports()) { vars_stack().mark_changed_exported(); } n->env.erase(result); @@ -1272,7 +1270,7 @@ const wcstring_list_t &env_var_t::as_list() const { return vals; } wcstring env_var_t::as_string(void) const { if (this->vals.empty()) return wcstring(ENV_NULL); - wchar_t sep = variable_is_colon_delimited_var(this->name) ? L':' : ARRAY_SEP; + wchar_t sep = (flags & flag_colon_delimit) ? L':' : ARRAY_SEP; auto it = this->vals.cbegin(); wcstring result(*it); while (++it != vals.end()) { @@ -1286,6 +1284,14 @@ void env_var_t::to_list(wcstring_list_t &out) const { out = vals; } +env_var_t::env_var_flags_t env_var_t::flags_for(const wchar_t *name) { + env_var_flags_t result = 0; + wcstring tmp(name); // todo: eliminate + if (is_read_only(tmp)) result |= flag_read_only; + if (variable_is_colon_delimited_var(tmp)) result |= flag_colon_delimit; + return result; +} + maybe_t env_get(const wcstring &key, env_mode_flags_t mode) { const bool has_scope = mode & (ENV_LOCAL | ENV_GLOBAL | ENV_UNIVERSAL); const bool search_local = !has_scope || (mode & ENV_LOCAL); @@ -1334,7 +1340,7 @@ maybe_t env_get(const wcstring &key, env_mode_flags_t mode) { var_table_t::const_iterator result = env->env.find(key); if (result != env->env.end()) { const env_var_t &var = result->second; - if (var.exportv ? search_exported : search_unexported) { + if (var.exports() ? search_exported : search_unexported) { return var; } } @@ -1387,7 +1393,7 @@ static void add_key_to_string_set(const var_table_t &envs, std::set *s for (iter = envs.begin(); iter != envs.end(); ++iter) { const env_var_t &var = iter->second; - if ((var.exportv && show_exported) || (!var.exportv && show_unexported)) { + if ((var.exports() && show_exported) || (!var.exports() && show_unexported)) { // Insert this key. str_set->insert(iter->first); } @@ -1454,7 +1460,7 @@ static void get_exported(const env_node_t *n, var_table_t &h) { const wcstring &key = iter->first; const env_var_t var = iter->second; - if (var.exportv) { + if (var.exports()) { // Export the variable. Don't use std::map::insert here, since we need to overwrite // existing values from previous scopes. h[key] = var; @@ -1570,7 +1576,7 @@ maybe_t env_vars_snapshot_t::get(const wcstring &key) const { } auto iter = vars.find(key); if (iter == vars.end()) return none(); - return env_var_t(iter->second); + return iter->second; } const wchar_t *const env_vars_snapshot_t::highlighting_keys[] = {L"PATH", L"CDPATH", diff --git a/src/env.h b/src/env.h index 88e8949f7..543b4dd11 100644 --- a/src/env.h +++ b/src/env.h @@ -70,28 +70,34 @@ void misc_init(); class env_var_t { private: - wcstring name; // name of the var + using env_var_flags_t = uint8_t; wcstring_list_t vals; // list of values assigned to the var + env_var_flags_t flags; public: - bool exportv = false; // whether the variable should be exported + enum { + flag_export = 1 << 0, // whether the variable is exported + flag_colon_delimit = 1 << 1, // whether the variable is colon delimited + flag_read_only = 1 << 2 // whether the variable is read only + }; // Constructors. env_var_t(const env_var_t &) = default; env_var_t(env_var_t &&) = default; - env_var_t(wcstring name, wcstring_list_t vals) : name(std::move(name)), vals(std::move(vals)) {} + env_var_t(wcstring_list_t vals, env_var_flags_t flags) : vals(std::move(vals)), flags(flags) {} + env_var_t(wcstring val, env_var_flags_t flags) + : env_var_t(wcstring_list_t{std::move(val)}, flags) {} - env_var_t(wcstring name, wcstring val) - : name(std::move(name)), - vals({ - std::move(val), - }), - exportv(false) {} + // Constructors that infer the flags from a name. + env_var_t(const wchar_t *name, wcstring_list_t vals) + : env_var_t(std::move(vals), flags_for(name)) {} + env_var_t(const wchar_t *name, wcstring val) : env_var_t(std::move(val), flags_for(name)) {} env_var_t() = default; bool empty() const { return vals.empty() || (vals.size() == 1 && vals[0].empty()); }; - bool read_only() const; + bool read_only() const { return flags & flag_read_only; } + bool exports() const { return flags & flag_export; } wcstring as_string() const; void to_list(wcstring_list_t &out) const; @@ -99,6 +105,16 @@ class env_var_t { void set_vals(wcstring_list_t v) { vals = std::move(v); } + void set_exports(bool exportv) { + if (exportv) { + flags |= flag_export; + } else { + flags &= ~flag_export; + } + } + + static env_var_flags_t flags_for(const wchar_t *name); + env_var_t &operator=(const env_var_t &var) = default; env_var_t &operator=(env_var_t &&) = default; diff --git a/src/env_universal_common.cpp b/src/env_universal_common.cpp index 42703146c..1bce23c75 100644 --- a/src/env_universal_common.cpp +++ b/src/env_universal_common.cpp @@ -268,7 +268,7 @@ bool env_universal_t::get_export(const wcstring &name) const { bool result = false; var_table_t::const_iterator where = vars.find(name); if (where != vars.end()) { - result = where->second.exportv; + result = where->second.exports(); } return result; } @@ -282,9 +282,9 @@ void env_universal_t::set_internal(const wcstring &key, wcstring_list_t vals, bo } env_var_t &entry = vars[key]; - if (entry.exportv != exportv || entry.as_list() != vals) { + if (entry.exports() != exportv || entry.as_list() != vals) { entry.set_vals(std::move(vals)); - entry.exportv = exportv; + entry.set_exports(exportv); // If we are overwriting, then this is now modified. if (overwrite) { @@ -319,7 +319,7 @@ wcstring_list_t env_universal_t::get_names(bool show_exported, bool show_unexpor for (iter = vars.begin(); iter != vars.end(); ++iter) { const wcstring &key = iter->first; const env_var_t &var = iter->second; - if ((var.exportv && show_exported) || (!var.exportv && show_unexported)) { + if ((var.exports() && show_exported) || (!var.exports() && show_unexported)) { result.push_back(key); } } @@ -357,11 +357,11 @@ void env_universal_t::generate_callbacks(const var_table_t &new_vars, // See if the value has changed. const env_var_t &new_entry = iter->second; var_table_t::const_iterator existing = this->vars.find(key); - if (existing == this->vars.end() || existing->second.exportv != new_entry.exportv || + if (existing == this->vars.end() || existing->second.exports() != new_entry.exports() || existing->second != new_entry) { // Value has changed. - callbacks.push_back( - callback_data_t(new_entry.exportv ? SET_EXPORT : SET, key, new_entry.as_string())); + callbacks.push_back(callback_data_t(new_entry.exports() ? SET_EXPORT : SET, key, + new_entry.as_string())); } } } @@ -448,7 +448,7 @@ bool env_universal_t::write_to_fd(int fd, const wcstring &path) { // variable; soldier on. const wcstring &key = iter->first; const env_var_t &var = iter->second; - append_file_entry(var.exportv ? SET_EXPORT : SET, key, var.as_string(), &contents, + append_file_entry(var.exports() ? SET_EXPORT : SET, key, var.as_string(), &contents, &storage); // Go to next. @@ -830,7 +830,7 @@ void env_universal_t::parse_message_internal(const wcstring &msgstr, var_table_t wcstring val; if (unescape_string(tmp + 1, &val, 0)) { env_var_t &entry = (*vars)[key]; - entry.exportv = exportv; + entry.set_exports(exportv); entry.set_vals(decode_serialized(val)); } } else { diff --git a/src/fish_tests.cpp b/src/fish_tests.cpp index ceec2ef2b..6d86c07d6 100644 --- a/src/fish_tests.cpp +++ b/src/fish_tests.cpp @@ -2634,7 +2634,7 @@ static void test_universal() { if (j == 0) { expected_val = none(); } else { - expected_val = env_var_t(L"", format_string(L"val_%d_%d", i, j)); + expected_val = env_var_t(format_string(L"val_%d_%d", i, j), 0); } const maybe_t var = uvars.get(key); if (j == 0) assert(!expected_val); diff --git a/src/highlight.cpp b/src/highlight.cpp index e419230ad..fa10aa516 100644 --- a/src/highlight.cpp +++ b/src/highlight.cpp @@ -224,11 +224,9 @@ static bool is_potential_cd_path(const wcstring &path, const wcstring &working_d } else { // Get the CDPATH. auto cdpath = env_get(L"CDPATH"); - if (cdpath.missing_or_empty()) cdpath = env_var_t(L"CDPATH", L"."); + std::vector pathsv = + cdpath.missing_or_empty() ? wcstring_list_t{L"."} : cdpath->as_list(); - // Tokenize it into directories. - std::vector pathsv; - cdpath->to_list(pathsv); for (auto next_path : pathsv) { if (next_path.empty()) next_path = L"."; // Ensure that we use the working directory for relative cdpaths like ".". @@ -355,7 +353,7 @@ bool autosuggest_validate_from_history(const history_item_t &item, string_prefixes_string(cd_dir, L"--help") || string_prefixes_string(cd_dir, L"-h"); if (!is_help) { wcstring path; - env_var_t dir_var(L"n/a", cd_dir); + env_var_t dir_var(cd_dir, 0); bool can_cd = path_get_cdpath(dir_var, &path, working_directory.c_str(), vars); if (can_cd && !paths_are_same_file(working_directory, path)) { suggestionOK = true;