empty CDPATH elements are equivalent to "."

In the process of fixing the issue I decided it didn't make sense to
have two, incompatible, ways of converting variable strings to arrays.
Especially since the one I'm removing does not return empty array elements.

Fixes #2106

CHANGELOG.md

@@ -9,6 +9,7 @@
 - The `string` command now supports a `repeat` subcommand with the obvious behavior (#3864).
 - The `functions --metadata --verbose` output now includes the function description (#597).
 - Completions for `helm` added (#3829).
+- Empty CDPATH elements are now equivalent to "." (#2106).
 
 ---
 

src/common.cpp

@@ -1919,22 +1919,6 @@ scoped_rwlock::~scoped_rwlock() {
     }
 }
 
-wcstokenizer::wcstokenizer(const wcstring &s, const wcstring &separator)
-    : buffer(), str(), state(), sep(separator) {
-    buffer = wcsdup(s.c_str());
-    str = buffer;
-    state = NULL;
-}
-
-bool wcstokenizer::next(wcstring &result) {
-    wchar_t *tmp = wcstok(str, sep.c_str(), &state);
-    str = NULL;
-    if (tmp) result = tmp;
-    return tmp != NULL;
-}
-
-wcstokenizer::~wcstokenizer() { free(buffer); }
-
 template <typename CharType_t>
 static CharType_t **make_null_terminated_array_helper(
     const std::vector<std::basic_string<CharType_t> > &argv) {

src/common.h

@@ -640,21 +640,6 @@ class scoped_push {
     }
 };
 
-/// Wrapper around wcstok.
-class wcstokenizer {
-    wchar_t *buffer, *str, *state;
-    const wcstring sep;
-
-    // No copying.
-    wcstokenizer &operator=(const wcstokenizer &);
-    wcstokenizer(const wcstokenizer &);
-
-   public:
-    wcstokenizer(const wcstring &s, const wcstring &separator);
-    bool next(wcstring &result);
-    ~wcstokenizer();
-};
-
 /// Appends a path component, with a / if necessary.
 void append_path_component(wcstring &path, const wcstring &component);
 

src/expand.cpp

@@ -1403,7 +1403,8 @@ static expand_error_t expand_stage_wildcards(const wcstring &input, std::vector<
             // mostly the same. There's the following differences:
             //
             // 1. An empty CDPATH should be treated as '.', but an empty PATH should be left empty
-            // (no commands can be found).
+            // (no commands can be found). Also, an empty element of CDPATH is treated as '.' for
+            // consistency with POSIX shells.
             //
             // 2. PATH is only "one level," while CDPATH is multiple levels. That is, input like
             // 'foo/bar' should resolve against CDPATH, but not PATH.
@@ -1423,9 +1424,14 @@ static expand_error_t expand_stage_wildcards(const wcstring &input, std::vector<
                 if (paths.missing_or_empty()) paths = for_cd ? L"." : L"";
 
                 // Tokenize it into directories.
-                wcstokenizer tokenizer(paths, ARRAY_SEP_STR);
+                std::vector<wcstring> pathsv;
-                wcstring next_path;
+                tokenize_variable_array(paths, pathsv);
-                while (tokenizer.next(next_path)) {
+
+                for (auto next_path : pathsv) {
+                    if (next_path.empty()) {
+                        if (!for_cd) continue;
+                        next_path = L".";
+                    }
                     // Ensure that we use the working directory for relative cdpaths like ".".
                     effective_working_dirs.push_back(
                         path_apply_working_directory(next_path, working_dir));
@@ -1574,29 +1580,29 @@ bool expand_abbreviation(const wcstring &src, wcstring *output) {
     if (src.empty()) return false;
 
     // Get the abbreviations. Return false if we have none.
-    env_var_t var = env_get_string(USER_ABBREVIATIONS_VARIABLE_NAME);
+    env_var_t abbrs = env_get_string(USER_ABBREVIATIONS_VARIABLE_NAME);
-    if (var.missing_or_empty()) return false;
+    if (abbrs.missing_or_empty()) return false;
 
     bool result = false;
-    wcstring line;
+    std::vector<wcstring> abbrsv;
-    wcstokenizer tokenizer(var, ARRAY_SEP_STR);
+    tokenize_variable_array(abbrs, abbrsv);
-    while (tokenizer.next(line)) {
+    for (auto abbr : abbrsv) {
-        // Line is expected to be of the form 'foo=bar' or 'foo bar'. Parse out the first = or
+        // Abbreviation is expected to be of the form 'foo=bar' or 'foo bar'. Parse out the first =
-        // space. Silently skip on failure (no equals, or equals at the end or beginning). Try to
+        // or space. Silently skip on failure (no equals, or equals at the end or beginning). Try to
         // avoid copying any strings until we are sure this is a match.
-        size_t equals_pos = line.find(L'=');
+        size_t equals_pos = abbr.find(L'=');
-        size_t space_pos = line.find(L' ');
+        size_t space_pos = abbr.find(L' ');
         size_t separator = mini(equals_pos, space_pos);
-        if (separator == wcstring::npos || separator == 0 || separator + 1 == line.size()) continue;
+        if (separator == wcstring::npos || separator == 0 || separator + 1 == abbr.size()) continue;
 
         // Find the character just past the end of the command. Walk backwards, skipping spaces.
         size_t cmd_end = separator;
-        while (cmd_end > 0 && iswspace(line.at(cmd_end - 1))) cmd_end--;
+        while (cmd_end > 0 && iswspace(abbr.at(cmd_end - 1))) cmd_end--;
 
         // See if this command matches.
-        if (line.compare(0, cmd_end, src) == 0) {
+        if (abbr.compare(0, cmd_end, src) == 0) {
             // Success. Set output to everything past the end of the string.
-            if (output != NULL) output->assign(line, separator + 1, wcstring::npos);
+            if (output != NULL) output->assign(abbr, separator + 1, wcstring::npos);
 
             result = true;
             break;

src/highlight.cpp

@@ -216,9 +216,10 @@ static bool is_potential_cd_path(const wcstring &path, const wcstring &working_d
         if (cdpath.missing_or_empty()) cdpath = L".";
 
         // Tokenize it into directories.
-        wcstokenizer tokenizer(cdpath, ARRAY_SEP_STR);
+        std::vector<wcstring> pathsv;
-        wcstring next_path;
+        tokenize_variable_array(cdpath, pathsv);
-        while (tokenizer.next(next_path)) {
+        for (auto next_path : pathsv) {
+            if (next_path.empty()) next_path = L".";
             // Ensure that we use the working directory for relative cdpaths like ".".
             directories.push_back(path_apply_working_directory(next_path, working_directory));
         }

src/path.cpp

@@ -58,21 +58,21 @@ static bool path_get_path_core(const wcstring &cmd, wcstring *out_path,
         }
     }
 
-    wcstring nxt_path;
+    std::vector<wcstring> pathsv;
-    wcstokenizer tokenizer(bin_path, ARRAY_SEP_STR);
+    tokenize_variable_array(bin_path, pathsv);
-    while (tokenizer.next(nxt_path)) {
+    for (auto next_path : pathsv) {
-        if (nxt_path.empty()) continue;
+        if (next_path.empty()) continue;
-        append_path_component(nxt_path, cmd);
+        append_path_component(next_path, cmd);
-        if (waccess(nxt_path, X_OK) == 0) {
+        if (waccess(next_path, X_OK) == 0) {
             struct stat buff;
-            if (wstat(nxt_path, &buff) == -1) {
+            if (wstat(next_path, &buff) == -1) {
                 if (errno != EACCES) {
                     wperror(L"stat");
                 }
                 continue;
             }
             if (S_ISREG(buff.st_mode)) {
-                if (out_path) *out_path = std::move(nxt_path);
+                if (out_path) *out_path = std::move(next_path);
                 return true;
             }
             err = EACCES;
@@ -85,7 +85,7 @@ static bool path_get_path_core(const wcstring &cmd, wcstring *out_path,
                     break;
                 }
                 default: {
-                    debug(1, MISSING_COMMAND_ERR_MSG, nxt_path.c_str());
+                    debug(1, MISSING_COMMAND_ERR_MSG, next_path.c_str());
                     wperror(L"access");
                     break;
                 }
@@ -128,24 +128,22 @@ bool path_get_cdpath(const wcstring &dir, wcstring *out, const wchar_t *wd,
         paths.push_back(path);
     } else {
         // Respect CDPATH.
-        env_var_t path = env_vars.get(L"CDPATH");
+        env_var_t cdpaths = env_vars.get(L"CDPATH");
-        if (path.missing_or_empty()) path = L".";  // we'll change this to the wd if we have one
+        if (cdpaths.missing_or_empty()) cdpaths = L".";
 
-        wcstring nxt_path;
+        std::vector<wcstring> cdpathsv;
-        wcstokenizer tokenizer(path, ARRAY_SEP_STR);
+        tokenize_variable_array(cdpaths, cdpathsv);
-        while (tokenizer.next(nxt_path)) {
+        for (auto next_path : cdpathsv) {
-            if (nxt_path == L"." && wd != NULL) {
+            if (next_path.empty()) next_path = L".";
-                // nxt_path is just '.', and we have a working directory, so use the wd instead.
+            if (next_path == L"." && wd != NULL) {
-                // TODO: if nxt_path starts with ./ we need to replace the . with the wd.
+                // next_path is just '.', and we have a working directory, so use the wd instead.
-                nxt_path = wd;
+                // TODO: if next_path starts with ./ we need to replace the . with the wd.
+                next_path = wd;
             }
-            expand_tilde(nxt_path);
+            expand_tilde(next_path);
+            if (next_path.empty()) continue;
 
-            // debug( 2, L"woot %ls\n", expanded_path.c_str() );
+            wcstring whole_path = next_path;
-
-            if (nxt_path.empty()) continue;
-
-            wcstring whole_path = nxt_path;
             append_path_component(whole_path, dir);
             paths.push_back(whole_path);
         }