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

@@ -812,11 +812,12 @@ Completions CommandManager::complete(const Context& context,
 
         auto& command = command_it->value;
 
-        const bool has_switches = not command.param_desc.switches.empty();
+        auto raw_params = tokens | skip(1) | transform(&Token::content) | gather<Vector<String>>();
-        auto is_switch = [=](StringView s) { return has_switches and s.substr(0_byte, 1_byte) == "-"; };
+        ParametersParser parser{raw_params, command.param_desc, true};
 
-        if (is_switch(token.content)
+        switch (parser.state())
-            and not contains(tokens | drop(1) | transform(&Token::content), "--"))
+        {
+            case ParametersParser::State::Switch:
             {
                 auto switches = Kakoune::complete(token.content.substr(1_byte), pos_in_token,
                                                   concatenated(command.param_desc.switches
@@ -826,10 +827,16 @@ Completions CommandManager::complete(const Context& context,
                         ? 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>();
+        Vector<String> 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);

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())
+            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();
 }
 

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<size_t, MemoryDomain::Commands> m_positional_indices;
     HashMap<String, StringView> m_switches;
+    Optional<State> m_state;
 };
 
 }