Move highlighters into Scopes

That means we can now have highlighters active at global, buffer, and
window scope. The add-highlighter and remove-highlighter syntax changed
to take the parent path (scope/group/...) as a mandatory argument,
superseeding the previous -group switch.

6 files changed, 102 insertions, 95 deletions

diff --git a/src/commands.cc b/src/commands.cc
index 1593901e..c0cdc2be 100644
--- a/src/commands.cc
+++ b/src/commands.cc
@@ -633,112 +633,93 @@ const CommandDesc rename_buffer_cmd = {
     }
 };
 
-Completions complete_highlighter(const Context& context,
-                                 StringView arg, ByteCount pos_in_token, bool only_group)
-{
-    const bool shared = not arg.empty() and arg[0_byte] == '/';
-    if (shared)
-    {
-        auto& group = DefinedHighlighters::instance();
-        return offset_pos(group.complete_child(arg.substr(1_byte), pos_in_token-1, only_group), 1);
-    }
-    else
-    {
-        auto& group = context.window().highlighters();
-        return group.complete_child(arg, pos_in_token, only_group);
-    }
-}
+static constexpr auto highlighter_scopes = { "global", "buffer", "window", "shared" };
 
-Completions remove_highlighter_completer(
+template<bool add>
+Completions highlighter_cmd_completer(
     const Context& context, CompletionFlags flags, CommandParameters params,
     size_t token_to_complete, ByteCount pos_in_token)
 {
-    const String& arg = params[token_to_complete];
-    if (token_to_complete == 0 and not arg.empty() and arg.front() == '/')
+    if (token_to_complete == 0)
     {
-        auto& group = DefinedHighlighters::instance();
-        return offset_pos(group.complete_child(arg.substr(1_byte), pos_in_token-1, false), 1);
-    }
-    else if (token_to_complete == 0)
-        return context.window().highlighters().complete_child(arg, pos_in_token, false);
-    return {};
-}
 
-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(context, params[1], pos_in_token, true);
-    else if (token_to_complete == 0 or (token_to_complete == 2 and params[0] == "-group"))
-        return { 0_byte, arg.length(), complete(arg, pos_in_token, HighlighterRegistry::instance() | transform(std::mem_fn(&HighlighterRegistry::Item::key))) };
-    return Completions{};
-}
+        StringView path = params[0];
+        auto sep_it = find(path, '/');
+        if (sep_it == path.end())
+           return { 0_byte, pos_in_token, complete(path, pos_in_token, highlighter_scopes) };
 
-Highlighter& get_highlighter(const Context& context, StringView path)
-{
-    if (path.empty())
-        throw runtime_error("group path should not be empty");
+        StringView scope{path.begin(), sep_it};
+        HighlighterGroup* root = nullptr;
+        if (scope == "shared")
+            root = &DefinedHighlighters::instance();
+        else if (auto* s = get_scope_ifp(scope, context))
+            root = &s->highlighters();
+        else
+            return {};
 
-    Highlighter* root = nullptr;
-    if (path[0_byte] == '/')
+        auto offset = scope.length() + 1;
+        return offset_pos(root->complete_child(StringView{sep_it+1, path.end()}, pos_in_token - offset, add), offset);
+    }
+    else if (add and token_to_complete == 1)
     {
-        root = &DefinedHighlighters::instance();
-        path = path.substr(1_byte);
+        StringView name = params[1];
+        return { 0_byte, name.length(), complete(name, pos_in_token, HighlighterRegistry::instance() | transform(std::mem_fn(&HighlighterRegistry::Item::key))) };
     }
     else
-        root = &context.window().highlighters();
+        return {};
+}
 
-    if (path.back() == '/')
+Highlighter& get_highlighter(const Context& context, StringView path)
+{
+    if (not path.empty() and path.back() == '/')
         path = path.substr(0_byte, path.length() - 1);
 
-    if (not path.empty())
-        return root->get_child(path);
+    auto sep_it = find(path, '/');
+    StringView scope{path.begin(), sep_it};
+    auto* root = (scope == "shared") ? (HighlighterGroup*)&DefinedHighlighters::instance()
+                                     : (HighlighterGroup*)&get_scope(scope, context).highlighters();
+    if (sep_it != path.end())
+        return root->get_child(StringView{sep_it+1, path.end()});
     return *root;
 }
 
 const CommandDesc add_highlighter_cmd = {
     "add-highlighter",
     "addhl",
-    "add-highlighter <type> <type params>...: add an highlighter",
-    ParameterDesc{
-        { { "group", { true, "Set the group in which to put the highlighter. "
-                             "If starting with /, search in shared highlighters, "
-                             "else search in the current window" } } },
-      ParameterDesc::Flags::SwitchesOnlyAtStart, 1
-    },
+    "add-highlighter <path> <type> <type params>...: add an highlighter to the group identified by <path>\n"
+    "    <path> is a '/' delimited path of highlighters, starting with either\n"
+    "   'global', 'buffer', 'window' or 'shared'",
+    ParameterDesc{ {}, ParameterDesc::Flags::SwitchesAsPositional, 2 },
     CommandFlags::None,
     [](const Context& context, CommandParameters params) -> String
     {
-        if (params.size() > 0)
+        if (params.size() > 1)
         {
             HighlighterRegistry& registry = HighlighterRegistry::instance();
-            auto it = registry.find(params[0]);
+            auto it = registry.find(params[1]);
             if (it != registry.end())
-                return format("{}:\n{}", params[0], indent(it->value.docstring));
+                return format("{}:\n{}", params[1], indent(it->value.docstring));
         }
         return "";
     },
-    add_highlighter_completer,
+    highlighter_cmd_completer<true>,
     [](const ParametersParser& parser, Context& context, const ShellContext&)
     {
         HighlighterRegistry& registry = HighlighterRegistry::instance();
 
         auto begin = parser.begin();
-        const String& name = *begin++;
+        StringView path = *begin++;
+        StringView name = *begin++;
         Vector<String> highlighter_params;
         for (; begin != parser.end(); ++begin)
             highlighter_params.push_back(*begin);
 
-        auto group_name = parser.get_switch("group");
-        auto& group = group_name ? get_highlighter(context, *group_name)
-                                 : context.window().highlighters();
         auto it = registry.find(name);
         if (it == registry.end())
             throw runtime_error(format("No such highlighter factory '{}'", name));
-        group.add_child(it->value.factory(highlighter_params));
+        get_highlighter(context, path).add_child(it->value.factory(highlighter_params));
 
+        // TODO: better, this will fail if we touch scopes highlighters that impact multiple windows
         if (context.has_window())
             context.window().force_redraw();
     }
@@ -747,20 +728,19 @@ const CommandDesc add_highlighter_cmd = {
 const CommandDesc remove_highlighter_cmd = {
     "remove-highlighter",
     "rmhl",
-    "remove-highlighter <name>: remove highlighter <name>",
-    single_param,
+    "remove-highlighter <path>: remove highlighter identified by <path>",
+    { {}, ParameterDesc::Flags::None, 1, 1 },
     CommandFlags::None,
     CommandHelper{},
-    remove_highlighter_completer,
+    highlighter_cmd_completer<false>,
     [](const ParametersParser& parser, Context& context, const ShellContext&)
     {
         StringView path = parser[0];
-        auto sep_it = find(path | reverse(), '/');
-        auto& group = sep_it != path.rend() ?
-            get_highlighter(context, {path.begin(), sep_it.base()})
-          : context.window().highlighters();
-
-        group.remove_child({sep_it.base(), path.end()});
+        auto rev_path = path | reverse();
+        auto sep_it = find(rev_path, '/');
+        if (sep_it == rev_path.end())
+            return;
+        get_highlighter(context, {path.begin(), sep_it.base()}).remove_child({sep_it.base(), path.end()});
 
         if (context.has_window())
             context.window().force_redraw();
diff --git a/src/highlighter_group.cc b/src/highlighter_group.cc
index 33723d66..39130159 100644
--- a/src/highlighter_group.cc
+++ b/src/highlighter_group.cc
@@ -69,4 +69,19 @@ Completions HighlighterGroup::complete_child(StringView path, ByteCount cursor_p
     return { 0, 0, std::move(candidates) };
 }
 
+void Highlighters::do_highlight(const Context& context, HighlightPass pass,
+                                    DisplayBuffer& display_buffer, BufferRange range)
+{
+    if (m_parent)
+        m_parent->do_highlight(context, pass, display_buffer, range);
+    HighlighterGroup::do_highlight(context, pass, display_buffer, range);
+}
+
+void Highlighters::do_compute_display_setup(const Context& context, HighlightPass pass, DisplaySetup& setup)
+{
+    if (m_parent)
+        m_parent->do_compute_display_setup(context, pass, setup);
+    HighlighterGroup::do_compute_display_setup(context, pass, setup);
+}
+
 }
diff --git a/src/highlighter_group.hh b/src/highlighter_group.hh
index 12fc07d3..95a88a6b 100644
--- a/src/highlighter_group.hh
+++ b/src/highlighter_group.hh
@@ -5,6 +5,7 @@
 #include "hash_map.hh"
 #include "highlighter.hh"
 #include "utils.hh"
+#include "safe_ptr.hh"
 
 namespace Kakoune
 {
@@ -27,7 +28,7 @@ public:
 
     Completions complete_child(StringView path, ByteCount cursor_pos, bool group) const override;
 
-private:
+protected:
     void do_highlight(const Context& context, HighlightPass pass, DisplayBuffer& display_buffer, BufferRange range) override;
     void do_compute_display_setup(const Context& context, HighlightPass pass, DisplaySetup& setup) override;
 
@@ -35,6 +36,21 @@ private:
     HighlighterMap m_highlighters;
 };
 
+class Highlighters : public HighlighterGroup, public SafeCountable
+{
+public:
+    Highlighters(Highlighters& parent) : HighlighterGroup{HighlightPass::All}, SafeCountable{}, m_parent(&parent) {}
+
+private:
+    void do_highlight(const Context& context, HighlightPass pass, DisplayBuffer& display_buffer, BufferRange range) override;
+    void do_compute_display_setup(const Context& context, HighlightPass pass, DisplaySetup& setup) override;
+
+    friend class Scope;
+    Highlighters() : HighlighterGroup{HighlightPass::All} {}
+
+    SafePtr<Highlighters> m_parent;
+};
+
 struct DefinedHighlighters : public HighlighterGroup,
                              public Singleton<DefinedHighlighters>
 {
diff --git a/src/scope.hh b/src/scope.hh
index a27d5fce..58cc9477 100644
--- a/src/scope.hh
+++ b/src/scope.hh
@@ -5,6 +5,7 @@
 #include "hook_manager.hh"
 #include "keymap_manager.hh"
 #include "option_manager.hh"
+#include "highlighter_group.hh"
 #include "utils.hh"
 
 namespace Kakoune
@@ -17,16 +18,19 @@ public:
         : m_options(parent.options()),
           m_hooks(parent.hooks()),
           m_keymaps(parent.keymaps()),
-          m_aliases(parent.aliases()) {}
-
-    OptionManager&       options()       { return m_options; }
-    const OptionManager& options() const { return m_options; }
-    HookManager&         hooks()         { return m_hooks; }
-    const HookManager&   hooks()   const { return m_hooks; }
-    KeymapManager&       keymaps()       { return m_keymaps; }
-    const KeymapManager& keymaps() const { return m_keymaps; }
-    AliasRegistry&       aliases()       { return m_aliases; }
-    const AliasRegistry& aliases() const { return m_aliases; }
+          m_aliases(parent.aliases()),
+          m_highlighters(parent.highlighters()) {}
+
+    OptionManager&       options()            { return m_options; }
+    const OptionManager& options()      const { return m_options; }
+    HookManager&         hooks()              { return m_hooks; }
+    const HookManager&   hooks()        const { return m_hooks; }
+    KeymapManager&       keymaps()            { return m_keymaps; }
+    const KeymapManager& keymaps()      const { return m_keymaps; }
+    AliasRegistry&       aliases()            { return m_aliases; }
+    const AliasRegistry& aliases()      const { return m_aliases; }
+    Highlighters&        highlighters()       { return m_highlighters; }
+    const Highlighters&  highlighters() const { return m_highlighters; }
 
 private:
     friend class GlobalScope;
@@ -36,6 +40,7 @@ private:
     HookManager   m_hooks;
     KeymapManager m_keymaps;
     AliasRegistry m_aliases;
+    Highlighters  m_highlighters;
 };
 
 class GlobalScope : public Scope, public Singleton<GlobalScope>
diff --git a/src/window.cc b/src/window.cc
index 09652a9b..e1bd3ee5 100644
--- a/src/window.cc
+++ b/src/window.cc
@@ -22,8 +22,7 @@ void setup_builtin_highlighters(HighlighterGroup& group);
 Window::Window(Buffer& buffer)
     : Scope(buffer),
       m_buffer(&buffer),
-      m_highlighters{HighlightPass::All},
-      m_builtin_highlighters{HighlightPass::All}
+      m_builtin_highlighters{highlighters()}
 {
     run_hook_in_own_context("WinCreate", buffer.name());
 
@@ -144,10 +143,7 @@ const DisplayBuffer& Window::update_display_buffer(const Context& context)
     m_display_buffer.compute_range();
     BufferRange range{{0,0}, buffer().end_coord()};
     for (auto pass : { HighlightPass::Wrap, HighlightPass::Move, HighlightPass::Colorize })
-    {
-        m_highlighters.highlight(context, pass, m_display_buffer, range);
         m_builtin_highlighters.highlight(context, pass, m_display_buffer, range);
-    }
 
     m_display_buffer.optimize();
 
@@ -204,8 +200,6 @@ DisplaySetup Window::compute_display_setup(const Context& context)
         false
     };
     for (auto pass : { HighlightPass::Move, HighlightPass::Wrap })
-        m_highlighters.compute_display_setup(context, pass, setup);
-    for (auto pass : { HighlightPass::Move, HighlightPass::Wrap })
         m_builtin_highlighters.compute_display_setup(context, pass, setup);
 
     // now ensure the cursor column is visible
diff --git a/src/window.hh b/src/window.hh
index 6aeaa0c0..28971bcb 100644
--- a/src/window.hh
+++ b/src/window.hh
@@ -40,8 +40,6 @@ public:
     Optional<DisplayCoord> display_position(BufferCoord coord) const;
     BufferCoord buffer_coord(DisplayCoord coord) const;
 
-    Highlighter& highlighters() { return m_highlighters; }
-
     Buffer& buffer() const { return *m_buffer; }
 
     bool needs_redraw(const Context& context) const;
@@ -67,8 +65,7 @@ private:
     DisplayCoord m_dimensions;
     DisplayBuffer m_display_buffer;
 
-    HighlighterGroup m_highlighters;
-    HighlighterGroup m_builtin_highlighters;
+    Highlighters m_builtin_highlighters;
 
     struct Setup
     {