From 64d4d29d43c78a403cff81d52ad0dabe06518f51 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sun, 12 Feb 2023 20:12:52 +0100 Subject: [PATCH] Use parameter parser to skip switch args in completion The command line "hook -group xyz " should get scope completions but it actually gets hook completions because "xyz" is wrongly interpreted as positional argument. Fix this by using the parameters parser to compute positional arguments. Fixes #4840 --- src/command_manager.cc | 31 +++++++++++++++++++------------ src/parameters_parser.cc | 28 ++++++++++++++++++++++++---- src/parameters_parser.hh | 11 ++++++++++- 3 files changed, 53 insertions(+), 17 deletions(-) diff --git a/src/command_manager.cc b/src/command_manager.cc index 13d79b2d..a2f1aad5 100644 --- a/src/command_manager.cc +++ b/src/command_manager.cc @@ -812,24 +812,31 @@ Completions CommandManager::complete(const Context& context, auto& command = command_it->value; - const bool has_switches = not command.param_desc.switches.empty(); - auto is_switch = [=](StringView s) { return has_switches and s.substr(0_byte, 1_byte) == "-"; }; + auto raw_params = tokens | skip(1) | transform(&Token::content) | gather>(); + ParametersParser parser{raw_params, command.param_desc, true}; - if (is_switch(token.content) - and not contains(tokens | drop(1) | transform(&Token::content), "--")) + switch (parser.state()) { - auto switches = Kakoune::complete(token.content.substr(1_byte), pos_in_token, - concatenated(command.param_desc.switches - | transform(&SwitchMap::Item::key), - ConstArrayView{"-"})); - return switches.empty() - ? Completions{} - : Completions{start+1, cursor_pos, std::move(switches), Completions::Flags::Menu}; + case ParametersParser::State::Switch: + { + auto switches = Kakoune::complete(token.content.substr(1_byte), pos_in_token, + concatenated(command.param_desc.switches + | transform(&SwitchMap::Item::key), + ConstArrayView{"-"})); + return switches.empty() + ? Completions{} + : Completions{start+1, cursor_pos, std::move(switches), Completions::Flags::Menu}; + } + case ParametersParser::State::SwitchArgument: + return Completions{}; + case ParametersParser::State::Positional: + break; } + if (not command.completer) return Completions{}; - auto params = tokens | skip(1) | transform(&Token::content) | filter(std::not_fn(is_switch)) | gather(); + Vector params{parser.begin(), parser.end()}; auto index = params.size() - 1; return offset_pos(requote(command.completer(context, flags, params, index, pos_in_token), token.type), start); diff --git a/src/parameters_parser.cc b/src/parameters_parser.cc index 8de8241b..633cb092 100644 --- a/src/parameters_parser.cc +++ b/src/parameters_parser.cc @@ -25,7 +25,7 @@ String generate_switches_doc(const SwitchMap& switches) return res; } -ParametersParser::ParametersParser(ParameterList params, const ParameterDesc& desc) +ParametersParser::ParametersParser(ParameterList params, const ParameterDesc& desc, bool ignore_errors) : m_params(params) { const bool switches_only_at_start = desc.flags & ParameterDesc::Flags::SwitchesOnlyAtStart; @@ -36,11 +36,16 @@ ParametersParser::ParametersParser(ParameterList params, const ParameterDesc& de for (size_t i = 0; i < params.size(); ++i) { if (not only_pos and not ignore_unknown_switches and params[i] == "--") + { + m_state = State::Switch; only_pos = true; + } else if (not only_pos and not params[i].empty() and params[i][0_byte] == '-') { StringView switch_name = params[i].substr(1_byte); auto it = desc.switches.find(switch_name); + m_state = it == desc.switches.end() and ignore_unknown_switches ? + State::Positional : State::Switch; if (it == desc.switches.end()) { if (ignore_unknown_switches) @@ -50,28 +55,43 @@ ParametersParser::ParametersParser(ParameterList params, const ParameterDesc& de only_pos = true; continue; } + if (ignore_errors) + continue; throw unknown_option(params[i]); } auto switch_index = it - desc.switches.begin(); if (switch_seen[switch_index]) + { + if (ignore_errors) + continue; throw runtime_error{format("switch '-{}' specified more than once", it->key)}; + } switch_seen[switch_index] = true; - if (it->value.takes_arg and ++i == params.size()) - throw missing_option_value(it->key); + if (it->value.takes_arg) + { + if (++i == params.size()) + { + if (ignore_errors) + continue; + throw missing_option_value(it->key); + } + m_state = State::SwitchArgument; + } m_switches[switch_name.str()] = it->value.takes_arg ? params[i] : StringView{}; } else // positional { + m_state = State::Positional; if (switches_only_at_start) only_pos = true; m_positional_indices.push_back(i); } } size_t count = m_positional_indices.size(); - if (count > desc.max_positionals or count < desc.min_positionals) + if (not ignore_errors and (count > desc.max_positionals or count < desc.min_positionals)) throw wrong_argument_count(); } diff --git a/src/parameters_parser.hh b/src/parameters_parser.hh index 69fa88eb..680c6f93 100644 --- a/src/parameters_parser.hh +++ b/src/parameters_parser.hh @@ -74,7 +74,13 @@ struct ParametersParser // the options defines named options, if they map to true, then // they are understood as string options, else they are understood as // boolean option. - ParametersParser(ParameterList params, const ParameterDesc& desc); + ParametersParser(ParameterList params, const ParameterDesc& desc, bool ignore_errors = false); + + enum class State { + Switch, + SwitchArgument, + Positional, + }; // Return a valid optional if the switch was given, with // a non empty StringView value if the switch took an argument. @@ -132,10 +138,13 @@ struct ParametersParser iterator begin() const { return iterator(*this, 0); } iterator end() const { return iterator(*this, m_positional_indices.size()); } + State state() const { return *m_state; } + private: ParameterList m_params; Vector m_positional_indices; HashMap m_switches; + Optional m_state; }; }