stop subclassing env_var_t from wcstring

This is the first step to implementing issue #4200 is to stop subclassing
env_var_t from wcstring. Not too surprisingly doing this identified
several places that were incorrectly treating env_var_t and wcstring as
interchangeable types. I'm not talking about those places that passed
an env_var_t instance to a function that takes a wcstring. I'm talking
about doing things like assigning the former to the latter type, relying
on the implicit conversion, and thus losing information.

We also rename `env_get_string()` to `env_get()` for symmetry with
`env_set()` and to make it clear the function does not return a string.

Author: krader1961

src/autoload.cpp

@@ -69,7 +69,7 @@ int autoload_t::load(const wcstring &cmd, bool reload) {
     CHECK_BLOCK(0);
     ASSERT_IS_MAIN_THREAD();
 
-    env_var_t path_var = env_get_string(env_var_name);
+    env_var_t path_var = env_get(env_var_name);
 
     // Do we know where to look?
     if (path_var.empty()) return 0;
@@ -79,7 +79,7 @@ int autoload_t::load(const wcstring &cmd, bool reload) {
     if (path_var != this->last_path) {
         this->last_path = path_var;
         this->last_path_tokenized.clear();
-        tokenize_variable_array(this->last_path, this->last_path_tokenized);
+        this->last_path.to_list(this->last_path_tokenized);
 
         scoped_lock locker(lock);
         this->evict_all_nodes();
@@ -115,7 +115,7 @@ bool autoload_t::can_load(const wcstring &cmd, const env_vars_snapshot_t &vars)
     if (path_var.missing_or_empty()) return false;
 
     std::vector<wcstring> path_list;
-    tokenize_variable_array(path_var, path_list);
+    path_var.to_list(path_list);
     return this->locate_file_and_maybe_load_it(cmd, false, false, path_list);
 }
 

src/autoload.h

@@ -8,6 +8,7 @@
 #include <set>
 
 #include "common.h"
+#include "env.h"
 #include "lru.h"
 
 /// Record of an attempt to access a file.
@@ -50,7 +51,7 @@ class autoload_t : public lru_cache_t<autoload_t, autoload_function_t> {
     /// The environment variable name.
     const wcstring env_var_name;
     /// The path from which we most recently autoloaded.
-    wcstring last_path;
+    env_var_t last_path;
     /// the most reecently autoloaded path, tokenized (split on separators).
     wcstring_list_t last_path_tokenized;
     /// A table containing all the files that are currently being loaded.

src/builtin_cd.cpp

@@ -39,7 +39,7 @@ int builtin_cd(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
     if (argv[optind]) {
         dir_in = env_var_t(argv[optind]);
     } else {
-        dir_in = env_get_string(L"HOME");
+        dir_in = env_get(L"HOME");
         if (dir_in.missing_or_empty()) {
             streams.err.append_format(_(L"%ls: Could not find home directory\n"), cmd);
             return STATUS_CMD_ERROR;

src/builtin_functions.cpp

@@ -193,7 +193,7 @@ static wcstring functions_def(const wcstring &name) {
          it != end; ++it) {
         wcstring_list_t lst;
         if (!it->second.missing()) {
-            tokenize_variable_array(it->second, lst);
+            it->second.to_list(lst);
         }
 
         // This forced tab is crummy, but we don't know what indentation style the function uses.

src/builtin_read.cpp

@@ -427,9 +427,9 @@ int builtin_read(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
     }
 
     if (!opts.have_delimiter) {
-        env_var_t ifs = env_get_string(L"IFS");
+        env_var_t ifs = env_get(L"IFS");
         if (!ifs.missing_or_empty()) {
-            opts.delimiter = ifs;
+            opts.delimiter = ifs.as_string();
         }
     }
     if (opts.delimiter.empty()) {

src/builtin_set.cpp

@@ -212,7 +212,7 @@ static int validate_cmd_opts(const wchar_t *cmd, set_cmd_opts_t &opts,  //!OCLIN
 static int check_global_scope_exists(const wchar_t *cmd, set_cmd_opts_t &opts, const wchar_t *dest,
                                      io_streams_t &streams) {
     if (opts.universal) {
-        env_var_t global_dest = env_get_string(dest, ENV_GLOBAL);
+        env_var_t global_dest = env_get(dest, ENV_GLOBAL);
         if (!global_dest.missing()) {
             streams.err.append_format(
                 _(L"%ls: Warning: universal scope selected, but a global variable '%ls' exists.\n"),
@@ -239,9 +239,8 @@ static int my_env_path_setup(const wchar_t *cmd, const wchar_t *key,  //!OCLINT(
     // not the (missing) local value. Also don't bother to complain about relative paths, which
     // don't start with /.
     wcstring_list_t existing_values;
-    const env_var_t existing_variable = env_get_string(key, ENV_DEFAULT);
-    if (!existing_variable.missing_or_empty())
-        tokenize_variable_array(existing_variable, existing_values);
+    const env_var_t existing_variable = env_get(key, ENV_DEFAULT);
+    if (!existing_variable.missing_or_empty()) existing_variable.to_list(existing_values);
 
     for (size_t i = 0; i < list.size(); i++) {
         const wcstring &dir = list.at(i);
@@ -346,9 +345,9 @@ static int parse_index(std::vector<long> &indexes, wchar_t *src, int scope, io_s
     *p = L'\0';        // split the var name from the indexes/slices
     p++;
 
-    env_var_t var_str = env_get_string(src, scope);
+    env_var_t var_str = env_get(src, scope);
     wcstring_list_t var;
-    if (!var_str.missing()) tokenize_variable_array(var_str, var);
+    if (!var_str.missing()) var_str.to_list(var);
 
     int count = 0;
 
@@ -457,15 +456,16 @@ static int builtin_set_list(const wchar_t *cmd, set_cmd_opts_t &opts, int argc,
         streams.out.append(e_key);
 
         if (!names_only) {
-            env_var_t value = env_get_string(key, compute_scope(opts));
-            if (!value.missing()) {
+            env_var_t var = env_get(key, compute_scope(opts));
+            if (!var.missing()) {
                 bool shorten = false;
-                if (opts.shorten_ok && value.length() > 64) {
+                wcstring val = var.as_string();
+                if (opts.shorten_ok && val.length() > 64) {
                     shorten = true;
-                    value.resize(60);
+                    val.resize(60);
                 }
 
-                wcstring e_value = expand_escape_variable(value);
+                wcstring e_value = expand_escape_variable(val);
                 streams.out.append(L" ");
                 streams.out.append(e_value);
 
@@ -500,8 +500,8 @@ static int builtin_set_query(const wchar_t *cmd, set_cmd_opts_t &opts, int argc,
 
         if (idx_count) {
             wcstring_list_t result;
-            env_var_t dest_str = env_get_string(dest, scope);
-            if (!dest_str.missing()) tokenize_variable_array(dest_str, result);
+            env_var_t dest_str = env_get(dest, scope);
+            if (!dest_str.missing()) dest_str.to_list(result);
 
             for (auto idx : indexes) {
                 if (idx < 1 || (size_t)idx > result.size()) retval++;
@@ -538,12 +538,12 @@ static void show_scope(const wchar_t *var_name, int scope, io_streams_t &streams
     }
 
     if (env_exist(var_name, scope)) {
-        const env_var_t evar = env_get_string(var_name, scope | ENV_EXPORT | ENV_USER);
+        const env_var_t evar = env_get(var_name, scope | ENV_EXPORT | ENV_USER);
         const wchar_t *exportv = evar.missing() ? _(L"unexported") : _(L"exported");
 
-        const env_var_t var = env_get_string(var_name, scope | ENV_USER);
+        const env_var_t var = env_get(var_name, scope | ENV_USER);
         wcstring_list_t result;
-        if (!var.empty()) tokenize_variable_array(var, result);
+        if (!var.empty()) var.to_list(result);
 
         streams.out.append_format(_(L"$%ls: set in %ls scope, %ls, with %d elements\n"), var_name,
                                   scope_name, exportv, result.size());
@@ -632,10 +632,10 @@ static int builtin_set_erase(const wchar_t *cmd, set_cmd_opts_t &opts, int argc,
     if (idx_count == 0) {  // unset the var
         retval = env_remove(dest, scope);
     } else {  // remove just the specified indexes of the var
-        const env_var_t dest_var = env_get_string(dest, scope);
+        const env_var_t dest_var = env_get(dest, scope);
         if (dest_var.missing()) return STATUS_CMD_ERROR;
         wcstring_list_t result;
-        tokenize_variable_array(dest_var, result);
+        dest_var.to_list(result);
         erase_values(result, indexes);
         retval = my_env_set(cmd, dest, result, scope, streams);
     }
@@ -658,9 +658,9 @@ static int set_var_array(const wchar_t *cmd, set_cmd_opts_t &opts, const wchar_t
             for (int i = 0; i < argc; i++) new_values.push_back(argv[i]);
         }
 
-        env_var_t var_str = env_get_string(varname, scope);
+        env_var_t var_str = env_get(varname, scope);
         wcstring_list_t var_array;
-        if (!var_str.missing()) tokenize_variable_array(var_str, var_array);
+        if (!var_str.missing()) var_str.to_list(var_array);
         new_values.insert(new_values.end(), var_array.begin(), var_array.end());
 
         if (opts.append) {
@@ -691,8 +691,8 @@ static int set_var_slices(const wchar_t *cmd, set_cmd_opts_t &opts, const wchar_
     }
 
     int scope = compute_scope(opts);  // calculate the variable scope based on the provided options
-    const env_var_t var_str = env_get_string(varname, scope);
-    if (!var_str.missing()) tokenize_variable_array(var_str, new_values);
+    const env_var_t var_str = env_get(varname, scope);
+    if (!var_str.missing()) var_str.to_list(new_values);
 
     // Slice indexes have been calculated, do the actual work.
     wcstring_list_t result;

src/common.cpp

@@ -1554,8 +1554,8 @@ static void validate_new_termsize(struct winsize *new_termsize) {
         }
 #endif
         // Fallback to the environment vars.
-        env_var_t col_var = env_get_string(L"COLUMNS");
-        env_var_t row_var = env_get_string(L"LINES");
+        env_var_t col_var = env_get(L"COLUMNS");
+        env_var_t row_var = env_get(L"LINES");
         if (!col_var.missing_or_empty() && !row_var.missing_or_empty()) {
             // Both vars have to have valid values.
             int col = fish_wcstoi(col_var.c_str());
@@ -1582,11 +1582,11 @@ static void validate_new_termsize(struct winsize *new_termsize) {
 /// Export the new terminal size as env vars and to the kernel if possible.
 static void export_new_termsize(struct winsize *new_termsize) {
     wchar_t buf[64];
-    env_var_t cols = env_get_string(L"COLUMNS", ENV_EXPORT);
+    env_var_t cols = env_get(L"COLUMNS", ENV_EXPORT);
     swprintf(buf, 64, L"%d", (int)new_termsize->ws_col);
     env_set(L"COLUMNS", buf, ENV_GLOBAL | (cols.missing_or_empty() ? 0 : ENV_EXPORT));
 
-    env_var_t lines = env_get_string(L"LINES", ENV_EXPORT);
+    env_var_t lines = env_get(L"LINES", ENV_EXPORT);
     swprintf(buf, 64, L"%d", (int)new_termsize->ws_row);
     env_set(L"LINES", buf, ENV_GLOBAL | (lines.missing_or_empty() ? 0 : ENV_EXPORT));
 

src/complete.cpp

@@ -1106,12 +1106,13 @@ bool completer_t::complete_variable(const wcstring &str, size_t start_offset) {
         wcstring desc;
         if (this->wants_descriptions()) {
             // Can't use this->vars here, it could be any variable.
-            env_var_t value_unescaped = env_get_string(env_name);
-            if (value_unescaped.missing()) continue;
+            env_var_t var = env_get(env_name);
+            if (var.missing()) continue;
 
-            wcstring value = expand_escape_variable(value_unescaped);
-            if (this->type() != COMPLETE_AUTOSUGGEST)
+            wcstring value = expand_escape_variable(var.as_string());
+            if (this->type() != COMPLETE_AUTOSUGGEST) {
                 desc = format_string(COMPLETE_VAR_DESC_VAL, value.c_str());
+            }
         }
 
         append_completion(&this->completions, comp, desc, flags, match);