From b5693c6253bd99af5d27488562f9598c1c45c85d Mon Sep 17 00:00:00 2001
From: Maxime Coste <mawww@kakoune.org>
Date: Sun, 27 May 2018 13:00:04 +1000
Subject: [PATCH] Refactor option_from_string to return directly the option
 value

---
 src/color.cc            |   4 +-
 src/color.hh            |   3 +-
 src/highlighters.cc     |   4 +-
 src/highlighters.hh     |   2 +-
 src/insert_completer.cc |  26 ++------
 src/insert_completer.hh |   2 +-
 src/option_manager.hh   |   4 +-
 src/option_types.cc     |   3 +-
 src/option_types.hh     | 131 +++++++++++++++++-----------------------
 src/regex.cc            |   4 +-
 src/regex.hh            |   2 +-
 src/string_utils.hh     |   2 +-
 12 files changed, 73 insertions(+), 114 deletions(-)

diff --git a/src/color.cc b/src/color.cc
index a8d04ee6..4373684d 100644
--- a/src/color.cc
+++ b/src/color.cc
@@ -81,9 +81,9 @@ String option_to_string(Color color)
     return to_string(color);
 }
 
-void option_from_string(StringView str, Color& color)
+Color option_from_string(Meta::Type<Color>, StringView str)
 {
-    color = str_to_color(str);
+    return str_to_color(str);
 }
 
 }
diff --git a/src/color.hh b/src/color.hh
index ae8046ab..d42b112b 100644
--- a/src/color.hh
+++ b/src/color.hh
@@ -2,6 +2,7 @@
 #define color_hh_INCLUDED
 
 #include "hash.hh"
+#include "meta.hh"
 
 namespace Kakoune
 {
@@ -59,7 +60,7 @@ Color str_to_color(StringView color);
 String to_string(Color color);
 
 String option_to_string(Color color);
-void option_from_string(StringView str, Color& color);
+Color option_from_string(Meta::Type<Color>, StringView str);
 
 bool is_color_name(StringView color);
 
diff --git a/src/highlighters.cc b/src/highlighters.cc
index 181b8fd5..71cfcbae 100644
--- a/src/highlighters.cc
+++ b/src/highlighters.cc
@@ -1379,7 +1379,7 @@ String option_to_string(InclusiveBufferRange range)
                   range.last.line+1, range.last.column+1);
 }
 
-void option_from_string(StringView str, InclusiveBufferRange& opt)
+InclusiveBufferRange option_from_string(Meta::Type<InclusiveBufferRange>, StringView str)
 {
     auto sep = find_if(str, [](char c){ return c == ',' or c == '+'; });
     auto dot_beg = find(StringView{str.begin(), sep}, '.');
@@ -1400,7 +1400,7 @@ void option_from_string(StringView str, InclusiveBufferRange& opt)
     if (first.line < 0 or first.column < 0 or last.line < 0 or last.column < 0)
         throw runtime_error("coordinates elements should be >= 1");
 
-    opt = { std::min(first, last), std::max(first, last) };
+    return { std::min(first, last), std::max(first, last) };
 }
 
 BufferCoord& get_first(RangeAndString& r) { return std::get<0>(r).first; }
diff --git a/src/highlighters.hh b/src/highlighters.hh
index 69979e32..c635b8df 100644
--- a/src/highlighters.hh
+++ b/src/highlighters.hh
@@ -17,7 +17,7 @@ inline bool operator==(const InclusiveBufferRange& lhs, const InclusiveBufferRan
     return lhs.first == rhs.first and lhs.last == rhs.last;
 }
 String option_to_string(InclusiveBufferRange range);
-void option_from_string(StringView str, InclusiveBufferRange& opt);
+InclusiveBufferRange option_from_string(Meta::Type<InclusiveBufferRange>, StringView str);
 
 using LineAndSpec = std::tuple<LineCount, String>;
 using LineAndSpecList = TimestampedList<LineAndSpec>;
diff --git a/src/insert_completer.cc b/src/insert_completer.cc
index 86b02e64..5d067ba0 100644
--- a/src/insert_completer.cc
+++ b/src/insert_completer.cc
@@ -40,39 +40,23 @@ String option_to_string(const InsertCompleterDesc& opt)
     return "";
 }
 
-void option_from_string(StringView str, InsertCompleterDesc& opt)
+InsertCompleterDesc option_from_string(Meta::Type<InsertCompleterDesc>, StringView str)
 {
     if (str.substr(0_byte, 7_byte) == "option=")
-    {
-        opt.mode = InsertCompleterDesc::Option;
-        opt.param = str.substr(7_byte).str();
-        return;
-    }
+        return {InsertCompleterDesc::Option, str.substr(7_byte).str()};
     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;
-        }
+            return {InsertCompleterDesc::Word, param.str()};
     }
     else if (str == "filename")
-    {
-        opt.mode = InsertCompleterDesc::Filename;
-        opt.param = Optional<String>{};
-        return;
-    }
+        return {InsertCompleterDesc::Filename, {}};
     else if (str.substr(0_byte, 5_byte) == "line=")
     {
         auto param = str.substr(5_byte);
         if (param == "all" or param == "buffer")
-        {
-            opt.mode = InsertCompleterDesc::Line;
-            opt.param = param.str();
-            return;
-        }
+            return {InsertCompleterDesc::Line, param.str()};
     }
     throw runtime_error(format("invalid completer description: '{}'", str));
 }
diff --git a/src/insert_completer.hh b/src/insert_completer.hh
index 7e79bd09..0f16e8aa 100644
--- a/src/insert_completer.hh
+++ b/src/insert_completer.hh
@@ -38,7 +38,7 @@ struct InsertCompleterDesc
 using InsertCompleterDescList = Vector<InsertCompleterDesc, MemoryDomain::Options>;
 
 String option_to_string(const InsertCompleterDesc& opt);
-void option_from_string(StringView str, InsertCompleterDesc& opt);
+InsertCompleterDesc option_from_string(Meta::Type<InsertCompleterDesc>, StringView str);
 
 inline StringView option_type_name(Meta::Type<InsertCompleterDesc>)
 {
diff --git a/src/option_manager.hh b/src/option_manager.hh
index f5d36ac0..b1ecf4ef 100644
--- a/src/option_manager.hh
+++ b/src/option_manager.hh
@@ -147,9 +147,7 @@ public:
     }
     void set_from_string(StringView str) override
     {
-        T val;
-        option_from_string(str, val);
-        set(std::move(val));
+        set(option_from_string(Meta::Type<T>{}, str));
     }
     void add_from_string(StringView str) override
     {
diff --git a/src/option_types.cc b/src/option_types.cc
index bd30d4f8..f32e361f 100644
--- a/src/option_types.cc
+++ b/src/option_types.cc
@@ -9,8 +9,7 @@ UnitTest test_option_parsing{[]{
     {
         auto repr = option_to_string(value);
         kak_assert(repr == str);
-        std::decay_t<decltype(value)> parsed;
-        option_from_string(str, parsed);
+        auto parsed = option_from_string(Meta::Type<std::decay_t<decltype(value)>>{}, str);
         kak_assert(parsed == value);
     };
 
diff --git a/src/option_types.hh b/src/option_types.hh
index 0617a183..c7b5d5f8 100644
--- a/src/option_types.hh
+++ b/src/option_types.hh
@@ -34,7 +34,7 @@ option_type_name(Meta::Type<Enum>)
 }
 
 inline String option_to_string(int opt) { return to_string(opt); }
-inline void option_from_string(StringView str, int& opt) { opt = str_to_int(str); }
+inline int option_from_string(Meta::Type<int>, StringView str) { return str_to_int(str); }
 inline bool option_add(int& opt, StringView str)
 {
     auto val = str_to_int(str);
@@ -44,26 +44,26 @@ inline bool option_add(int& opt, StringView str)
 constexpr StringView option_type_name(Meta::Type<int>) { return "int"; }
 
 inline String option_to_string(size_t opt) { return to_string(opt); }
-inline void option_from_string(StringView str, size_t& opt) { opt = str_to_int(str); }
+inline size_t option_from_string(Meta::Type<size_t>, StringView str) { return str_to_int(str); }
 
 inline String option_to_string(bool opt) { return opt ? "true" : "false"; }
-inline void option_from_string(StringView str, bool& opt)
+inline bool option_from_string(Meta::Type<bool>, StringView str)
 {
     if (str == "true" or str == "yes")
-        opt = true;
+        return true;
     else if (str == "false" or str == "no")
-        opt = false;
+        return false;
     else
         throw runtime_error("boolean values are either true, yes, false or no");
 }
 constexpr StringView option_type_name(Meta::Type<bool>) { return "bool"; }
 
 inline String option_to_string(Codepoint opt) { return to_string(opt); }
-inline void option_from_string(StringView str, Codepoint& opt)
+inline Codepoint option_from_string(Meta::Type<Codepoint>, StringView str)
 {
     if (str.char_length() != 1)
         throw runtime_error{format("'{}' is not a single codepoint", str)};
-    opt = str[0_char];
+    return str[0_char];
 }
 constexpr StringView option_type_name(Meta::Type<Codepoint>) { return "codepoint"; }
 
@@ -81,24 +81,20 @@ void option_list_postprocess(Vector<T, domain>& opt)
 {}
 
 template<typename T, MemoryDomain domain>
-void option_from_string(StringView str, Vector<T, domain>& opt)
+Vector<T, domain> option_from_string(Meta::Type<Vector<T, domain>>, StringView str)
 {
-    opt.clear();
-    for (auto&& elem : str | split<StringView>(list_separator, '\\')
-                           | transform(unescape<list_separator, '\\'>))
-    {
-        T opt_elem;
-        option_from_string(elem, opt_elem);
-        opt.push_back(opt_elem);
-    }
-    option_list_postprocess(opt);
+    auto res =  str | split<StringView>(list_separator, '\\')
+                    | transform(unescape<list_separator, '\\'>)
+                    | transform([](auto&& s) { return option_from_string(Meta::Type<T>{}, s); })
+                    | gather<Vector<T, domain>>();
+    option_list_postprocess(res);
+    return res;
 }
 
 template<typename T, MemoryDomain domain>
 bool option_add(Vector<T, domain>& opt, StringView str)
 {
-    Vector<T, domain> vec;
-    option_from_string(str, vec);
+    auto vec = option_from_string(Meta::Type<Vector<T, domain>>{}, str);
     opt.insert(opt.end(),
                std::make_move_iterator(vec.begin()),
                std::make_move_iterator(vec.end()));
@@ -139,20 +135,18 @@ bool option_add(HashMap<Key, Value, domain>& opt, StringView str)
                               | transform(unescape<'=', '\\'>)
                               | static_gather<error, 2>();
 
-        HashItem<Key, Value> item;
-        option_from_string(key_value[0], item.key);
-        option_from_string(key_value[1], item.value);
-        opt[std::move(item.key)] = std::move(item.value);
+        opt[option_from_string(Meta::Type<Key>{}, key_value[0])] = option_from_string(Meta::Type<Value>{}, key_value[1]);
         changed = true;
     }
     return changed;
 }
 
 template<typename Key, typename Value, MemoryDomain domain>
-void option_from_string(StringView str, HashMap<Key, Value, domain>& opt)
+HashMap<Key, Value, domain> option_from_string(Meta::Type<HashMap<Key, Value, domain>>, StringView str)
 {
-    opt.clear();
-    option_add(opt, str);
+    HashMap<Key, Value, domain> res;
+    option_add(res, str);
+    return res;
 }
 
 template<typename K, typename V, MemoryDomain D>
@@ -164,44 +158,21 @@ String option_type_name(Meta::Type<HashMap<K, V, D>>)
 
 constexpr char tuple_separator = '|';
 
-template<size_t I, typename... Types>
-struct TupleOptionDetail
+template<typename... Types, size_t... I>
+String option_to_string_impl(const std::tuple<Types...>& opt, std::index_sequence<I...>)
 {
-    static String to_string(const std::tuple<Types...>& opt)
-    {
-        return TupleOptionDetail<I-1, Types...>::to_string(opt) +
-               tuple_separator + escape(option_to_string(std::get<I>(opt)), tuple_separator, '\\');
-    }
-
-    static void from_string(ConstArrayView<String> elems, std::tuple<Types...>& opt)
-    {
-        option_from_string(elems[I], std::get<I>(opt));
-        TupleOptionDetail<I-1, Types...>::from_string(elems, opt);
-    }
-};
-
-template<typename... Types>
-struct TupleOptionDetail<0, Types...>
-{
-    static String to_string(const std::tuple<Types...>& opt)
-    {
-        return option_to_string(std::get<0>(opt));
-    }
-
-    static void from_string(ConstArrayView<String> elems, std::tuple<Types...>& opt)
-    {
-        option_from_string(elems[0], std::get<0>(opt));
-    }
-};
+    return join(make_array({option_to_string(std::get<I>(opt))...}), tuple_separator);
+}
 
 template<typename... Types>
 String option_to_string(const std::tuple<Types...>& opt)
 {
-    return TupleOptionDetail<sizeof...(Types)-1, Types...>::to_string(opt);
+    return option_to_string_impl(opt, std::make_index_sequence<sizeof...(Types)>());
 }
 
-template<typename... Types>
-void option_from_string(StringView str, std::tuple<Types...>& opt)
+template<typename... Types, size_t... I>
+std::tuple<Types...> option_from_string_impl(Meta::Type<std::tuple<Types...>>, StringView str,
+                                             std::index_sequence<I...>)
 {
     struct error : runtime_error
     {
@@ -212,8 +183,14 @@ void option_from_string(StringView str, std::tuple<Types...>& opt)
     auto elems = str | split<StringView>(tuple_separator, '\\')
                      | transform(unescape<tuple_separator, '\\'>)
                      | static_gather<error, sizeof...(Types)>();
+    return {option_from_string(Meta::Type<Types>{}, elems[I])...};
+}
 
-    TupleOptionDetail<sizeof...(Types)-1, Types...>::from_string(elems, opt);
+template<typename... Types>
+std::tuple<Types...> option_from_string(Meta::Type<std::tuple<Types...>>, StringView str)
+{
+    return option_from_string_impl(Meta::Type<std::tuple<Types...>>{}, str,
+                                   std::make_index_sequence<sizeof...(Types)>());
 }
 
 template<typename RealType, typename ValueType>
@@ -222,15 +199,15 @@ inline String option_to_string(const StronglyTypedNumber<RealType, ValueType>& o
     return to_string(opt);
 }
 
-template<typename RealType, typename ValueType>
-inline void option_from_string(StringView str, StronglyTypedNumber<RealType, ValueType>& opt)
+template<typename Number>
+std::enable_if_t<std::is_base_of<StronglyTypedNumber<Number, int>, Number>::value, Number>
+option_from_string(Meta::Type<Number>, StringView str)
 {
-     opt = StronglyTypedNumber<RealType, ValueType>{str_to_int(str)};
+     return Number{str_to_int(str)};
 }
 
 template<typename RealType, typename ValueType>
-inline bool option_add(StronglyTypedNumber<RealType, ValueType>& opt,
-                       StringView str)
+inline bool option_add(StronglyTypedNumber<RealType, ValueType>& opt, StringView str)
 {
     int val = str_to_int(str);
     opt += val;
@@ -251,14 +228,14 @@ inline void option_update(WorstMatch, const Context&)
     throw runtime_error("no update operation supported for this option type");
 }
 
-template<typename EffectiveType, typename LineType, typename ColumnType>
-inline void option_from_string(StringView str, LineAndColumn<EffectiveType, LineType, ColumnType>& opt)
+template<typename Coord>
+std::enable_if_t<std::is_base_of<LineAndColumn<Coord, decltype(Coord::line), decltype(Coord::column)>, Coord>::value, Coord>
+option_from_string(Meta::Type<Coord>, StringView str)
 {
     struct error : runtime_error { error(size_t) : runtime_error{"expected <line>,<column>"} {} };
     auto vals = str | split<StringView>(',')
                     | static_gather<error, 2>();
-    opt.line = str_to_int(vals[0]);
-    opt.column = str_to_int(vals[1]);
+    return {str_to_int(vals[0]), str_to_int(vals[1])};
 }
 
 template<typename EffectiveType, typename LineType, typename ColumnType>
@@ -295,10 +272,10 @@ EnableIfWithoutBitOps<Enum, String> option_to_string(Enum e)
 }
 
 template<typename Flags, typename = decltype(enum_desc(Meta::Type<Flags>{}))>
-EnableIfWithBitOps<Flags> option_from_string(StringView str, Flags& flags)
+EnableIfWithBitOps<Flags, Flags> option_from_string(Meta::Type<Flags>, StringView str)
 {
     constexpr auto desc = enum_desc(Meta::Type<Flags>{});
-    flags = Flags{};
+    Flags flags{};
     for (auto s : str | split<StringView>('|'))
     {
         auto it = find_if(desc, [s](const EnumDesc<Flags>& d) { return d.name == s; });
@@ -306,23 +283,23 @@ EnableIfWithBitOps<Flags> option_from_string(StringView str, Flags& flags)
             throw runtime_error(format("invalid flag value '{}'", s));
         flags |= it->value;
     }
+    return flags;
 }
 
 template<typename Enum, typename = decltype(enum_desc(Meta::Type<Enum>{}))>
-EnableIfWithoutBitOps<Enum> option_from_string(StringView str, Enum& e)
+EnableIfWithoutBitOps<Enum, Enum> option_from_string(Meta::Type<Enum>, StringView str)
 {
     constexpr auto desc = enum_desc(Meta::Type<Enum>{});
     auto it = find_if(desc, [str](const EnumDesc<Enum>& d) { return d.name == str; });
     if (it == desc.end())
         throw runtime_error(format("invalid enum value '{}'", str));
-    e = it->value;
+    return it->value;
 }
 
 template<typename Flags, typename = decltype(enum_desc(Meta::Type<Flags>{}))>
 EnableIfWithBitOps<Flags, bool> option_add(Flags& opt, StringView str)
 {
-    Flags res = Flags{};
-    option_from_string(str, res);
+    const Flags res = option_from_string(Meta::Type<Flags>{}, str);
     opt |= res;
     return res != (Flags)0;
 }
@@ -338,12 +315,12 @@ inline String option_to_string(const PrefixedList<P, T>& opt)
 }
 
 template<typename P, typename T>
-inline void option_from_string(StringView str, PrefixedList<P, T>& opt)
+inline PrefixedList<P, T> option_from_string(Meta::Type<PrefixedList<P, T>>, StringView str)
 {
+    using VecType = Vector<T, MemoryDomain::Options>;
     auto it = find(str, list_separator);
-    option_from_string(StringView{str.begin(), it}, opt.prefix);
-    if (it != str.end())
-        option_from_string({it+1, str.end()}, opt.list);
+    return {option_from_string(Meta::Type<P>{}, StringView{str.begin(), it}), 
+            it != str.end() ? option_from_string(Meta::Type<VecType>{}, {it+1, str.end()}) : VecType{}};
 }
 
 template<typename P, typename T>
diff --git a/src/regex.cc b/src/regex.cc
index 3967cebc..970721c7 100644
--- a/src/regex.cc
+++ b/src/regex.cc
@@ -13,9 +13,9 @@ String option_to_string(const Regex& re)
     return re.str();
 }
 
-void option_from_string(StringView str, Regex& re)
+Regex option_from_string(Meta::Type<Regex>, StringView str)
 {
-    re = Regex{str};
+    return Regex{str};
 }
 
 }
diff --git a/src/regex.hh b/src/regex.hh
index 531c258b..aac66921 100644
--- a/src/regex.hh
+++ b/src/regex.hh
@@ -161,7 +161,7 @@ bool backward_regex_search(It begin, It end, It subject_begin, It subject_end,
 }
 
 String option_to_string(const Regex& re);
-void option_from_string(StringView str, Regex& re);
+Regex option_from_string(Meta::Type<Regex>, StringView str);
 
 template<typename Iterator, MatchDirection direction = MatchDirection::Forward>
 struct RegexIterator
diff --git a/src/string_utils.hh b/src/string_utils.hh
index 6f0124a1..bd803536 100644
--- a/src/string_utils.hh
+++ b/src/string_utils.hh
@@ -66,7 +66,7 @@ int str_to_int(StringView str); // throws on error
 Optional<int> str_to_int_ifp(StringView str);
 
 inline String option_to_string(StringView opt) { return opt.str(); }
-inline void option_from_string(StringView str, String& opt) { opt = str.str(); }
+inline String option_from_string(Meta::Type<String>, StringView str) { return str.str(); }
 inline bool option_add(String& opt, StringView val) { opt += val; return not val.empty(); }
 
 template<size_t N>