Add a FilterGroup, and cleanup HighlighterGroup

FilterGroup is similar to HighlighterGroup, it permits grouping
together filters for easy removal.

src/commands.cc

@@ -393,15 +393,23 @@ void rm_highlighter(const CommandParameters& params, const Context& context)
 
 void add_filter(const CommandParameters& params, const Context& context)
 {
-    if (params.size() < 1)
+    ParametersParser parser(params, { { "group", true } });
+    if (parser.positional_count() < 1)
         throw wrong_argument_count();
-
     try
     {
         FilterRegistry& registry = FilterRegistry::instance();
-        FilterParameters filter_params(params.begin()+1, params.end());
+
-        registry.add_filter_to_window(context.window(), params[0],
+        auto begin = parser.begin();
-                                      filter_params);
+        const String& name = *begin;
+        std::vector<String> filter_params(++begin, parser.end());
+
+        Window& window = context.window();
+        FilterGroup& group = parser.has_option("group") ?
+           window.filters().get_group(parser.option_value("group"))
+         : window.filters();
+
+        registry.add_filter_to_group(group, name, filter_params);
     }
     catch (runtime_error& err)
     {
@@ -411,10 +419,22 @@ void add_filter(const CommandParameters& params, const Context& context)
 
 void rm_filter(const CommandParameters& params, const Context& context)
 {
-    if (params.size() != 1)
+    ParametersParser parser(params, { { "group", true } });
+    if (parser.positional_count() != 1)
         throw wrong_argument_count();
+    try
+    {
+        Window& window = context.window();
+        FilterGroup& group = parser.has_option("group") ?
+           window.filters().get_group(parser.option_value("group"))
+         : window.filters();
 
-    context.window().remove_filter(params[0]);
+        group.remove(*parser.begin());
+    }
+    catch (runtime_error& err)
+    {
+        print_status("error: " + err.description());
+    }
 }
 
 void add_hook(const CommandParameters& params, const Context& context)
@@ -855,15 +875,34 @@ void register_commands()
                              else
                                  return w.highlighters().complete_id(arg, pos_in_token);
                          });
-
-    PerArgumentCommandCompleter filter_completer({
-        [](const String& prefix, size_t cursor_pos)
-        { return FilterRegistry::instance().complete_filter(prefix, cursor_pos); }
-    });
     cm.register_commands({ "af", "addfilter" }, add_filter,
-                         CommandManager::None, filter_completer);
+                         CommandManager::None,
+                         [](const CommandParameters& params, size_t token_to_complete, size_t pos_in_token)
+                         {
+                             Window& w = main_context.window();
+                             const String& arg = token_to_complete < params.size() ?
+                                                      params[token_to_complete] : String();
+                             if (token_to_complete == 1 and params[0] == "-group")
+                                 return w.filters().complete_group_id(arg, pos_in_token);
+                             else if (token_to_complete == 0 or token_to_complete == 2 and params[0] == "-group")
+                                 return FilterRegistry::instance().complete_filter(arg, pos_in_token);
+                             else
+                                 return CandidateList();
+                         });
     cm.register_commands({ "rf", "rmfilter" }, rm_filter,
-                         CommandManager::None, filter_completer);
+                         CommandManager::None,
+                         [](const CommandParameters& params, size_t token_to_complete, size_t pos_in_token)
+                         {
+                             Window& w = main_context.window();
+                             const String& arg = token_to_complete < params.size() ?
+                                                      params[token_to_complete] : String();
+                             if (token_to_complete == 1 and params[0] == "-group")
+                                 return w.filters().complete_group_id(arg, pos_in_token);
+                             else if (token_to_complete == 2 and params[0] == "-group")
+                                 return w.filters().get_group(params[1]).complete_id(arg, pos_in_token);
+                             else
+                                 return w.filters().complete_id(arg, pos_in_token);
+                         });
 
     cm.register_command("hook", add_hook, CommandManager::IgnoreSemiColons | CommandManager::DeferredShellEval);
 

src/editor.cc

@@ -240,24 +240,6 @@ struct id_not_unique : public runtime_error
         : runtime_error("id not unique: " + id) {}
 };
 
-void Editor::add_filter(FilterAndId&& filter)
-{
-    if (m_filters.contains(filter.first))
-        throw id_not_unique(filter.first);
-    m_filters.append(std::forward<FilterAndId>(filter));
-}
-
-void Editor::remove_filter(const String& id)
-{
-    m_filters.remove(id);
-}
-
-CandidateList Editor::complete_filterid(const String& prefix,
-                                        size_t cursor_pos)
-{
-    return m_filters.complete_id<str_to_str>(prefix, cursor_pos);
-}
-
 void Editor::begin_edition()
 {
     ++m_edition_level;
@@ -327,8 +309,7 @@ IncrementalInserter::~IncrementalInserter()
 
 void IncrementalInserter::apply(Modification&& modification) const
 {
-    for (auto filter : m_editor.m_filters)
+    m_editor.filters()(m_editor.buffer(), modification);
-        filter.second(m_editor.buffer(), modification);
     m_editor.buffer().modify(std::move(modification));
 }
 

src/editor.hh

@@ -6,6 +6,7 @@
 #include "filter.hh"
 #include "idvaluemap.hh"
 #include "memoryview.hh"
+#include "filter_group.hh"
 
 namespace Kakoune
 {
@@ -54,8 +55,7 @@ public:
     bool undo();
     bool redo();
 
-    void add_filter(FilterAndId&& filter);
+    FilterGroup& filters() { return m_filters; }
-    void remove_filter(const String& id);
 
     CandidateList complete_filterid(const String& prefix,
                                     size_t cursor_pos = String::npos);
@@ -77,7 +77,7 @@ private:
 
     Buffer&                             m_buffer;
     std::vector<SelectionList>          m_selections;
-    idvaluemap<String, FilterFunc> m_filters;
+    FilterGroup                         m_filters;
 };
 
 struct scoped_edition

src/filter_group.cc

@@ -0,0 +1,56 @@
+#include "filter_group.hh"
+
+#include "exception.hh"
+#include "utils.hh"
+
+namespace Kakoune
+{
+
+void FilterGroup::operator()(Buffer& buffer, Modification& modification)
+{
+    for (auto& filter : m_filters)
+       filter.second(buffer, modification);
+}
+
+void FilterGroup::append(FilterAndId&& filter)
+{
+    if (m_filters.contains(filter.first))
+        throw runtime_error("duplicate filter id: " + filter.first);
+
+    m_filters.append(std::forward<FilterAndId>(filter));
+}
+
+void FilterGroup::remove(const String& id)
+{
+    m_filters.remove(id);
+}
+
+FilterGroup& FilterGroup::get_group(const String& id)
+{
+    auto it = m_filters.find(id);
+    if (it == m_filters.end())
+        throw runtime_error("no such id: " + id);
+    FilterGroup* group = it->second.target<FilterGroup>();
+    if (not group)
+        throw runtime_error("not a group: " + id);
+
+    return *group;
+}
+
+
+CandidateList FilterGroup::complete_id(const String& prefix,
+                                            size_t cursor_pos)
+{
+    return m_filters.complete_id<str_to_str>(prefix, cursor_pos);
+}
+
+CandidateList FilterGroup::complete_group_id(const String& prefix,
+                                                  size_t cursor_pos)
+{
+    return m_filters.complete_id_if<str_to_str>(
+        prefix, cursor_pos,
+        [](std::pair<String, FilterFunc>& func)
+        { return func.second.target<FilterGroup>() != nullptr; });
+}
+
+}

src/filter_group.hh

@@ -0,0 +1,31 @@
+#ifndef filter_group_hh_INCLUDED
+#define filter_group_hh_INCLUDED
+
+#include "filter.hh"
+#include "idvaluemap.hh"
+
+namespace Kakoune
+{
+
+// FilterGroup is an filter which delegate to multiple
+// other filters in order of insertion.
+class FilterGroup
+{
+public:
+    void operator()(Buffer& buffer, Modification& modification);
+
+    void append(FilterAndId&& filter);
+    void remove(const String& id);
+
+    FilterGroup& get_group(const String& id);
+
+    CandidateList complete_id(const String& prefix, size_t cursor_pos);
+    CandidateList complete_group_id(const String& prefix, size_t cursor_pos);
+
+private:
+    idvaluemap<String, FilterFunc> m_filters;
+};
+
+}
+
+#endif // filter_group_hh_INCLUDED

src/filter_registry.cc

@@ -1,7 +1,7 @@
 #include "filter_registry.hh"
 
 #include "exception.hh"
-#include "window.hh"
+#include "filter_group.hh"
 
 namespace Kakoune
 {
@@ -19,15 +19,15 @@ void FilterRegistry::register_factory(const String& name,
     m_factories.append(std::make_pair(name, factory));
 }
 
-void FilterRegistry::add_filter_to_window(Window& window,
+void FilterRegistry::add_filter_to_group(FilterGroup& group,
-                                          const String& name,
+                                         const String& name,
-                                          const FilterParameters& parameters)
+                                         const FilterParameters& parameters)
 {
     auto it = m_factories.find(name);
     if (it == m_factories.end())
         throw factory_not_found(name);
 
-    window.add_filter(it->second(window, parameters));
+    group.append(it->second(parameters));
 }
 
 CandidateList FilterRegistry::complete_filter(const String& prefix,

src/filter_registry.hh

@@ -13,12 +13,11 @@
 namespace Kakoune
 {
 
-class Window;
+class FilterGroup;
 
 typedef memoryview<String> FilterParameters;
 
-typedef std::function<FilterAndId (Window& window,
+typedef std::function<FilterAndId (const FilterParameters& params)> FilterFactory;
-                                   const FilterParameters& params)> FilterFactory;
 
 class FilterRegistry : public Singleton<FilterRegistry>
 {
@@ -26,9 +25,9 @@ public:
     void register_factory(const String& name,
                           const FilterFactory& factory);
 
-    void add_filter_to_window(Window& window,
+    void add_filter_to_group(FilterGroup& group,
-                              const String& factory_name,
+                             const String& factory_name,
-                              const FilterParameters& parameters);
+                             const FilterParameters& parameters);
 
     CandidateList complete_filter(const String& prefix,
                                   size_t cursor_pos);

src/filters.cc

@@ -1,6 +1,7 @@
 #include "filters.hh"
 #include "filter_registry.hh"
 #include "buffer.hh"
+#include "filter_group.hh"
 
 namespace Kakoune
 {
@@ -68,8 +69,7 @@ class SimpleFilterFactory
 public:
     SimpleFilterFactory(const String& id) : m_id(id) {}
 
-    FilterAndId operator()(Window& window,
+    FilterAndId operator()(const FilterParameters& params) const
-                           const FilterParameters& params) const
     {
         return FilterAndId(m_id, FilterFunc(filter_func));
     }
@@ -77,6 +77,14 @@ private:
     String m_id;
 };
 
+FilterAndId filter_group_factory(const FilterParameters& params)
+{
+    if (params.size() != 1)
+        throw runtime_error("wrong parameter count");
+
+    return FilterAndId(params[0], FilterGroup());
+}
+
 void register_filters()
 {
     FilterRegistry& registry = FilterRegistry::instance();
@@ -84,6 +92,7 @@ void register_filters()
     registry.register_factory("preserve_indent", SimpleFilterFactory<preserve_indent>("preserve_indent"));
     registry.register_factory("cleanup_whitespaces", SimpleFilterFactory<cleanup_whitespaces>("cleanup_whitespaces"));
     registry.register_factory("expand_tabulations", SimpleFilterFactory<expand_tabulations>("expand_tabulations"));
+    registry.register_factory("group", filter_group_factory);
 }
 
 }

src/highlighter_group.hh

@@ -10,6 +10,8 @@ namespace Kakoune
 class DisplayBuffer;
 class Window;
 
+// HighlighterGroup is an highlighter which delegate to multiple
+// other highlighters in order of insertion.
 class HighlighterGroup
 {
 public:

src/highlighter_registry.cc

@@ -20,17 +20,6 @@ void HighlighterRegistry::register_factory(const String& name,
     m_factories.append(std::make_pair(name, factory));
 }
 
-void HighlighterRegistry::add_highlighter_to_window(Window& window,
-                                                    const String& name,
-                                                    const HighlighterParameters& parameters)
-{
-    auto it = m_factories.find(name);
-    if (it == m_factories.end())
-        throw factory_not_found(name);
-
-    window.highlighters().append(it->second(window, parameters));
-}
-
 void HighlighterRegistry::add_highlighter_to_group(Window& window,
                                                    HighlighterGroup& group,
                                                    const String& name,

src/highlighter_registry.hh

@@ -24,10 +24,6 @@ public:
     void register_factory(const String& name,
                           const HighlighterFactory& factory);
 
-    void add_highlighter_to_window(Window& window,
-                                  const String& factory_name,
-                                  const HighlighterParameters& parameters);
-
     void add_highlighter_to_group(Window& window,
                                   HighlighterGroup& group,
                                   const String& factory_name,

src/window.cc

@@ -22,8 +22,8 @@ Window::Window(Buffer& buffer)
 
     m_hook_manager.run_hook("WinCreate", buffer.name(), Context(*this));
 
-    registry.add_highlighter_to_window(*this, "expand_tabs", HighlighterParameters());
+    registry.add_highlighter_to_group(*this, m_highlighters, "expand_tabs", HighlighterParameters());
-    registry.add_highlighter_to_window(*this, "highlight_selections", HighlighterParameters());
+    registry.add_highlighter_to_group(*this, m_highlighters, "highlight_selections", HighlighterParameters());
 }
 
 BufferIterator Window::iterator_at(const DisplayCoord& window_pos) const