Migrate PATH-completion logic from complete.cpp to expand.cpp

Prior to this fix, when completing a command that doesn't have a /, we
would prepend each component of PATH and then expand the whole thing. So
any special characters in the PATH would be interpreted when performing
tab completion.

With this fix, we pull the PATH resolution out of complete.cpp and
migrate it to expand.cpp. This unifies nicely with the CDPATH resolution
already in that file. This requires introducing a new expand flag
EXPAND_SPECIAL_FOR_COMMAND, which is analogous to EXPAND_SPECIAL_CD
(which is renamed to EXPAND_SPECIAL_FOR_CD). This flag tells expand to
resolve paths against PATH instead of the working directory.

Fixes #952
This commit is contained in:
ridiculousfish 2016-04-07 15:24:52 -07:00 committed by Kurtis Rader
parent 36691df6fe
commit e395a0eb69
6 changed files with 74 additions and 68 deletions

View file

@ -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 */

View file

@ -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));
}
}
}

View file

@ -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;

View file

@ -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. */

View file

@ -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

View file

@ -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