diff options
| -rw-r--r-- | src/complete.cpp | 49 | ||||
| -rw-r--r-- | src/expand.cpp | 41 | ||||
| -rw-r--r-- | src/expand.h | 8 | ||||
| -rw-r--r-- | src/wildcard.cpp | 8 | ||||
| -rw-r--r-- | tests/test6.in | 34 | ||||
| -rw-r--r-- | tests/test6.out | 2 |
6 files changed, 74 insertions, 68 deletions
diff --git a/src/complete.cpp b/src/complete.cpp index 8af2c822..5c817b8d 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -851,8 +851,7 @@ void completer_t::complete_cmd(const wcstring &str_cmd, bool use_function, bool if (use_command) { - - if (expand_string(str_cmd, &this->completions, EXPAND_FOR_COMPLETIONS | EXECUTABLES_ONLY | this->expand_flags(), NULL) != EXPAND_ERROR) + if (expand_string(str_cmd, &this->completions, EXPAND_SPECIAL_FOR_COMMAND | EXPAND_FOR_COMPLETIONS | EXECUTABLES_ONLY | this->expand_flags(), NULL) != EXPAND_ERROR) { if (this->wants_descriptions()) { @@ -869,52 +868,8 @@ void completer_t::complete_cmd(const wcstring &str_cmd, bool use_function, bool } if (str_cmd.find(L'/') == wcstring::npos && str_cmd.at(0) != L'~') { - if (use_command) - { - - const env_var_t path = this->vars.get(L"PATH"); - if (!path.missing()) - { - wcstring base_path; - wcstokenizer tokenizer(path, ARRAY_SEP_STR); - while (tokenizer.next(base_path)) - { - if (base_path.empty()) - continue; - - /* Make sure the base path ends with a slash */ - if (base_path.at(base_path.size() - 1) != L'/') - base_path.push_back(L'/'); - - wcstring nxt_completion = base_path; - nxt_completion.append(str_cmd); - - size_t prev_count = this->completions.size(); - expand_flags_t expand_flags = EXPAND_FOR_COMPLETIONS | EXECUTABLES_ONLY | EXPAND_NO_FUZZY_DIRECTORIES | this->expand_flags(); - if (expand_string(nxt_completion, - &this->completions, - expand_flags, NULL) != EXPAND_ERROR) - { - /* For all new completions, if COMPLETE_REPLACES_TOKEN is set, then use only the last path component */ - for (size_t i=prev_count; i< this->completions.size(); i++) - { - completion_t &c = this->completions.at(i); - if (c.flags & COMPLETE_REPLACES_TOKEN) - { - - c.completion.erase(0, base_path.size()); - } - } - } - } - if (this->wants_descriptions()) - this->complete_cmd_desc(str_cmd); - } - } - if (use_function) { - //function_get_names( &possible_comp, cmd[0] == L'_' ); wcstring_list_t names = function_get_names(str_cmd.at(0) == L'_'); for (size_t i=0; i < names.size(); i++) { @@ -1361,7 +1316,7 @@ void completer_t::complete_param_expand(const wcstring &str, bool do_file, bool if (handle_as_special_cd && do_file) { - flags |= DIRECTORIES_ONLY | EXPAND_SPECIAL_CD | EXPAND_NO_DESCRIPTIONS; + flags |= DIRECTORIES_ONLY | EXPAND_SPECIAL_FOR_CD | EXPAND_NO_DESCRIPTIONS; } /* Squelch file descriptions per issue 254 */ diff --git a/src/expand.cpp b/src/expand.cpp index de67ffa8..cb68eb98 100644 --- a/src/expand.cpp +++ b/src/expand.cpp @@ -1800,36 +1800,47 @@ static expand_error_t expand_stage_wildcards(const wcstring &input, std::vector< const wcstring working_dir = env_get_pwd_slash(); wcstring_list_t effective_working_dirs; - if (! (flags & EXPAND_SPECIAL_CD)) + bool for_cd = !!(flags & EXPAND_SPECIAL_FOR_CD); + bool for_command = !!(flags & EXPAND_SPECIAL_FOR_COMMAND); + if (!for_cd && !for_command) { /* Common case */ effective_working_dirs.push_back(working_dir); } else { - /* Ignore the CDPATH if we start with ./ or / */ - if (string_prefixes_string(L"./", path_to_expand)) - { - effective_working_dirs.push_back(working_dir); - } - else if (string_prefixes_string(L"/", path_to_expand)) + /* Either EXPAND_SPECIAL_FOR_COMMAND or EXPAND_SPECIAL_FOR_CD. We can handle these 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). + 2. PATH is only "one level," while CDPATH is multiple levels. That is, input like 'foo/bar' should resolve + against CDPATH, but not PATH. + + In either case, we ignore the path if we start with ./ or /. + Also ignore it if we are doing command completion and we contain a slash, per IEEE 1003.1, chapter 8 under PATH + */ + if (string_prefixes_string(L"./", path_to_expand) || + string_prefixes_string(L"/", path_to_expand) || + (for_command && path_to_expand.find(L'/') != wcstring::npos)) { effective_working_dirs.push_back(working_dir); } else { - /* Get the CDPATH and cwd. Perhaps these should be passed in. */ - env_var_t cdpath = env_get_string(L"CDPATH"); - if (cdpath.missing_or_empty()) - cdpath = L"."; + /* Get the PATH/CDPATH and cwd. Perhaps these should be passed in. + An empty CDPATH implies just the current directory, while an empty PATH is left empty. + */ + env_var_t paths = env_get_string(for_cd ? L"CDPATH" : L"PATH"); + if (paths.missing_or_empty()) + paths = for_cd ? L"." : L""; /* Tokenize it into directories */ - wcstokenizer tokenizer(cdpath, ARRAY_SEP_STR); - wcstring next_cd_path; - while (tokenizer.next(next_cd_path)) + wcstokenizer tokenizer(paths, ARRAY_SEP_STR); + wcstring next_path; + while (tokenizer.next(next_path)) { /* Ensure that we use the working directory for relative cdpaths like "." */ - effective_working_dirs.push_back(path_apply_working_directory(next_cd_path, working_dir)); + effective_working_dirs.push_back(path_apply_working_directory(next_path, working_dir)); + } } } diff --git a/src/expand.h b/src/expand.h index 6ae91cca..fafbcd2a 100644 --- a/src/expand.h +++ b/src/expand.h @@ -52,8 +52,12 @@ enum // Disallow directory abbreviations like /u/l/b for /usr/local/bin. Only // applicable if EXPAND_FUZZY_MATCH is set. EXPAND_NO_FUZZY_DIRECTORIES = 1 << 10, - // Do expansions specifically to support cd (CDPATH, etc). - EXPAND_SPECIAL_CD = 1 << 11 + // Do expansions specifically to support cd + // This means using CDPATH as a list of potential working directories + EXPAND_SPECIAL_FOR_CD = 1 << 11, + // Do expansions specifically to support external command completions. + // This means using PATH as a list of potential working directories + EXPAND_SPECIAL_FOR_COMMAND = 1 << 12 }; typedef int expand_flags_t; diff --git a/src/wildcard.cpp b/src/wildcard.cpp index db211818..202b836d 100644 --- a/src/wildcard.cpp +++ b/src/wildcard.cpp @@ -789,8 +789,8 @@ class wildcard_expander_t c.prepend_token_prefix(this->original_base); } - /* Hack. Implement EXPAND_SPECIAL_CD by descending the deepest unique hierarchy we can, and then appending any components to each new result. */ - if (flags & EXPAND_SPECIAL_CD) + /* Hack. Implement EXPAND_SPECIAL_FOR_CD by descending the deepest unique hierarchy we can, and then appending any components to each new result. */ + if (flags & EXPAND_SPECIAL_FOR_CD) { wcstring unique_hierarchy = this->descend_unique_hierarchy(abs_path); if (! unique_hierarchy.empty()) @@ -1138,8 +1138,8 @@ int wildcard_expand_string(const wcstring &wc, const wcstring &working_directory /* Fuzzy matching only if we're doing completions */ assert((flags & (EXPAND_FUZZY_MATCH | EXPAND_FOR_COMPLETIONS)) != EXPAND_FUZZY_MATCH); - /* EXPAND_SPECIAL_CD requires DIRECTORIES_ONLY and EXPAND_FOR_COMPLETIONS and EXPAND_NO_DESCRIPTIONS */ - assert(!(flags & EXPAND_SPECIAL_CD) || + /* EXPAND_SPECIAL_FOR_CD requires DIRECTORIES_ONLY and EXPAND_FOR_COMPLETIONS and EXPAND_NO_DESCRIPTIONS */ + assert(!(flags & EXPAND_SPECIAL_FOR_CD) || ((flags & DIRECTORIES_ONLY) && (flags & EXPAND_FOR_COMPLETIONS) && (flags & EXPAND_NO_DESCRIPTIONS))); /* Hackish fix for 1631. We are about to call c_str(), which will produce a string truncated at any embedded nulls. We could fix this by passing around the size, etc. However embedded nulls are never allowed in a filename, so we just check for them and return 0 (no matches) if there is an embedded null. */ diff --git a/tests/test6.in b/tests/test6.in index 252eb606..0d6ef487 100644 --- a/tests/test6.in +++ b/tests/test6.in @@ -72,3 +72,37 @@ if begin; rm -rf test6.tmp.dir; and mkdir test6.tmp.dir; end else echo "error: could not create temp environment" >&2 end + +# Test command expansion with parened PATHs (#952) +begin + set -l parened_path $PWD/'test6.tmp2.(paren).dir' + set -l parened_subpath $parened_path/subdir + if not begin + rm -rf $parened_path + and mkdir $parened_path + and mkdir $parened_subpath + and ln -s /bin/ls $parened_path/'__test6_(paren)_command' + and ln -s /bin/ls $parened_subpath/'__test6_subdir_(paren)_command' + end + echo "error: could not create command expansion temp environment" >&2 + end + + # Verify that we can expand commands when PATH has parens + set -l PATH $parened_path $PATH + set -l completed (complete -C__test6_ | cut -f 1 -d \t) + if test "$completed" = '__test6_(paren)_command' + echo "Command completion with parened PATHs test passed" + else + echo "Command completion with parened PATHs test failed. Expected __test6_(paren)_command, got $completed" >&2 + end + + # Verify that commands with intermediate slashes do NOT expand with respect to PATH + set -l completed (complete -Csubdir/__test6_subdir) + if test -z "$completed" + echo "Command completion with intermediate slashes passed" + else + echo "Command completion with intermediate slashes: should output nothing, instead got $completed" >&2 + end + + rm -rf $parened_path +end diff --git a/tests/test6.out b/tests/test6.out index 268b05ea..ca6e6906 100644 --- a/tests/test6.out +++ b/tests/test6.out @@ -33,3 +33,5 @@ CCCC: implicit cd complete works no implicit cd complete after 'command' PATH does not cause incorrect implicit cd +Command completion with parened PATHs test passed +Command completion with intermediate slashes passed |