Remove option checkers, handle that through the type system

Use a specific type for InsertCompleterDesc with checks in
the option_{from,to}_string functions

src/insert_completer.cc

@@ -16,6 +16,48 @@ namespace Kakoune
 
 using StringList = std::vector<String>;
 
+String option_to_string(const InsertCompleterDesc& opt)
+{
+    switch (opt.mode)
+    {
+        case InsertCompleterDesc::Word:
+            return "word=" + (opt.param ? *opt.param : "");
+        case InsertCompleterDesc::Filename:
+            return "filename";
+        case InsertCompleterDesc::Option:
+            return "option=" + (opt.param ? *opt.param : "");
+    }
+    kak_assert(false);
+    return "";
+}
+
+void option_from_string(const String& str, InsertCompleterDesc& opt)
+{
+    if (str.substr(0_byte, 7_byte) == "option=")
+    {
+        opt.mode = InsertCompleterDesc::Option;
+        opt.param = str.substr(7_byte).str();
+        return;
+    }
+    else if (str.substr(0_byte, 5_byte) == "word=")
+    {
+        auto param = str.substr(5_byte);
+        if (param == "all" or param == "buffer")
+        {
+            opt.mode = InsertCompleterDesc::Word;
+            opt.param = param.str();
+            return;
+        }
+    }
+    else if (str == "filename")
+    {
+        opt.mode = InsertCompleterDesc::Filename;
+        opt.param = Optional<String>{};
+        return;
+    }
+    throw runtime_error("invalid completer description: " + str);;
+}
+
 namespace
 {
 
@@ -272,26 +314,28 @@ bool InsertCompleter::setup_ifn()
     using namespace std::placeholders;
     if (not m_completions.is_valid())
     {
-        auto& completers = options()["completers"].get<StringList>();
+        auto& completers = options()["completers"].get<InsertCompleterDescList>();
         for (auto& completer : completers)
         {
-            if (completer == "filename" and
+            if (completer.mode == InsertCompleterDesc::Filename and
                 try_complete([this](const Buffer& buffer, ByteCoord cursor_pos) {
                     return complete_filename<true>(buffer, cursor_pos,
                                                    options());
                 }))
                 return true;
-            if (completer.substr(0_byte, 7_byte) == "option=" and
+            if (completer.mode == InsertCompleterDesc::Option and
                 try_complete([&,this](const Buffer& buffer, ByteCoord cursor_pos) {
                    return complete_option(buffer, cursor_pos,
-                                          options(), completer.substr(7_byte));
+                                          options(), *completer.param);
                 }))
                 return true;
-            if (completer == "word=buffer" and
+            if (completer.mode == InsertCompleterDesc::Word and
+                *completer.param == "buffer" and
                 (try_complete(complete_word<false, false>) or
                  try_complete(complete_word<false, true>)))
                 return true;
-            if (completer == "word=all" and
+            if (completer.mode == InsertCompleterDesc::Word and
+                *completer.param == "all" and
                 (try_complete(complete_word<true, false>) or
                  try_complete(complete_word<true, true>)))
                 return true;
@@ -323,12 +367,12 @@ void InsertCompleter::menu_show()
 
 void InsertCompleter::on_option_changed(const Option& opt)
 {
-    auto& completers = options()["completers"].get<StringList>();
+    auto& completers = options()["completers"].get<InsertCompleterDescList>();
-    StringList option_names;
+    std::vector<StringView> option_names;
     for (auto& completer : completers)
     {
-        if (completer.substr(0_byte, 7_byte) == "option=")
+        if (completer.mode == InsertCompleterDesc::Option)
-            option_names.emplace_back(completer.substr(7_byte));
+            option_names.emplace_back(*completer.param);
     }
     if (contains(option_names, opt.name()))
     {

src/insert_completer.hh

@@ -4,9 +4,41 @@
 #include "buffer.hh"
 #include "option_manager.hh"
 
+#include "optional.hh"
+
 namespace Kakoune
 {
 
+struct InsertCompleterDesc
+{
+    enum Mode
+    {
+        Word,
+        Option,
+        Filename
+    };
+
+    InsertCompleterDesc(Mode mode = Filename,
+                        Optional<String> param = Optional<String>{})
+        : mode{mode}, param{std::move(param)}
+    {}
+
+    bool operator==(const InsertCompleterDesc& other) const
+    { return mode == other.mode && param == other.param; }
+
+    bool operator!=(const InsertCompleterDesc& other) const
+    { return !(*this == other); }
+
+    Mode mode;
+    Optional<String> param;
+};
+
+using InsertCompleterDescList = std::vector<InsertCompleterDesc>;
+
+
+String option_to_string(const InsertCompleterDesc& opt);
+void option_from_string(const String& str, InsertCompleterDesc& opt);
+
 struct InsertCompletion
 {
     ByteCoord begin;

src/option_manager.cc

@@ -1,5 +1,7 @@
 #include "option_manager.hh"
 
+#include "insert_completer.hh"
+
 #include "assert.hh"
 
 #include <sstream>
@@ -159,18 +161,10 @@ GlobalOptions::GlobalOptions()
     declare_option("path", "path to consider when trying to find a file",
                    std::vector<String>({ "./", "/usr/include" }));
     declare_option("completers", "insert mode completers to execute.",
-                    std::vector<String>({"filename", "word=all"}),
+                    std::vector<InsertCompleterDesc>({
-                    OptionFlags::None,
+                        InsertCompleterDesc{ InsertCompleterDesc::Filename },
-                    OptionChecker<std::vector<String>>([](const std::vector<String>& s) {
+                        InsertCompleterDesc{ InsertCompleterDesc::Word, "all"_str }
-                        static const auto values = {"word=buffer", "word=all", "filename"};
+                    }), OptionFlags::None);
-                        for (auto& v : s)
-                        {
-                            if (v.substr(0_byte, 7_byte) == "option=")
-                                continue;
-                            if (not contains(values, v))
-                                throw runtime_error(v + " is not a recognised value for completers");
-                        }
-                    }));
     declare_option("autoreload",
                    "autoreload buffer when a filesystem modification is detected",
                     Ask);

src/option_manager.hh

@@ -117,23 +117,17 @@ private:
     std::vector<OptionManagerWatcher*> m_watchers;
 };
 
-template<typename T> using OptionChecker = std::function<void (const T&)>;
-
 template<typename T>
 class TypedOption : public Option
 {
 public:
-    TypedOption(OptionManager& manager, const OptionDesc& desc,
+    TypedOption(OptionManager& manager, const OptionDesc& desc, const T& value)
-                const T& value, OptionChecker<T> checker)
+        : Option(desc, manager), m_value(value) {}
-        : Option(desc, manager),
-          m_value(value), m_checker(std::move(checker)) {}
 
     void set(T value)
     {
         if (m_value != value)
         {
-            if (m_checker)
-                m_checker(value);
             m_value = std::move(value);
             manager().on_option_changed(*this);
         }
@@ -154,19 +148,16 @@ public:
     {
         T val;
         option_from_string(str, val);
-        if (m_checker)
-            m_checker(val);
         if (option_add(m_value, val))
             m_manager.on_option_changed(*this);
     }
 
     Option* clone(OptionManager& manager) const override
     {
-        return new TypedOption{manager, m_desc, m_value, m_checker};
+        return new TypedOption{manager, m_desc, m_value};
     }
 private:
     T m_value;
-    OptionChecker<T> m_checker;
 };
 
 template<typename T> const T& Option::get() const
@@ -206,8 +197,7 @@ public:
     template<typename T>
     Option& declare_option(const String& name, const String& docstring,
                            const T& value,
-                           OptionFlags flags = OptionFlags::None,
+                           OptionFlags flags = OptionFlags::None)
-                           OptionChecker<T> checker = OptionChecker<T>{})
     {
         auto it = find_option(m_options, name);
         if (it != m_options.end())
@@ -217,8 +207,7 @@ public:
             throw runtime_error("option " + name + " already declared with different type or flags");
         }
         m_descs.emplace_back(new OptionDesc{name, docstring, flags});
-        m_options.emplace_back(new TypedOption<T>{*this, *m_descs.back(),
+        m_options.emplace_back(new TypedOption<T>{*this, *m_descs.back(), value});
-                                                  value, std::move(checker)});
         return *m_options.back();
     }