Refactor highlighters, use an interface with virtual methods

8 files changed, 308 insertions, 376 deletions

diff --git a/src/commands.cc b/src/commands.cc
index 1770df1e..2c21fb0e 100644
--- a/src/commands.cc
+++ b/src/commands.cc
@@ -370,19 +370,19 @@ const CommandDesc namebuf_cmd = {
     }
 };
 
-Completions complete_highlighter_group(const Context& context,
-                                       StringView arg, ByteCount pos_in_token)
+Completions complete_highlighter(const Context& context,
+                                 StringView arg, ByteCount pos_in_token, bool only_group)
 {
     const bool shared = not arg.empty() and arg[0] == '/';
     if (shared)
     {
         auto& group = DefinedHighlighters::instance();
-        return offset_pos(group.complete_group_id(arg.substr(1_byte), pos_in_token-1), 1);
+        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_group_id(arg, pos_in_token);
+        return group.complete_child(arg, pos_in_token, only_group);
     }
 }
 
@@ -394,10 +394,10 @@ Completions rm_highlighter_completer(
     if (token_to_complete == 0 and not arg.empty() and arg.front() == '/')
     {
         auto& group = DefinedHighlighters::instance();
-        return offset_pos(group.complete_id(arg.substr(1_byte), pos_in_token-1), 1);
+        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_id(arg, pos_in_token);
+        return context.window().highlighters().complete_child(arg, pos_in_token, false);
     return {};
 }
 
@@ -407,32 +407,32 @@ Completions add_highlighter_completer(
 {
     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);
+        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(), HighlighterRegistry::instance().complete_name(arg, pos_in_token) };
     return Completions{};
 }
 
-HighlighterGroup& get_highlighter_group(const Context& context, StringView path)
+Highlighter& get_highlighter(const Context& context, StringView path)
 {
     if (path.empty())
         throw runtime_error("group path should not be empty");
 
-    HighlighterGroup* group = nullptr;
+    Highlighter* root = nullptr;
     if (path[0] == '/')
     {
-        group = &DefinedHighlighters::instance();
+        root = &DefinedHighlighters::instance();
         path = path.substr(1_byte);
     }
     else
-        group = &context.window().highlighters();
+        root = &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;
+        return root->get_child(path);
+    return *root;
 }
 
 const CommandDesc add_highlighter_cmd = {
@@ -456,9 +456,9 @@ const CommandDesc add_highlighter_cmd = {
             highlighter_params.push_back(*begin);
 
         auto& group = (parser.has_option("group")) ?
-            get_highlighter_group(context, parser.option_value("group"))
+            get_highlighter(context, parser.option_value("group"))
           : context.window().highlighters();
-        group.append(registry[name](highlighter_params));
+        group.add_child(registry[name](highlighter_params));
     }
 };
 
@@ -477,10 +477,10 @@ const CommandDesc rm_highlighter_cmd = {
         StringView path = parser[0];
         auto sep_it = find(reversed(path), '/');
         auto& group = sep_it != path.rend() ?
-            get_highlighter_group(context, {path.begin(), sep_it.base()-1})
+            get_highlighter(context, {path.begin(), sep_it.base()-1})
           : context.window().highlighters();
 
-        group.remove({sep_it.base(), path.end()});
+        group.remove_child({sep_it.base(), path.end()});
     }
 };
 
diff --git a/src/highlighter.hh b/src/highlighter.hh
index 8060675d..494abfff 100644
--- a/src/highlighter.hh
+++ b/src/highlighter.hh
@@ -25,8 +25,39 @@ enum class HighlightFlags
 // color, adding information text (line numbering for example) or replacing
 // buffer content (folding for example)
 
-using HighlighterFunc = std::function<void (const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)>;
-using HighlighterAndId = std::pair<String, HighlighterFunc>;
+struct Highlighter;
+
+using HighlighterAndId = std::pair<String, std::unique_ptr<Highlighter>>;
+
+struct Highlighter
+{
+    virtual void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer) = 0;
+
+    virtual bool has_children() const { return false; }
+    virtual Highlighter& get_child(StringView path) { throw runtime_error("this highlighter do not hold children"); }
+    virtual void add_child(HighlighterAndId&& hl) { throw runtime_error("this highlighter do not hold children"); }
+    virtual void remove_child(StringView id) { throw runtime_error("this highlighter do not hold children"); }
+    virtual Completions complete_child(StringView path, ByteCount cursor_pos, bool group) const { throw runtime_error("this highlighter do not hold children"); }
+};
+
+template<typename Func>
+struct SimpleHighlighter : public Highlighter
+{
+    SimpleHighlighter(Func func) : m_func(std::move(func)) {}
+    virtual void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer) override
+    {
+        m_func(context, flags, display_buffer);
+    }
+private:
+    Func m_func;
+};
+
+template<typename T>
+std::unique_ptr<SimpleHighlighter<T>> make_simple_highlighter(T func)
+{
+    return make_unique<SimpleHighlighter<T>>(std::move(func));
+}
+
 using HighlighterParameters = memoryview<String>;
 using HighlighterFactory = std::function<HighlighterAndId (HighlighterParameters params)>;
 
diff --git a/src/highlighter_group.cc b/src/highlighter_group.cc
index d9c8131d..4379c4c5 100644
--- a/src/highlighter_group.cc
+++ b/src/highlighter_group.cc
@@ -3,164 +3,54 @@
 namespace Kakoune
 {
 
-static constexpr Codepoint path_separator = '/';
-
-
-void HighlighterGroup::operator()(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer) const
+void HighlighterGroup::highlight(const Context& context, HighlightFlags flags,
+                                 DisplayBuffer& display_buffer)
 {
     for (auto& hl : m_highlighters)
-       hl.second(context, flags, display_buffer);
+       hl.second->highlight(context, flags, display_buffer);
 }
 
-void HighlighterGroup::append(HighlighterAndId&& hl)
+void HighlighterGroup::add_child(HighlighterAndId&& hl)
 {
     if (m_highlighters.contains(hl.first))
         throw runtime_error("duplicate id: " + hl.first);
 
     m_highlighters.append(std::move(hl));
 }
-void HighlighterGroup::remove(StringView id)
-{
-    m_highlighters.remove(id);
-}
 
-HighlighterGroup& HighlighterGroup::get_group(StringView path)
+void HighlighterGroup::remove_child(StringView id)
 {
-    auto sep_it = find(path, path_separator);
-    StringView id(path.begin(), sep_it);
-    auto it = m_highlighters.find(id);
-    if (it == m_highlighters.end())
-        throw group_not_found("no such id: "_str + id);
-    if (auto* group = it->second.target<HighlighterGroup>())
-    {
-        if (sep_it != path.end())
-            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()});
-    }
-    else
-        throw group_not_found("not a group: "_str + id);
+    m_highlighters.remove(id);
 }
 
-HighlighterFunc HighlighterGroup::get_highlighter(StringView path) const
+Highlighter& HighlighterGroup::get_child(StringView path)
 {
-    auto sep_it = find(path, path_separator);
+    auto sep_it = find(path, '/');
     StringView id(path.begin(), sep_it);
     auto it = m_highlighters.find(id);
     if (it == m_highlighters.end())
-        throw group_not_found("no such id: "_str + id);
-    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()});
-    else if (auto* hier_group = it->second.target<HierachicalHighlighter>())
-        return hier_group->get_highlighter({sep_it+1, path.end()});
-    else
-        throw group_not_found("not a group: "_str + id);
-}
-
-template<Completions (HighlighterGroup::*hg_complete)(StringView path, ByteCount cursor_pos) const,
-         Completions (HierachicalHighlighter::*hh_complete)(StringView path, ByteCount cursor_pos) const,
-         typename Condition>
-Completions complete_impl(const id_map<HighlighterFunc>& highlighters, Condition condition,
-                          StringView path, ByteCount cursor_pos)
-{
-    auto sep_it = find(path, path_separator);
-    StringView id(path.begin(), sep_it);
+        throw child_not_found("no such id: "_str + id);
     if (sep_it == path.end())
-        return { 0_byte, path.length(), highlighters.complete_id_if(path, cursor_pos, condition) };
-
-    auto it = highlighters.find(id);
-    if (it != 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->*hg_complete)({sep_it+1, path.end()}, cursor_pos), offset);
-        if (auto* hier_group = it->second.target<HierachicalHighlighter>())
-            return offset_pos((hier_group->*hh_complete)({sep_it+1, path.end()}, cursor_pos), offset);
-    }
-    return {};
-}
-
-Completions HighlighterGroup::complete_id(StringView path, ByteCount cursor_pos) const
-{
-    return complete_impl<
-        &HighlighterGroup::complete_id,
-        &HierachicalHighlighter::complete_id
-    >(m_highlighters, [](const HighlighterAndId&) { return true; }, path, cursor_pos);
-}
-
-Completions HighlighterGroup::complete_group_id(StringView path, ByteCount cursor_pos) const
-{
-    return complete_impl<
-        &HighlighterGroup::complete_group_id,
-        &HierachicalHighlighter::complete_group_id
-    >(m_highlighters, [](const HighlighterAndId& func) {
-          return func.second.target<HighlighterGroup>() or
-                 func.second.target<HierachicalHighlighter>();
-      }, path, cursor_pos);
-}
-
-HighlighterGroup& HierachicalHighlighter::get_group(StringView path)
-{
-    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()));
+        return *it->second;
     else
-        return it->second;
+        return it->second->get_child({sep_it+1, path.end()});
 }
 
-HighlighterFunc HierachicalHighlighter::get_highlighter(StringView path) const
+Completions HighlighterGroup::complete_child(StringView path, ByteCount cursor_pos, bool group) const
 {
-    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);
+    auto sep_it = find(path, '/');
     if (sep_it != path.end())
-        return it->second.get_highlighter(StringView(sep_it+1, path.end()));
-    else
-        return HighlighterFunc(std::ref(it->second));
-}
-
-template<Completions (HighlighterGroup::*complete)(StringView path, ByteCount cursor_pos) const>
-Completions complete_impl(const HierachicalHighlighter::GroupMap& groups,
-                          StringView path, ByteCount cursor_pos)
-{
-    auto sep_it = find(path, path_separator);
-    StringView id(path.begin(), sep_it);
-    auto it = groups.find(id);
-    if (sep_it == path.end())
-        return { 0_byte, id.length(), groups.complete_id(id, cursor_pos) };
-
-    if (it != groups.end())
     {
-        const ByteCount offset = (int)(sep_it + 1- path.begin());
-        return offset_pos(
-            (it->second.*complete)({sep_it+1, path.end()},
-                                   cursor_pos - offset), offset);
+        ByteCount offset = sep_it+1 - path.begin();
+        Highlighter& hl = const_cast<HighlighterGroup*>(this)->get_child({path.begin(), sep_it});
+        return offset_pos(hl.complete_child(path.substr(offset), cursor_pos - offset, group), offset);
     }
-    return {};
-}
-
-Completions HierachicalHighlighter::complete_id(StringView path, ByteCount cursor_pos) const
-{
-    return complete_impl<&HighlighterGroup::complete_id>(m_groups, path, cursor_pos);
-}
 
-Completions HierachicalHighlighter::complete_group_id(StringView path, ByteCount cursor_pos) const
-{
-    return complete_impl<&HighlighterGroup::complete_group_id>(m_groups, path, cursor_pos);
+    auto condition = [=](const HighlighterMap::value_type& hl)
+    {
+        return not group || hl.second->has_children();
+    };
+    return { 0, 0, m_highlighters.complete_id_if(path, cursor_pos, condition) };
 }
 
 }
diff --git a/src/highlighter_group.hh b/src/highlighter_group.hh
index 94dbad12..7ba5c481 100644
--- a/src/highlighter_group.hh
+++ b/src/highlighter_group.hh
@@ -9,59 +9,27 @@
 namespace Kakoune
 {
 
-struct group_not_found : public runtime_error
+struct child_not_found : public runtime_error
 {
     using runtime_error::runtime_error;
 };
 
-class HighlighterGroup
+class HighlighterGroup : public Highlighter
 {
 public:
-    void operator()(const Context& context,
-                    HighlightFlags flags,
-                    DisplayBuffer& display_buffer) const;
+    void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer) override;
 
-    void append(HighlighterAndId&& hl);
-    void remove(StringView id);
+    bool has_children() const { return true; }
+    void add_child(HighlighterAndId&& hl) override;
+    void remove_child(StringView id) override;
 
-    HighlighterGroup& get_group(StringView path);
-    HighlighterFunc get_highlighter(StringView path) const;
+    Highlighter& get_child(StringView path) override;
 
-    Completions complete_id(StringView path, ByteCount cursor_pos) const;
-    Completions complete_group_id(StringView path, ByteCount cursor_pos) const;
+    Completions complete_child(StringView path, ByteCount cursor_pos, bool group) const override;
 
 private:
-    id_map<HighlighterFunc> m_highlighters;
-};
-
-class HierachicalHighlighter
-{
-public:
-    using GroupMap = id_map<HighlighterGroup>;
-    using Callback = std::function<void (GroupMap& groups,
-                                         const Context& context,
-                                         HighlightFlags flags,
-                                         DisplayBuffer& display_buffer)>;
-
-    HierachicalHighlighter(Callback callback, GroupMap groups)
-        : m_callback(std::move(callback)), m_groups(std::move(groups)) {}
-
-    void operator()(const Context& context,
-                    HighlightFlags flags,
-                    DisplayBuffer& display_buffer)
-    {
-        m_callback(m_groups, context, flags, display_buffer);
-    }
-
-    HighlighterGroup& get_group(StringView path);
-    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;
-    GroupMap m_groups;
+    using HighlighterMap = id_map<std::unique_ptr<Highlighter>>;
+    HighlighterMap m_highlighters;
 };
 
 struct DefinedHighlighters : public HighlighterGroup,
diff --git a/src/highlighters.cc b/src/highlighters.cc
index d9942b85..8acd83c8 100644
--- a/src/highlighters.cc
+++ b/src/highlighters.cc
@@ -63,12 +63,11 @@ void highlight_range(DisplayBuffer& display_buffer,
     }
 }
 
-template<typename T>
 void apply_highlighter(const Context& context,
                        HighlightFlags flags,
                        DisplayBuffer& display_buffer,
                        ByteCoord begin, ByteCoord end,
-                       T&& highlighter)
+                       Highlighter& highlighter)
 {
     using LineIterator = DisplayBuffer::LineList::iterator;
     LineIterator first_line;
@@ -135,7 +134,7 @@ void apply_highlighter(const Context& context,
     }
 
     region_display.compute_range();
-    highlighter(context, flags, region_display);
+    highlighter.highlight(context, flags, region_display);
 
     for (size_t i = 0; i < region_lines.size(); ++i)
     {
@@ -161,7 +160,7 @@ auto apply_face = [](const Face& face)
 
 using FacesSpec = std::vector<String>;
 
-HighlighterAndId fill_factory(HighlighterParameters params)
+static HighlighterAndId create_fill_highlighter(HighlighterParameters params)
 {
     if (params.size() != 1)
         throw runtime_error("wrong parameter count");
@@ -169,14 +168,14 @@ HighlighterAndId fill_factory(HighlighterParameters params)
     const String& facespec = params[0];
     get_face(facespec); // validate param
 
-    auto fill = [facespec](const Context& context, HighlightFlags flags,
-                            DisplayBuffer& display_buffer)
+    auto func = [=](const Context& context, HighlightFlags flags,
+                    DisplayBuffer& display_buffer)
     {
         auto range = display_buffer.range();
         highlight_range(display_buffer, range.first, range.second, true,
                         apply_face(get_face(facespec)));
     };
-    return HighlighterAndId("fill_" + params[0], fill);
+    return {"fill_" + facespec, make_simple_highlighter(std::move(func))};
 }
 
 template<typename T>
@@ -195,7 +194,7 @@ private:
     ValueId m_id;
 };
 
-class RegexHighlighter
+class RegexHighlighter : public Highlighter
 {
 public:
     RegexHighlighter(Regex regex, FacesSpec faces)
@@ -203,7 +202,7 @@ public:
     {
     }
 
-    void operator()(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
+    void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer) override
     {
         if (flags != HighlightFlags::Highlight)
             return;
@@ -225,6 +224,48 @@ public:
         }
     }
 
+    void reset(Regex regex, FacesSpec faces)
+    {
+        m_regex = std::move(regex);
+        m_faces = std::move(faces);
+        m_force_update = true;
+    }
+
+    static HighlighterAndId create(HighlighterParameters params)
+    {
+        if (params.size() < 2)
+            throw runtime_error("wrong parameter count");
+
+        try
+        {
+            static Regex face_spec_ex(R"((\d+):(.*))");
+            FacesSpec faces;
+            for (auto it = params.begin() + 1;  it != params.end(); ++it)
+            {
+                MatchResults<String::const_iterator> res;
+                if (not regex_match(it->begin(), it->end(), res, face_spec_ex))
+                    throw runtime_error("wrong face spec: '" + *it +
+                                         "' expected <capture>:<facespec>");
+                get_face(res[2].str()); // throw if wrong face spec
+                int capture = str_to_int(res[1].str());
+                if (capture >= faces.size())
+                    faces.resize(capture+1);
+                faces[capture] = res[2].str();
+            }
+
+            String id = "hlregex'" + params[0] + "'";
+
+            Regex ex{params[0].begin(), params[0].end(), Regex::optimize};
+
+            return {id, make_unique<RegexHighlighter>(std::move(ex),
+                                                      std::move(faces))};
+        }
+        catch (RegexError& err)
+        {
+            throw runtime_error(String("regex error: ") + err.what());
+        }
+    }
+
 private:
     struct Cache
     {
@@ -237,6 +278,8 @@ private:
     Regex     m_regex;
     FacesSpec m_faces;
 
+    bool m_force_update = false;
+
     Cache& update_cache_ifn(const Buffer& buffer, const BufferRange& range)
     {
         Cache& cache = m_cache.get(buffer);
@@ -244,11 +287,14 @@ private:
         LineCount first_line = range.first.line;
         LineCount last_line = std::min(buffer.line_count()-1, range.second.line);
 
-        if (buffer.timestamp() == cache.m_timestamp and
+        if (not m_force_update and
+            buffer.timestamp() == cache.m_timestamp and
             first_line >= cache.m_range.first and
             last_line <= cache.m_range.second)
            return cache;
 
+        m_force_update = false;
+
         cache.m_range.first  = std::max(0_line, first_line - 10);
         cache.m_range.second = std::min(buffer.line_count()-1, last_line+10);
         cache.m_timestamp = buffer.timestamp();
@@ -270,43 +316,8 @@ private:
     }
 };
 
-HighlighterAndId highlight_regex_factory(HighlighterParameters params)
-{
-    if (params.size() < 2)
-        throw runtime_error("wrong parameter count");
-
-    try
-    {
-        static Regex face_spec_ex(R"((\d+):(.*))");
-        FacesSpec faces;
-        for (auto it = params.begin() + 1;  it != params.end(); ++it)
-        {
-            MatchResults<String::const_iterator> res;
-            if (not regex_match(it->begin(), it->end(), res, face_spec_ex))
-                throw runtime_error("wrong face spec: '" + *it +
-                                     "' expected <capture>:<facespec>");
-            get_face(res[2].str()); // throw if wrong face spec
-            int capture = str_to_int(res[1].str());
-            if (capture >= faces.size())
-                faces.resize(capture+1);
-            faces[capture] = res[2].str();
-        }
-
-        String id = "hlregex'" + params[0] + "'";
-
-        Regex ex{params[0].begin(), params[0].end(), Regex::optimize};
-
-        return HighlighterAndId(id, RegexHighlighter(std::move(ex),
-                                                   std::move(faces)));
-    }
-    catch (RegexError& err)
-    {
-        throw runtime_error(String("regex error: ") + err.what());
-    }
-}
-
 template<typename RegexGetter, typename FaceGetter>
-class DynamicRegexHighlighter
+class DynamicRegexHighlighter : public Highlighter
 {
 public:
     DynamicRegexHighlighter(RegexGetter regex_getter, FaceGetter face_getter)
@@ -314,7 +325,7 @@ public:
           m_face_getter(std::move(face_getter)),
           m_highlighter(Regex(), FacesSpec{}) {}
 
-    void operator()(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
+    void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
     {
         if (flags != HighlightFlags::Highlight)
             return;
@@ -326,10 +337,10 @@ public:
             m_last_regex = regex;
             m_last_face = face;
             if (not m_last_regex.empty())
-                m_highlighter= RegexHighlighter{m_last_regex, face};
+                m_highlighter.reset(m_last_regex, m_last_face);
         }
         if (not m_last_regex.empty() and not m_last_face.empty())
-            m_highlighter(context, flags, display_buffer);
+            m_highlighter.highlight(context, flags, display_buffer);
     }
 
 private:
@@ -343,15 +354,15 @@ private:
 };
 
 template<typename RegexGetter, typename FaceGetter>
-DynamicRegexHighlighter<RegexGetter, FaceGetter>
+std::unique_ptr<DynamicRegexHighlighter<RegexGetter, FaceGetter>>
 make_dynamic_regex_highlighter(RegexGetter regex_getter, FaceGetter face_getter)
 {
-    return DynamicRegexHighlighter<RegexGetter, FaceGetter>(
+    return make_unique<DynamicRegexHighlighter<RegexGetter, FaceGetter>>(
         std::move(regex_getter), std::move(face_getter));
 }
 
 
-HighlighterAndId highlight_search_factory(HighlighterParameters params)
+HighlighterAndId create_search_highlighter(HighlighterParameters params)
 {
     if (params.size() != 0)
         throw runtime_error("wrong parameter count");
@@ -372,7 +383,7 @@ HighlighterAndId highlight_search_factory(HighlighterParameters params)
         return {"hlsearch", make_dynamic_regex_highlighter(get_regex, get_face)};
 }
 
-HighlighterAndId highlight_regex_option_factory(HighlighterParameters params)
+HighlighterAndId create_regex_option_highlighter(HighlighterParameters params)
 {
     if (params.size() != 2)
         throw runtime_error("wrong parameter count");
@@ -392,27 +403,26 @@ HighlighterAndId highlight_regex_option_factory(HighlighterParameters params)
     return {"hloption_" + option_name, make_dynamic_regex_highlighter(get_regex, get_face)};
 }
 
-HighlighterAndId highlight_line_option_factory(HighlighterParameters params)
+HighlighterAndId create_line_option_highlighter(HighlighterParameters params)
 {
     if (params.size() != 2)
         throw runtime_error("wrong parameter count");
 
     String facespec = params[1];
-    get_face(facespec); // validate facespec
-
     String option_name = params[0];
-    // verify option type now
-    GlobalOptions::instance()[option_name].get<int>();
 
-    auto highlighter = [=](const Context& context, HighlightFlags flags,
-                           DisplayBuffer& display_buffer)
+    get_face(facespec); // validate facespec
+    GlobalOptions::instance()[option_name].get<int>(); // verify option type now
+
+    auto func = [=](const Context& context, HighlightFlags flags,
+                    DisplayBuffer& display_buffer)
     {
         int line = context.options()[option_name].get<int>();
         highlight_range(display_buffer, {line-1, 0}, {line, 0}, false,
                         apply_face(get_face(facespec)));
     };
 
-    return {"hlline_" + option_name, std::move(highlighter)};
+    return {"hlline_" + params[0], make_simple_highlighter(std::move(func))};
 }
 
 void expand_tabulations(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
@@ -626,7 +636,7 @@ void expand_unprintable(const Context& context, HighlightFlags flags, DisplayBuf
     }
 }
 
-HighlighterAndId flag_lines_factory(HighlighterParameters params)
+HighlighterAndId create_flag_lines_highlighter(HighlighterParameters params)
 {
     if (params.size() != 2)
         throw runtime_error("wrong parameter count");
@@ -637,54 +647,42 @@ HighlighterAndId flag_lines_factory(HighlighterParameters params)
     // throw if wrong option type
     GlobalOptions::instance()[option_name].get<std::vector<LineAndFlag>>();
 
-    return {"hlflags_" + params[1],
-            [=](const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
-            {
-                auto& lines_opt = context.options()[option_name];
-                auto& lines = lines_opt.get<std::vector<LineAndFlag>>();
-
-                CharCount width = 0;
-                for (auto& l : lines)
-                     width = std::max(width, std::get<2>(l).char_length());
-                const String empty{' ', width};
-                for (auto& line : display_buffer.lines())
-                {
-                    int line_num = (int)line.range().first.line + 1;
-                    auto it = find_if(lines,
-                                      [&](const LineAndFlag& l)
-                                      { return std::get<0>(l) == line_num; });
-                    String content = it != lines.end() ? std::get<2>(*it) : empty;
-                    content += String(' ', width - content.char_length());
-                    DisplayAtom atom{std::move(content)};
-                    atom.face = { it != lines.end() ? std::get<1>(*it) : Colors::Default , bg };
-                    line.insert(line.begin(), std::move(atom));
-                }
-            }};
-}
-
-template<void (*highlighter_func)(const Context&, HighlightFlags, DisplayBuffer&)>
-class SimpleHighlighterFactory
-{
-public:
-    SimpleHighlighterFactory(const String& id) : m_id(id) {}
-
-    HighlighterAndId operator()(HighlighterParameters params) const
+    auto func = [=](const Context& context, HighlightFlags flags,
+                    DisplayBuffer& display_buffer)
     {
-        return HighlighterAndId(m_id, HighlighterFunc(highlighter_func));
-    }
-private:
-    String m_id;
-};
+        auto& lines_opt = context.options()[option_name];
+        auto& lines = lines_opt.get<std::vector<LineAndFlag>>();
+
+        CharCount width = 0;
+        for (auto& l : lines)
+             width = std::max(width, std::get<2>(l).char_length());
+        const String empty{' ', width};
+        for (auto& line : display_buffer.lines())
+        {
+            int line_num = (int)line.range().first.line + 1;
+            auto it = find_if(lines,
+                              [&](const LineAndFlag& l)
+                              { return std::get<0>(l) == line_num; });
+            String content = it != lines.end() ? std::get<2>(*it) : empty;
+            content += String(' ', width - content.char_length());
+            DisplayAtom atom{std::move(content)};
+            atom.face = { it != lines.end() ? std::get<1>(*it) : Colors::Default , bg };
+            line.insert(line.begin(), std::move(atom));
+        }
+    };
 
-HighlighterAndId highlighter_group_factory(HighlighterParameters params)
+    return {"hlflags_" + params[1], make_simple_highlighter(func) };
+}
+
+HighlighterAndId create_highlighter_group(HighlighterParameters params)
 {
     if (params.size() != 1)
         throw runtime_error("wrong parameter count");
 
-    return HighlighterAndId(params[0], HighlighterGroup());
+    return HighlighterAndId(params[0], make_unique<HighlighterGroup>());
 }
 
-HighlighterAndId reference_factory(HighlighterParameters params)
+HighlighterAndId create_reference_highlighter(HighlighterParameters params)
 {
     if (params.size() != 1)
         throw runtime_error("wrong parameter count");
@@ -694,17 +692,18 @@ HighlighterAndId reference_factory(HighlighterParameters params)
     // throw if not found
     //DefinedHighlighters::instance().get_group(name, '/');
 
-    return {name,
-            [name](const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
-            {
-                try
-                {
-                    DefinedHighlighters::instance().get_highlighter(name)(context, flags, display_buffer);
-                }
-                catch (group_not_found&)
-                {
-                }
-            }};
+    auto func = [=](const Context& context, HighlightFlags flags,
+                    DisplayBuffer& display_buffer)
+    {
+        try
+        {
+            DefinedHighlighters::instance().get_child(name).highlight(context, flags, display_buffer);
+        }
+        catch (child_not_found&)
+        {}
+    };
+
+    return {name, make_simple_highlighter(func)};
 }
 
 struct RegexMatch
@@ -868,7 +867,7 @@ struct RegionDesc
     }
 };
 
-struct RegionsHighlighter
+struct RegionsHighlighter : public Highlighter
 {
 public:
     using NamedRegionDescList = std::vector<std::pair<String, RegionDesc>>;
@@ -880,12 +879,16 @@ public:
             throw runtime_error("at least one region must be defined");
 
         for (auto& region : m_regions)
+        {
+            m_groups.append({region.first, HighlighterGroup{}});
             if (region.second.m_begin.empty() or region.second.m_end.empty())
                 throw runtime_error("invalid regex for region highlighter");
+        }
+        if (not m_default_group.empty())
+            m_groups.append({m_default_group, HighlighterGroup{}});
     }
 
-    void operator()(HierachicalHighlighter::GroupMap groups, const Context& context,
-                    HighlightFlags flags, DisplayBuffer& display_buffer)
+    void highlight(const Context& context, HighlightFlags flags, DisplayBuffer& display_buffer)
     {
         if (flags != HighlightFlags::Highlight)
             return;
@@ -904,8 +907,8 @@ public:
             return c;
         };
 
-        auto default_group_it = groups.find(m_default_group);
-        const bool apply_default = default_group_it != groups.end();
+        auto default_group_it = m_groups.find(m_default_group);
+        const bool apply_default = default_group_it != m_groups.end();
 
         auto last_begin = range.first;
         for (; begin != end; ++begin)
@@ -915,8 +918,8 @@ public:
                                   correct(last_begin), correct(begin->begin),
                                   default_group_it->second);
 
-            auto it = groups.find(begin->group);
-            if (it == groups.end())
+            auto it = m_groups.find(begin->group);
+            if (it == m_groups.end())
                 continue;
             apply_highlighter(context, flags, display_buffer,
                               correct(begin->begin), correct(begin->end),
@@ -929,9 +932,79 @@ public:
                               default_group_it->second);
 
     }
+
+    bool has_children() const override { return true; }
+
+    Highlighter& get_child(StringView path) override
+    {
+        auto sep_it = find(path, '/');
+        StringView id(path.begin(), sep_it);
+        auto it = m_groups.find(id);
+        if (it == m_groups.end())
+            throw child_not_found("no such id: "_str + id);
+        if (sep_it == path.end())
+            return it->second;
+        else
+            return it->second.get_child({sep_it+1, path.end()});
+    }
+
+    Completions complete_child(StringView path, ByteCount cursor_pos, bool group) const override
+    {
+        auto sep_it = find(path, '/');
+        if (sep_it != path.end())
+        {
+            ByteCount offset = sep_it+1 - path.begin();
+            Highlighter& hl = const_cast<RegionsHighlighter*>(this)->get_child({path.begin(), sep_it});
+            return offset_pos(hl.complete_child(path.substr(offset), cursor_pos - offset, group), offset);
+        }
+
+        return { 0, 0, m_groups.complete_id(path, cursor_pos) };
+    }
+
+    static HighlighterAndId create(HighlighterParameters params)
+    {
+        try
+        {
+            static const ParameterDesc param_desc{
+                SwitchMap{ { "default", { true, "" } } },
+                ParameterDesc::Flags::SwitchesOnlyAtStart,
+                5};
+
+            ParametersParser parser{params, param_desc};
+            if ((parser.positional_count() % 4) != 1)
+                throw runtime_error("wrong parameter count, expect <id> (<group name> <begin> <end> <recurse>)+");
+
+            RegionsHighlighter::NamedRegionDescList regions;
+            for (size_t i = 1; i < parser.positional_count(); i += 4)
+            {
+                if (parser[i].empty() or parser[i+1].empty() or parser[i+2].empty())
+                    throw runtime_error("group id, begin and end must not be empty");
+
+                Regex begin{parser[i+1], Regex::nosubs | Regex::optimize };
+                Regex end{parser[i+2], Regex::nosubs | Regex::optimize };
+                Regex recurse;
+                if (not parser[i+3].empty())
+                    recurse = Regex{parser[i+3], Regex::nosubs | Regex::optimize };
+
+                regions.push_back({ parser[i], {std::move(begin), std::move(end), std::move(recurse)} });
+            }
+            String default_group;
+            if (parser.has_option("default"))
+                default_group = parser.option_value("default");
+
+            return {parser[0], make_unique<RegionsHighlighter>(std::move(regions),
+                                                               std::move(default_group))};
+        }
+        catch (RegexError& err)
+        {
+            throw runtime_error(String("regex error: ") + err.what());
+        }
+    }
+
 private:
     const NamedRegionDescList m_regions;
     const String m_default_group;
+    id_map<HighlighterGroup> m_groups;
 
     struct Region
     {
@@ -1029,68 +1102,31 @@ private:
     }
 };
 
-HighlighterAndId regions_factory(HighlighterParameters params)
+template<typename Func>
+HighlighterFactory simple_factory(const String id, Func func)
 {
-    try
-    {
-        static const ParameterDesc param_desc{
-            SwitchMap{ { "default", { true, "" } } },
-            ParameterDesc::Flags::SwitchesOnlyAtStart,
-            5};
-
-        ParametersParser parser{params, param_desc};
-        if ((parser.positional_count() % 4) != 1)
-            throw runtime_error("wrong parameter count, expect <id> (<group name> <begin> <end> <recurse>)+");
-
-        RegionsHighlighter::NamedRegionDescList regions;
-        id_map<HighlighterGroup> groups;
-        for (size_t i = 1; i < parser.positional_count(); i += 4)
-        {
-            if (parser[i].empty() or parser[i+1].empty() or parser[i+2].empty())
-                throw runtime_error("group id, begin and end must not be empty");
-
-            Regex begin{parser[i+1], Regex::nosubs | Regex::optimize };
-            Regex end{parser[i+2], Regex::nosubs | Regex::optimize };
-            Regex recurse;
-            if (not parser[i+3].empty())
-                recurse = Regex{parser[i+3], Regex::nosubs | Regex::optimize };
-
-            regions.push_back({ parser[i], {std::move(begin), std::move(end), std::move(recurse)} });
-            groups.append({ parser[i], HighlighterGroup{} });
-        }
-        String default_group;
-        if (parser.has_option("default"))
-        {
-            default_group = parser.option_value("default");
-            groups.append({ default_group, HighlighterGroup{} });
-        }
-
-        return {parser[0],
-                HierachicalHighlighter(
-                    RegionsHighlighter(std::move(regions), std::move(default_group)), std::move(groups))};
-    }
-    catch (RegexError& err)
+    return [=](HighlighterParameters params)
     {
-        throw runtime_error(String("regex error: ") + err.what());
-    }
+        return HighlighterAndId(id, make_simple_highlighter(func));
+    };
 }
 
 void register_highlighters()
 {
     HighlighterRegistry& registry = HighlighterRegistry::instance();
 
-    registry.register_func("number_lines", SimpleHighlighterFactory<show_line_numbers>("number_lines"));
-    registry.register_func("show_matching", SimpleHighlighterFactory<show_matching_char>("show_matching"));
-    registry.register_func("show_whitespaces", SimpleHighlighterFactory<show_whitespaces>("show_whitespaces"));
-    registry.register_func("fill", fill_factory);
-    registry.register_func("regex", highlight_regex_factory);
-    registry.register_func("regex_option", highlight_regex_option_factory);
-    registry.register_func("search", highlight_search_factory);
-    registry.register_func("group", highlighter_group_factory);
-    registry.register_func("flag_lines", flag_lines_factory);
-    registry.register_func("line_option", highlight_line_option_factory);
-    registry.register_func("ref", reference_factory);
-    registry.register_func("regions", regions_factory);
+    registry.register_func("number_lines", simple_factory("number_lines", show_line_numbers));
+    registry.register_func("show_matching", simple_factory("show_matching", show_matching_char));
+    registry.register_func("show_whitespaces", simple_factory("show_whitespaces", show_whitespaces));
+    registry.register_func("fill", create_fill_highlighter);
+    registry.register_func("regex", RegexHighlighter::create);
+    registry.register_func("regex_option", create_regex_option_highlighter);
+    registry.register_func("search", create_search_highlighter);
+    registry.register_func("group", create_highlighter_group);
+    registry.register_func("flag_lines", create_flag_lines_highlighter);
+    registry.register_func("line_option", create_line_option_highlighter);
+    registry.register_func("ref", create_reference_highlighter);
+    registry.register_func("regions", RegionsHighlighter::create);
 }
 
 }
diff --git a/src/utils.hh b/src/utils.hh
index e999f405..354c2fc3 100644
--- a/src/utils.hh
+++ b/src/utils.hh
@@ -11,6 +11,13 @@
 
 namespace Kakoune
 {
+
+template<typename T, typename... Args>
+std::unique_ptr<T> make_unique(Args&&... args)
+{
+    return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
+}
+
 // *** Singleton ***
 //
 // Singleton helper class, every singleton type T should inherit
diff --git a/src/window.cc b/src/window.cc
index e76dcc8d..258c4d46 100644
--- a/src/window.cc
+++ b/src/window.cc
@@ -28,9 +28,9 @@ Window::Window(Buffer& buffer)
     m_hooks.run_hook("WinCreate", buffer.name(), hook_handler.context());
     m_options.register_watcher(*this);
 
-    m_builtin_highlighters.append({"tabulations", expand_tabulations});
-    m_builtin_highlighters.append({"unprintable", expand_unprintable});
-    m_builtin_highlighters.append({"selections",  highlight_selections});
+    m_builtin_highlighters.add_child({"tabulations"_str, make_simple_highlighter(expand_tabulations)});
+    m_builtin_highlighters.add_child({"unprintable"_str, make_simple_highlighter(expand_unprintable)});
+    m_builtin_highlighters.add_child({"selections"_str,  make_simple_highlighter(highlight_selections)});
 
     for (auto& option : m_options.flatten_options())
         on_option_changed(*option);
@@ -82,8 +82,8 @@ void Window::update_display_buffer(const Context& context)
     }
 
     m_display_buffer.compute_range();
-    m_highlighters(context, HighlightFlags::Highlight, m_display_buffer);
-    m_builtin_highlighters(context, HighlightFlags::Highlight, m_display_buffer);
+    m_highlighters.highlight(context, HighlightFlags::Highlight, m_display_buffer);
+    m_builtin_highlighters.highlight(context, HighlightFlags::Highlight, m_display_buffer);
 
     // cut the start of the line before m_position.column
     for (auto& line : lines)
@@ -182,8 +182,8 @@ void Window::scroll_to_keep_selection_visible_ifn(const Context& context)
     lines.emplace_back(AtomList{ {buffer(), cursor.line, cursor.line+1} });
 
     display_buffer.compute_range();
-    m_highlighters(context, HighlightFlags::MoveOnly, display_buffer);
-    m_builtin_highlighters(context, HighlightFlags::MoveOnly, display_buffer);
+    m_highlighters.highlight(context, HighlightFlags::MoveOnly, display_buffer);
+    m_builtin_highlighters.highlight(context, HighlightFlags::MoveOnly, display_buffer);
 
     // now we can compute where the cursor is in display columns
     // (this is only valid if highlighting one line and multiple lines put
@@ -266,8 +266,8 @@ ByteCoordAndTarget Window::offset_coord(ByteCoordAndTarget coord, LineCount offs
 
     InputHandler hook_handler{{ *m_buffer, Selection{} } };
     hook_handler.context().set_window(*this);
-    m_highlighters(hook_handler.context(), HighlightFlags::MoveOnly, display_buffer);
-    m_builtin_highlighters(hook_handler.context(), HighlightFlags::MoveOnly, display_buffer);
+    m_highlighters.highlight(hook_handler.context(), HighlightFlags::MoveOnly, display_buffer);
+    m_builtin_highlighters.highlight(hook_handler.context(), HighlightFlags::MoveOnly, display_buffer);
 
     CharCount column = coord.target == -1 ? find_display_column(lines[0], buffer(), coord) : coord.target;
     return { find_buffer_coord(lines[1], buffer(), column), column };
diff --git a/src/window.hh b/src/window.hh
index 6e26c082..d1e821b6 100644
--- a/src/window.hh
+++ b/src/window.hh
@@ -36,7 +36,7 @@ public:
 
     CharCoord display_position(ByteCoord coord);
 
-    HighlighterGroup& highlighters() { return m_highlighters; }
+    Highlighter& highlighters() { return m_highlighters; }
 
     OptionManager&       options()       { return m_options; }
     const OptionManager& options() const { return m_options; }