Refactor highlighter command completions

src/commands.cc

@@ -350,39 +350,70 @@ const CommandDesc namebuf_cmd = {
     }
 };
 
-template<typename GetRootGroup>
+Completions complete_highlighter_group(const Context& context,
-CommandCompleter group_rm_completer(GetRootGroup get_root_group)
+                                       StringView arg, ByteCount pos_in_token)
 {
-    return [=](const Context& context, CompletionFlags flags,
+    const bool shared = not arg.empty() and arg[0] == '/';
-               CommandParameters params, size_t token_to_complete,
+    if (shared)
-               ByteCount pos_in_token) -> Completions {
+    {
-        auto& root_group = get_root_group(context);
+        auto& group = DefinedHighlighters::instance();
-        const String& arg = params[token_to_complete];
+        return offset_pos(group.complete_group_id(arg.substr(1_byte), pos_in_token-1), 1);
-        if (token_to_complete == 1 and params[0] == "-group")
+    }
-            return { 0_byte, arg.length(), root_group.complete_group_id(arg, pos_in_token) };
+    else
-        else if (token_to_complete == 2 and params[0] == "-group")
+    {
-            return { 0_byte, arg.length(), root_group.get_group(params[1], '/').complete_id(arg, pos_in_token) };
+        auto& group = context.window().highlighters();
-        return { 0_byte, arg.length(), root_group.complete_id(arg, pos_in_token) };
+        return group.complete_group_id(arg, pos_in_token);
-    };
+    }
 }
 
-template<typename FactoryRegistry, typename GetRootGroup>
+Completions rm_highlighter_completer(
-CommandCompleter group_add_completer(GetRootGroup get_root_group)
+    const Context& context, CompletionFlags flags, CommandParameters params,
+    size_t token_to_complete, ByteCount pos_in_token)
 {
-    return [=](const Context& context, CompletionFlags flags,
+    const String& arg = params[token_to_complete];
-               CommandParameters params, size_t token_to_complete,
+    if (token_to_complete == 0 and not arg.empty() and arg.front() == '/')
-               ByteCount pos_in_token) -> Completions {
+    {
-        auto& root_group = get_root_group(context);
+        auto& group = DefinedHighlighters::instance();
-        const String& arg = params[token_to_complete];
+        return offset_pos(group.complete_id(arg.substr(1_byte), pos_in_token-1), 1);
-        if (token_to_complete == 1 and params[0] == "-group")
+    }
-            return { 0_byte, arg.length(), root_group.complete_group_id(arg, pos_in_token) };
+    else if (token_to_complete == 0)
-        else if (token_to_complete == 0 or (token_to_complete == 2 and params[0] == "-group"))
+        return context.window().highlighters().complete_id(arg, pos_in_token);
-            return { 0_byte, arg.length(), FactoryRegistry::instance().complete_name(arg, pos_in_token) };
+    return {};
-        return Completions{};
-    };
 }
 
-HighlighterGroup& get_highlighters(const Context& c) { return c.window().highlighters(); }
+Completions add_highlighter_completer(
+    const Context& context, CompletionFlags flags, CommandParameters params,
+    size_t token_to_complete, ByteCount pos_in_token)
+{
+    StringView arg = params[token_to_complete];
+    if (token_to_complete == 1 and params[0] == "-group")
+        return complete_highlighter_group(context, params[1], pos_in_token);
+    else if (token_to_complete == 0 or (token_to_complete == 2 and params[0] == "-group"))
+        return { 0_byte, arg.length(), HighlighterRegistry::instance().complete_name(arg, pos_in_token) };
+    return Completions{};
+}
+
+HighlighterGroup& get_highlighter_group(const Context& context, StringView path)
+{
+    if (path.empty())
+        throw runtime_error("group path should not be empty");
+
+    HighlighterGroup* group = nullptr;
+    if (path[0] == '/')
+    {
+        group = &DefinedHighlighters::instance();
+        path = path.substr(1_byte);
+    }
+    else
+        group = &context.window().highlighters();
+
+    if (path.back() == '/')
+        path = path.substr(0_byte, path.length() - 1);
+
+    if (not path.empty())
+        return group->get_group(path);
+    return *group;
+}
 
 const CommandDesc add_highlighter_cmd = {
     "addhl",
@@ -393,7 +424,7 @@ const CommandDesc add_highlighter_cmd = {
         SwitchMap{ { "group", { true, "specify the group in which to put the highlighter" } } },
         ParameterDesc::Flags::SwitchesOnlyAtStart, 1 },
     CommandFlags::None,
-    group_add_completer<HighlighterRegistry>(get_highlighters),
+    add_highlighter_completer,
     [](const ParametersParser& parser, Context& context)
     {
         HighlighterRegistry& registry = HighlighterRegistry::instance();
@@ -404,48 +435,32 @@ const CommandDesc add_highlighter_cmd = {
         for (; begin != parser.end(); ++begin)
             highlighter_params.push_back(*begin);
 
-        HighlighterGroup* group = nullptr;
+        auto& group = (parser.has_option("group")) ?
-        if (parser.has_option("group"))
+            get_highlighter_group(context, parser.option_value("group"))
-        {
+          : context.window().highlighters();
-            StringView path = parser.option_value("group");
+        group.append(registry[name](highlighter_params));
-            if (path.empty())
-                throw runtime_error("group param should not be empty");
-
-            if (path[0] == '/')
-            {
-                group = &DefinedHighlighters::instance();
-                path = path.substr(1_byte);
-            }
-            else
-                group = &context.window().highlighters();
-
-            if (not path.empty())
-                group = &group->get_group(path, '/');
-        }
-        else
-            group = &context.window().highlighters();
-        group->append(registry[name](highlighter_params));
     }
 };
 
 const CommandDesc rm_highlighter_cmd = {
     "rmhl",
     "rh",
-    "rmhl <switches> <name>: remove highlighter <name> from current window",
+    "rmhl <path>: remove highlighter <name>",
     ParameterDesc{
-        SwitchMap{ { "group", { true, "remove highlighter from given group" } } },
+        SwitchMap{},
         ParameterDesc::Flags::None, 1, 1
     },
     CommandFlags::None,
-    group_rm_completer(get_highlighters),
+    rm_highlighter_completer,
     [](const ParametersParser& parser, Context& context)
     {
-        HighlighterGroup& window_hl = context.window().highlighters();
+        StringView path = parser[0];
-        HighlighterGroup& group = parser.has_option("group") ?
+        auto sep_it = find(reversed(path), '/');
-            window_hl.get_group(parser.option_value("group"), '/')
+        auto& group = sep_it != path.rend() ?
-          : window_hl;
+            get_highlighter_group(context, {path.begin(), sep_it.base()-1})
+          : context.window().highlighters();
 
-        group.remove(parser[0]);
+        group.remove({sep_it.base(), path.end()});
     }
 };
 

src/completion.hh

@@ -47,5 +47,11 @@ inline Completions complete_nothing(const Context& context, CompletionFlags,
 Completions shell_complete(const Context& context, CompletionFlags,
                            StringView, ByteCount cursor_pos);
 
+inline Completions offset_pos(Completions completion, ByteCount offset)
+{
+    return { completion.start + offset, completion.end + offset,
+             std::move(completion.candidates) };
+}
+
 }
 #endif // completion_hh_INCLUDED

src/highlighter_group.cc

@@ -3,6 +3,9 @@
 namespace Kakoune
 {
 
+static constexpr Codepoint path_separator = '/';
+
+
 void HighlighterGroup::operator()(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer) const
 {
     for (auto& hl : m_highlighters)
@@ -21,9 +24,9 @@ void HighlighterGroup::remove(StringView id)
     m_highlighters.remove(id);
 }
 
-HighlighterGroup& HighlighterGroup::get_group(StringView path, Codepoint path_separator)
+HighlighterGroup& HighlighterGroup::get_group(StringView path)
 {
-    auto sep_it = std::find(path.begin(), path.end(), path_separator);
+    auto sep_it = find(path, path_separator);
     StringView id(path.begin(), sep_it);
     auto it = m_highlighters.find(id);
     if (it == m_highlighters.end())
@@ -31,22 +34,22 @@ HighlighterGroup& HighlighterGroup::get_group(StringView path, Codepoint path_se
     if (auto* group = it->second.target<HighlighterGroup>())
     {
         if (sep_it != path.end())
-            return group->get_group({sep_it+1, path.end()}, path_separator);
+            return group->get_group({sep_it+1, path.end()});
         return *group;
     }
     else if (auto* hier_group = it->second.target<HierachicalHighlighter>())
     {
         if (sep_it == path.end())
             throw group_not_found("not a leaf group: "_str + id);
-        return hier_group->get_group({sep_it+1, path.end()}, path_separator);
+        return hier_group->get_group({sep_it+1, path.end()});
     }
     else
         throw group_not_found("not a group: "_str + id);
 }
 
-HighlighterFunc HighlighterGroup::get_highlighter(StringView path, Codepoint path_separator) const
+HighlighterFunc HighlighterGroup::get_highlighter(StringView path) const
 {
-    auto sep_it = std::find(path.begin(), path.end(), path_separator);
+    auto sep_it = find(path, path_separator);
     StringView id(path.begin(), sep_it);
     auto it = m_highlighters.find(id);
     if (it == m_highlighters.end())
@@ -54,50 +57,117 @@ HighlighterFunc HighlighterGroup::get_highlighter(StringView path, Codepoint pat
     if (sep_it == path.end())
         return HighlighterFunc{std::ref(it->second)};
     else if (auto* group = it->second.target<HighlighterGroup>())
-        return group->get_highlighter({sep_it+1, path.end()}, path_separator);
+        return group->get_highlighter({sep_it+1, path.end()});
     else if (auto* hier_group = it->second.target<HierachicalHighlighter>())
-        return hier_group->get_highlighter({sep_it+1, path.end()}, path_separator);
+        return hier_group->get_highlighter({sep_it+1, path.end()});
     else
         throw group_not_found("not a group: "_str + id);
 }
 
-CandidateList HighlighterGroup::complete_id(StringView prefix, ByteCount cursor_pos) const
+Completions HighlighterGroup::complete_id(StringView path, ByteCount cursor_pos) const
 {
-    return m_highlighters.complete_id(prefix, cursor_pos);
+    auto sep_it = find(path, path_separator);
+    StringView id(path.begin(), sep_it);
+    if (sep_it == path.end())
+        return { 0_byte, id.length(), m_highlighters.complete_id(id, cursor_pos) };
+
+    auto it = m_highlighters.find(id);
+    if (it != m_highlighters.end())
+    {
+        const ByteCount offset = (int)(sep_it + 1 - path.begin());
+        cursor_pos -= offset;
+        if (auto* group = it->second.target<HighlighterGroup>())
+            return offset_pos(group->complete_id({sep_it+1, path.end()}, cursor_pos), offset);
+        if (auto* hier_group = it->second.target<HierachicalHighlighter>())
+            return offset_pos(hier_group->complete_id({sep_it+1, path.end()}, cursor_pos), offset);
+    }
+    return {};
 }
 
-CandidateList HighlighterGroup::complete_group_id(StringView prefix, ByteCount cursor_pos) const
+Completions HighlighterGroup::complete_group_id(StringView path, ByteCount cursor_pos) const
 {
-    return m_highlighters.complete_id_if(
+    auto sep_it = find(path, path_separator);
-        prefix, cursor_pos, [](const HighlighterAndId& func) {
+    StringView id(path.begin(), sep_it);
-            return func.second.template target<HighlighterGroup>() != nullptr;
+    if (sep_it == path.end())
-        });
+        return { 0_byte, path.length(), m_highlighters.complete_id_if(
+            path, cursor_pos, [](const HighlighterAndId& func) {
+                return func.second.template target<HighlighterGroup>() or
+                       func.second.template target<HierachicalHighlighter>();
+            }) };
+
+    auto it = m_highlighters.find(id);
+    if (it != m_highlighters.end())
+    {
+        const ByteCount offset = (int)(sep_it + 1 - path.begin());
+        cursor_pos -= offset;
+        if (auto* group = it->second.target<HighlighterGroup>())
+            return offset_pos(group->complete_group_id({sep_it+1, path.end()}, cursor_pos), offset);
+        if (auto* hier_group = it->second.target<HierachicalHighlighter>())
+            return offset_pos(hier_group->complete_group_id({sep_it+1, path.end()}, cursor_pos), offset);
+    }
+    return {};
 }
 
-HighlighterGroup& HierachicalHighlighter::get_group(StringView path, Codepoint path_separator)
+HighlighterGroup& HierachicalHighlighter::get_group(StringView path)
 {
-    auto sep_it = std::find(path.begin(), path.end(), path_separator);
+    auto sep_it = find(path, path_separator);
     StringView id(path.begin(), sep_it);
     auto it = m_groups.find(id);
     if (it == m_groups.end())
         throw group_not_found("no such id: "_str + id);
     if (sep_it != path.end())
-        return it->second.get_group(StringView(sep_it+1, path.end()), path_separator);
+        return it->second.get_group(StringView(sep_it+1, path.end()));
     else
         return it->second;
 }
 
-HighlighterFunc HierachicalHighlighter::get_highlighter(StringView path, Codepoint path_separator) const
+HighlighterFunc HierachicalHighlighter::get_highlighter(StringView path) const
 {
-    auto sep_it = std::find(path.begin(), path.end(), path_separator);
+    auto sep_it = find(path, path_separator);
     StringView id(path.begin(), sep_it);
     auto it = m_groups.find(id);
     if (it == m_groups.end())
         throw group_not_found("no such id: "_str + id);
     if (sep_it != path.end())
-        return it->second.get_highlighter(StringView(sep_it+1, path.end()), path_separator);
+        return it->second.get_highlighter(StringView(sep_it+1, path.end()));
     else
         return HighlighterFunc(std::ref(it->second));
 }
 
+Completions HierachicalHighlighter::complete_id(StringView path, ByteCount cursor_pos) const
+{
+    auto sep_it = find(path, path_separator);
+    StringView id(path.begin(), sep_it);
+    auto it = m_groups.find(id);
+    if (sep_it == path.end())
+        return { 0_byte, id.length(), m_groups.complete_id(id, cursor_pos) };
+
+    if (it != m_groups.end())
+    {
+        const ByteCount offset = (int)(sep_it + 1 - path.begin());
+        return offset_pos(
+            it->second.complete_id({sep_it+1, path.end()},
+                                   cursor_pos - offset), offset);
+    }
+    return {};
+}
+
+Completions HierachicalHighlighter::complete_group_id(StringView path, ByteCount cursor_pos) const
+{
+    auto sep_it = find(path, path_separator);
+    StringView id(path.begin(), sep_it);
+    auto it = m_groups.find(id);
+    if (sep_it == path.end())
+        return { 0_byte, id.length(), m_groups.complete_id(id, cursor_pos) };
+
+    if (it != m_groups.end())
+    {
+        const ByteCount offset = (int)(sep_it + 1- path.begin());
+        return offset_pos(
+            it->second.complete_group_id({sep_it+1, path.end()},
+                                         cursor_pos - offset), offset);
+    }
+    return {};
+}
+
 }

src/highlighter_group.hh

@@ -24,11 +24,11 @@ public:
     void append(HighlighterAndId&& hl);
     void remove(StringView id);
 
-    HighlighterGroup& get_group(StringView path, Codepoint path_separator = 0);
+    HighlighterGroup& get_group(StringView path);
-    HighlighterFunc get_highlighter(StringView path, Codepoint path_separator = 0) const;
+    HighlighterFunc get_highlighter(StringView path) const;
 
-    CandidateList complete_id(StringView prefix, ByteCount cursor_pos) const;
+    Completions complete_id(StringView path, ByteCount cursor_pos) const;
-    CandidateList complete_group_id(StringView prefix, ByteCount cursor_pos) const;
+    Completions complete_group_id(StringView path, ByteCount cursor_pos) const;
 
 private:
     id_map<HighlighterFunc> m_highlighters;
@@ -53,8 +53,11 @@ public:
         m_callback(m_groups, context, flags, display_buffer);
     }
 
-    HighlighterGroup& get_group(StringView path, Codepoint path_separator = 0);
+    HighlighterGroup& get_group(StringView path);
-    HighlighterFunc get_highlighter(StringView path, Codepoint path_separator = 0) const;
+    HighlighterFunc get_highlighter(StringView path) const;
+
+    Completions complete_id(StringView path, ByteCount cursor_pos) const;
+    Completions complete_group_id(StringView path, ByteCount cursor_pos) const;
 
 protected:
     Callback m_callback;

src/highlighters.cc

@@ -660,7 +660,7 @@ HighlighterAndId reference_factory(HighlighterParameters params)
             {
                 try
                 {
-                    DefinedHighlighters::instance().get_highlighter(name, '/')(context, flags, display_buffer);
+                    DefinedHighlighters::instance().get_highlighter(name)(context, flags, display_buffer);
                 }
                 catch (group_not_found&)
                 {

src/utils.hh

@@ -151,6 +151,18 @@ struct ReversedContainer
     decltype(container.rend())   end()   { return container.rend(); }
 };
 
+template<typename Container>
+auto begin(ReversedContainer<Container>& c) -> decltype(c.begin())
+{
+    return c.begin();
+}
+
+template<typename Container>
+auto end(ReversedContainer<Container>& c) -> decltype(c.end())
+{
+    return c.end();
+}
+
 template<typename Container>
 ReversedContainer<Container> reversed(Container&& container)
 {