From 41bcf77e25f07cc6748776290824b7b0bc01dbdd Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Tue, 10 Sep 2019 12:58:17 +0200 Subject: [PATCH 1/2] fix comment --- src/complete.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/complete.cpp b/src/complete.cpp index 4d78260fc..8829e4541 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -745,7 +745,7 @@ void completer_t::complete_abbr(const wcstring &cmd) { /// @param desc /// Description of the completion /// @param flags -/// The list into which the results will be inserted +/// The flags /// void completer_t::complete_from_args(const wcstring &str, const wcstring &args, const wcstring &desc, complete_flags_t flags) { From eae1683033ca3eddeece1ddf150e2fc7bc90fa45 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Tue, 10 Sep 2019 14:12:56 +0200 Subject: [PATCH 2/2] Completion: complete argument to last of a group of short options Consider a group of short options, like -xzPARAM, where x and z are options and z takes an argument. This commit enables completion of the argument to the last option (z), both within the same token (-xzP) or in the next one (-xz P). complete -C'-xz' will complete only parameters to z. complete -C'-xz ' will complete only parameters to z if z requires a parameter otherwise, it will also complete non-option parameters To do so this implements a heuristic to differentiate such strings from single long options. To detect whether our token contains some short options, we only require the first character after the dash (here x) to be an option. Previously, all characters had to be short options. The last option in our example is z. Everything after the last option is assumed to be a parameter to the last option. Assume there is also a single long option -x-foo, then complete -C'-x' will suggest both -x-foo and -xy. However, when the single option x requires an argument, this will not suggest -x-foo. However, I assume this will almost never happen in practise since completions very rarely mix short and single long options. Fixes #332 --- src/complete.cpp | 115 +++++++++++++++++++++---------------- tests/checks/complete.fish | 36 ++++++++++++ 2 files changed, 103 insertions(+), 48 deletions(-) diff --git a/src/complete.cpp b/src/complete.cpp index 8829e4541..28eed5b96 100644 --- a/src/complete.cpp +++ b/src/complete.cpp @@ -812,58 +812,43 @@ static const wchar_t *param_match2(const complete_entry_opt_t *e, const wchar_t // Short options are like -DNDEBUG. Long options are like --color=auto. So check for an equal // sign for long options. - if (e->type != option_type_short) { - if (optstr[cursor] != L'=') { - return NULL; - } - cursor += 1; + assert(e->type != option_type_short); + if (optstr[cursor] != L'=') { + return NULL; } + cursor += 1; return &optstr[cursor]; } -/// Tests whether a short option is a viable completion. arg_str will be like '-xzv', nextopt will -/// be a character like 'f' options will be the list of all options, used to validate the argument. -static bool short_ok(const wcstring &arg, const complete_entry_opt_t *entry, - const option_list_t &options) { - // Ensure it's a short option. - if (entry->type != option_type_short || entry->option.empty()) { - return false; +/// Parses a token of short options plus one optional parameter like +/// '-xzPARAM', where x and z are short options. +/// +/// Returns the position of the last option character (e.g. the position of z which is 2). +/// Everything after that is assumed to be part of the parameter. +/// Returns wcstring::npos if there is no valid short option. +size_t short_option_pos(const wcstring &arg, const option_list_t &options) { + if (arg.size() <= 1 || leading_dash_count(arg.c_str()) != 1) { + return wcstring::npos; } - const wchar_t nextopt = entry->option.at(0); - - // Empty strings are always 'OK'. - if (arg.empty()) { - return true; - } - - // The argument must start with exactly one dash. - if (leading_dash_count(arg.c_str()) != 1) { - return false; - } - - // Short option must not be already present. - if (arg.find(nextopt) != wcstring::npos) { - return false; - } - - // Verify that all characters in our combined short option list are present as short options in - // the options list. If we get a short option that can't be combined (NO_COMMON), then we stop. - bool result = true; - for (size_t i = 1; i < arg.size(); i++) { - wchar_t arg_char = arg.at(i); - const complete_entry_opt_t *match = NULL; - for (option_list_t::const_iterator iter = options.begin(); iter != options.end(); ++iter) { - if (iter->type == option_type_short && iter->option.at(0) == arg_char) { - match = &*iter; + for (size_t pos = 1; pos < arg.size(); pos++) { + wchar_t arg_char = arg.at(pos); + const complete_entry_opt_t *match = nullptr; + for (const complete_entry_opt_t &o : options) { + if (o.type == option_type_short && o.option.at(0) == arg_char) { + match = &o; break; } } - if (match == NULL || (match->result_mode.requires_param)) { - result = false; - break; + if (match == nullptr) { + // The first character after the dash is not a valid option. + if (pos == 1) return wcstring::npos; + return pos - 1; + } + if (match->result_mode.requires_param) { + return pos; } } - return result; + return arg.size() - 1; } /// Load command-specific completions for the specified command. @@ -954,13 +939,23 @@ bool completer_t::complete_param(const wcstring &cmd_orig, const wcstring &popt, // Now release the lock and test each option that we captured above. We have to do this outside // the lock because callouts (like the condition) may add or remove completions. See issue 2. for (const option_list_t &options : all_options) { + size_t short_opt_pos = short_option_pos(str, options); + bool last_option_requires_param = false; use_common = 1; if (use_switches) { if (str[0] == L'-') { // Check if we are entering a combined option and argument (like --color=auto or // -I/usr/include). for (const complete_entry_opt_t &o : options) { - const wchar_t *arg = param_match2(&o, str.c_str()); + const wchar_t *arg; + if (o.type == option_type_short) { + if (short_opt_pos == wcstring::npos) continue; + if (o.option.at(0) != str.at(short_opt_pos)) continue; + last_option_requires_param = o.result_mode.requires_param; + arg = str.c_str() + short_opt_pos + 1; + } else { + arg = param_match2(&o, str.c_str()); + } if (arg != NULL && this->condition_test(o.condition)) { if (o.result_mode.requires_param) use_common = false; if (o.result_mode.no_files) use_files = false; @@ -987,17 +982,27 @@ bool completer_t::complete_param(const wcstring &cmd_orig, const wcstring &popt, } // No old style option matched, or we are not using old style options. We check if - // any short (or gnu style options do. + // any short (or gnu style) options do. if (!old_style_match) { + size_t prev_short_opt_pos = short_option_pos(popt, options); for (const complete_entry_opt_t &o : options) { // Gnu-style options with _optional_ arguments must be specified as a single // token, so that it can be differed from a regular argument. // Here we are testing the previous argument for a GNU-style match, // to see how we should complete the current argument - if (o.type == option_type_double_long && !o.result_mode.requires_param) - continue; + if (!o.result_mode.requires_param) continue; - if (param_match(&o, popt.c_str()) && this->condition_test(o.condition)) { + bool match = false; + if (o.type == option_type_short) { + match = prev_short_opt_pos != wcstring::npos && + // Only if the option was the last char in the token, + // i.e. there is no parameter yet. + prev_short_opt_pos + 1 == popt.size() && + o.option.at(0) == popt.at(prev_short_opt_pos); + } else if (o.type == option_type_double_long) { + match = param_match(&o, popt.c_str()); + } + if (match && this->condition_test(o.condition)) { if (o.result_mode.requires_param) use_common = false; if (o.result_mode.no_files) use_files = false; if (o.result_mode.force_files) has_force = true; @@ -1026,7 +1031,21 @@ bool completer_t::complete_param(const wcstring &cmd_orig, const wcstring &popt, } // Check if the short style option matches. - if (short_ok(str, &o, options)) { + if (o.type == option_type_short) { + wchar_t optchar = o.option.at(0); + if (short_opt_pos == wcstring::npos) { + // str has no short option at all (but perhaps it is the + // prefix of a single long option). + // Only complete short options if there is no character after the dash. + if (str != L"-") continue; + } else { + // Only complete when the last short option has no parameter yet.. + if (short_opt_pos + 1 != str.size()) continue; + // .. and it does not require one .. + if (last_option_requires_param) continue; + // .. and the option is not already there. + if (str.find(optchar) != wcstring::npos) continue; + } // It's a match. wcstring desc = o.localized_desc(); // Append a short-style option diff --git a/tests/checks/complete.fish b/tests/checks/complete.fish index e8db603c7..48095f39d 100644 --- a/tests/checks/complete.fish +++ b/tests/checks/complete.fish @@ -88,3 +88,39 @@ complete -C'complete_test_recurse1 ' # CHECKERR: recursing # CHECKERR: recursing # CHECKERR: complete: maximum recursion depth reached + +# short options +complete -c foo -f -a non-option-argument +complete -c foo -f --short-option x +complete -c foo -f --short-option y -a 'ARGY' +complete -c foo -f --short-option z -a 'ARGZ' -r +complete -c foo -f --old-option single-long-ending-in-z +complete -c foo -f --old-option x-single-long +complete -c foo -f --old-option y-single-long +complete -c foo -f --old-option z-single-long +complete -c foo -f --long-option x-long -a 'ARGLONG' +# Make sure that arguments of concatenated short options are expanded (#332) +complete -C'foo -xy' +# CHECK: -xyARGY +# CHECK: -xyz +# A required parameter means we don't want more short options. +complete -C'foo -yz' +# CHECK: -yzARGZ +# Required parameter with space: complete only the parameter (no non-option arguments). +complete -C'foo -xz ' +# CHECK: ARGZ +# Optional parameter with space: complete only non-option arguments. +complete -C'foo -xy ' +# CHECK: non-option-argument +complete -C'foo -single-long-ending-in-z' +# CHECK: -single-long-ending-in-z +complete -C'foo -single-long-ending-in-z ' +# CHECK: non-option-argument +# CHECK: -x-single-long +complete -C'foo -x' | string match -- -x-single-long +# CHECK: -y-single-long +complete -C'foo -y' | string match -- -y-single-long +# This does NOT suggest -z-single-long, but will probably not occur in practise. +# CHECK: -zARGZ +complete -C'foo -z' +