From b548dd3a6f369e5a244fdcdca55061513026f82a Mon Sep 17 00:00:00 2001 From: Maxime Coste Date: Wed, 30 May 2018 23:23:38 +1000 Subject: [PATCH] Change option lists to be specified as separate arguments on commands line Option lists and maps are specified using separate arguments, avoiding the need for additional escaping of their separator and reusing the existing command line spliting logic instead. As discussed on #2087, this should make it much easier to work with list options, and make the general option system feel cleaner. --- src/command_manager.cc | 56 ++++++++++++++++------- src/commands.cc | 18 ++++---- src/completion.hh | 7 +-- src/option.hh | 27 +++++++++++ src/option_manager.hh | 23 +++++++--- src/option_types.cc | 19 ++++---- src/option_types.hh | 96 +++++++++++++++++++++++----------------- src/parameters_parser.hh | 7 +++ 8 files changed, 168 insertions(+), 85 deletions(-) diff --git a/src/command_manager.cc b/src/command_manager.cc index dd904bf4..c3dfc7c4 100644 --- a/src/command_manager.cc +++ b/src/command_manager.cc @@ -253,9 +253,31 @@ Token parse_percent_token(Reader& reader, bool throw_on_unterminated) } } -String expand_token(const Token& token, const Context& context, - const ShellContext& shell_context) +auto expand_option(Option& opt, std::true_type) { + return opt.get_as_string(); +} + +auto expand_option(Option& opt, std::false_type) +{ + return opt.get_as_strings(); +} + +String expand_arobase(ConstArrayView params, std::true_type) +{ + return join(params, ' ', false); +} + +Vector expand_arobase(ConstArrayView params, std::false_type) +{ + return {params.begin(), params.end()}; +} + +template +std::conditional_t> +expand_token(const Token& token, const Context& context, const ShellContext& shell_context) +{ + using IsSingle = std::integral_constant; auto& content = token.content; switch (token.type) { @@ -273,35 +295,35 @@ String expand_token(const Token& token, const Context& context, ++trailing_eol_count; } str.resize(str.length() - trailing_eol_count, 0); - return str; + return {str}; } case Token::Type::RegisterExpand: - return context.main_sel_register_value(content).str(); + return {context.main_sel_register_value(content).str()}; case Token::Type::OptionExpand: - return context.options()[content].get_as_string(); + return expand_option(context.options()[content], IsSingle{}); case Token::Type::ValExpand: { auto it = shell_context.env_vars.find(content); if (it != shell_context.env_vars.end()) - return it->value; - return ShellManager::instance().get_val(content, context); + return {it->value}; + return {ShellManager::instance().get_val(content, context)}; } case Token::Type::ArgExpand: { auto& params = shell_context.params; if (content == '@') - return join(params, ' '); + return expand_arobase(params, IsSingle{}); const int arg = str_to_int(content)-1; if (arg < 0) throw runtime_error("invalid argument index"); - return arg < params.size() ? params[arg] : String{}; + return {arg < params.size() ? params[arg] : String{}}; } case Token::Type::RawEval: - return expand(content, context, shell_context); + return {expand(content, context, shell_context)}; case Token::Type::Raw: case Token::Type::RawQuoted: - return content; + return {content}; default: kak_assert(false); } return {}; @@ -378,8 +400,7 @@ String expand_impl(StringView str, const Context& context, else { res += reader.substr_from(beg); - res += postprocess(expand_token(parse_percent_token(reader, true), - context, shell_context)); + res += postprocess(expand_token(parse_percent_token(reader, true), context, shell_context)); beg = reader.pos; } } @@ -483,7 +504,12 @@ void CommandManager::execute(StringView command_line, params.insert(params.end(), shell_context.params.begin(), shell_context.params.end()); else - params.push_back(expand_token(token, context, shell_context)); + { + auto tokens = expand_token(token, context, shell_context); + params.insert(params.end(), + std::make_move_iterator(tokens.begin()), + std::make_move_iterator(tokens.end())); + } } execute_single_command(params, context, shell_context, command_coord); } @@ -640,7 +666,7 @@ Completions CommandManager::complete(const Context& context, context, flags, params, tokens.size() - 2, cursor_pos_in_token), start); - if (token.type != Token::Type::RawQuoted) + if (not completions.quoted and token.type != Token::Type::RawQuoted) { StringView to_escape = token.type == Token::Type::Raw ? "% \t;" : "%"; for (auto& candidate : completions.candidates) diff --git a/src/commands.cc b/src/commands.cc index 8323aa56..7c840fb7 100644 --- a/src/commands.cc +++ b/src/commands.cc @@ -1318,7 +1318,7 @@ const CommandDesc set_option_cmd = { "scope the option is set in", ParameterDesc{ { { "add", { false, "add to option rather than replacing it" } } }, - ParameterDesc::Flags::SwitchesOnlyAtStart, 3, 3 + ParameterDesc::Flags::SwitchesOnlyAtStart, 2, (size_t)-1 }, CommandFlags::None, option_doc_helper, @@ -1337,13 +1337,11 @@ const CommandDesc set_option_cmd = { else if (token_to_complete == start + 1) return { 0_byte, params[start + 1].length(), GlobalScope::instance().option_registry().complete_option_name(params[start + 1], pos_in_token) }; - else if (not add and token_to_complete == start + 2 and + else if (not add and token_to_complete == start + 2 and params[start + 2].empty() and GlobalScope::instance().option_registry().option_exists(params[start + 1])) { OptionManager& options = get_scope(params[start], context).options(); - String val = options[params[start + 1]].get_as_string(); - if (prefix_match(val, params[start + 2])) - return { 0_byte, params[start + 2].length(), { std::move(val) } }; + return { 0_byte, params[start + 2].length(), { options[params[start + 1]].get_as_string() }, true }; } return Completions{}; }, @@ -1351,9 +1349,9 @@ const CommandDesc set_option_cmd = { { Option& opt = get_options(parser[0], context, parser[1]).get_local_option(parser[1]); if (parser.get_switch("add")) - opt.add_from_string(parser[2]); + opt.add_from_strings(parser.positionals_from(2)); else - opt.set_from_string(parser[2]); + opt.set_from_strings(parser.positionals_from(2)); } }; @@ -1427,7 +1425,7 @@ const CommandDesc declare_option_cmd = { ParameterDesc{ { { "hidden", { false, "do not display option name when completing" } }, { "docstring", { true, "specify option description" } } }, - ParameterDesc::Flags::SwitchesOnlyAtStart, 2, 3 + ParameterDesc::Flags::SwitchesOnlyAtStart, 2, (size_t)-1 }, CommandFlags::None, CommandHelper{}, @@ -1470,8 +1468,8 @@ const CommandDesc declare_option_cmd = { else throw runtime_error(format("no such option type: '{}'", parser[0])); - if (parser.positional_count() == 3) - opt->set_from_string(parser[2]); + if (parser.positional_count() > 2) + opt->set_from_strings(parser.positionals_from(2)); } }; diff --git a/src/completion.hh b/src/completion.hh index 19275824..553b37e7 100644 --- a/src/completion.hh +++ b/src/completion.hh @@ -21,6 +21,7 @@ struct Completions CandidateList candidates; ByteCount start; ByteCount end; + bool quoted = false; Completions() : start(0), end(0) {} @@ -28,8 +29,8 @@ struct Completions Completions(ByteCount start, ByteCount end) : start(start), end(end) {} - Completions(ByteCount start, ByteCount end, CandidateList candidates) - : candidates(std::move(candidates)), start(start), end(end) {} + Completions(ByteCount start, ByteCount end, CandidateList candidates, bool quoted = false) + : candidates(std::move(candidates)), start(start), end(end), quoted{quoted} {} }; enum class CompletionFlags @@ -56,7 +57,7 @@ Completions shell_complete(const Context& context, CompletionFlags, inline Completions offset_pos(Completions completion, ByteCount offset) { return { completion.start + offset, completion.end + offset, - std::move(completion.candidates) }; + std::move(completion.candidates), completion.quoted }; } template diff --git a/src/option.hh b/src/option.hh index f4aafe3a..19ca709f 100644 --- a/src/option.hh +++ b/src/option.hh @@ -16,6 +16,33 @@ inline String option_to_string(int opt); inline String option_to_string(size_t opt); inline String option_to_string(bool opt); +// Default fallback to single value functions +template +decltype(option_from_string(Meta::Type{}, StringView{})) +option_from_strings(Meta::Type, ConstArrayView strs) +{ + if (strs.size() != 1) + throw runtime_error("expected a single value for option"); + return option_from_string(Meta::Type{}, strs[0]); +} + +template +Vector()))> +option_to_strings(const T& opt) +{ + return Vector{option_to_string(opt)}; +} + +template +decltype(option_add(std::declval(), std::declval())) +option_add_from_strings(T& opt, ConstArrayView strs) +{ + if (strs.size() != 1) + throw runtime_error("expected a single value for option"); + return option_add(opt, strs[0]); +} + + template struct PrefixedList { diff --git a/src/option_manager.hh b/src/option_manager.hh index b1ecf4ef..b7de0e7e 100644 --- a/src/option_manager.hh +++ b/src/option_manager.hh @@ -54,9 +54,10 @@ public: template bool is_of_type() const; virtual String get_as_string() const = 0; - virtual void set_from_string(StringView str) = 0; - virtual void add_from_string(StringView str) = 0; - virtual void update(const Context& context) = 0; + virtual Vector get_as_strings() const = 0; + virtual void set_from_strings(ConstArrayView strs) = 0; + virtual void add_from_strings(ConstArrayView strs) = 0; + virtual void update(const Context& context) = 0; virtual Option* clone(OptionManager& manager) const = 0; OptionManager& manager() const { return m_manager; } @@ -141,19 +142,27 @@ public: const T& get() const { return m_value; } T& get_mutable() { return m_value; } + Vector get_as_strings() const override + { + return option_to_strings(m_value); + } + String get_as_string() const override { return option_to_string(m_value); } - void set_from_string(StringView str) override + + void set_from_strings(ConstArrayView strs) override { - set(option_from_string(Meta::Type{}, str)); + set(option_from_strings(Meta::Type{}, strs)); } - void add_from_string(StringView str) override + + void add_from_strings(ConstArrayView strs) override { - if (option_add(m_value, str)) + if (option_add_from_strings(m_value, strs)) m_manager.on_option_changed(*this); } + void update(const Context& context) override { option_update(m_value, context); diff --git a/src/option_types.cc b/src/option_types.cc index f32e361f..5cd4371c 100644 --- a/src/option_types.cc +++ b/src/option_types.cc @@ -5,19 +5,20 @@ namespace Kakoune { UnitTest test_option_parsing{[]{ - auto check = [](auto&& value, StringView str) + auto check = [](auto&& value, ConstArrayView strs) { - auto repr = option_to_string(value); - kak_assert(repr == str); - auto parsed = option_from_string(Meta::Type>{}, str); + auto repr = option_to_strings(value); + kak_assert(strs == ConstArrayView{repr}); + auto parsed = option_from_strings(Meta::Type>{}, strs); kak_assert(parsed == value); }; - check(123, "123"); - check(true, "true"); - check(Vector{"foo", "bar:", "baz"}, "foo:bar\\::baz"); - check(HashMap{{"foo", 10}, {"b=r", 20}, {"b:z", 30}}, "foo=10:b\\=r=20:b\\:z=30"); - check(DebugFlags::Keys | DebugFlags::Hooks, "hooks|keys"); + check(123, {"123"}); + check(true, {"true"}); + check(Vector{"foo", "bar:", "baz"}, {"foo", "bar:", "baz"}); + check(Vector{10, 20, 30}, {"10", "20", "30"}); + check(HashMap{{"foo", 10}, {"b=r", 20}, {"b:z", 30}}, {"foo=10", "b\\=r=20", "b:z=30"}); + check(DebugFlags::Keys | DebugFlags::Hooks, {"hooks|keys"}); }}; } diff --git a/src/option_types.hh b/src/option_types.hh index 889dedcc..886008b7 100644 --- a/src/option_types.hh +++ b/src/option_types.hh @@ -18,6 +18,12 @@ namespace Kakoune { + +inline String quote(StringView s) +{ + return format("'{}'", replace(s, "'", "''")); +} + template constexpr decltype(T::option_type_name) option_type_name(Meta::Type) { @@ -67,13 +73,16 @@ inline Codepoint option_from_string(Meta::Type, StringView str) } constexpr StringView option_type_name(Meta::Type) { return "codepoint"; } -constexpr char list_separator = ':'; +template +Vector option_to_strings(const Vector& opt) +{ + return opt | transform([](const T& t) { return option_to_string(t); }) | gather>(); +} template String option_to_string(const Vector& opt) { - return join(opt | transform([](const T& t) { return option_to_string(t); }), - list_separator); + return join(opt | transform([](const T& t) { return quote(option_to_string(t)); }), ' ', false); } template @@ -81,20 +90,18 @@ void option_list_postprocess(Vector& opt) {} template -Vector option_from_string(Meta::Type>, StringView str) +Vector option_from_strings(Meta::Type>, ConstArrayView strs) { - auto res = str | split(list_separator, '\\') - | transform(unescape) - | transform([](auto&& s) { return option_from_string(Meta::Type{}, s); }) - | gather>(); + auto res = strs | transform([](auto&& s) { return option_from_string(Meta::Type{}, s); }) + | gather>(); option_list_postprocess(res); return res; } template -bool option_add(Vector& opt, StringView str) +bool option_add_from_strings(Vector& opt, ConstArrayView strs) { - auto vec = option_from_string(Meta::Type>{}, str); + auto vec = option_from_strings(Meta::Type>{}, strs); opt.insert(opt.end(), std::make_move_iterator(vec.begin()), std::make_move_iterator(vec.end())); @@ -109,31 +116,35 @@ String option_type_name(Meta::Type>) } template -String option_to_string(const HashMap& opt) +Vector option_to_strings(const HashMap& opt) { - String res; - for (auto it = opt.begin(); it != opt.end(); ++it) - { - if (it != opt.begin()) - res += list_separator; - String elem = escape(option_to_string(it->key), '=', '\\') + "=" + - escape(option_to_string(it->value), '=', '\\'); - res += escape(elem, list_separator, '\\'); - } - return res; + return opt | transform([](auto&& item) { + return format("{}={}", + escape(option_to_string(item.key), '=', '\\'), + escape(option_to_string(item.value), '=', '\\')); + }) | gather>(); } template -bool option_add(HashMap& opt, StringView str) +String option_to_string(const HashMap& opt) +{ + return join(opt | transform([](auto&& item) { + return quote(format("{}={}", + escape(option_to_string(item.key), '=', '\\'), + escape(option_to_string(item.value), '=', '\\'))); + }), ' ', false); +} + +template +bool option_add_from_strings(HashMap& opt, ConstArrayView strs) { bool changed = false; - for (auto&& elem : str | split(list_separator, '\\') - | transform(unescape)) + for (auto&& str : strs) { struct error : runtime_error { error(size_t) : runtime_error{"map option expects key=value"} {} }; - auto key_value = elem | split('=', '\\') - | transform(unescape<'=', '\\'>) - | static_gather(); + auto key_value = str | split('=', '\\') + | transform(unescape<'=', '\\'>) + | static_gather(); opt[option_from_string(Meta::Type{}, key_value[0])] = option_from_string(Meta::Type{}, key_value[1]); changed = true; @@ -142,10 +153,10 @@ bool option_add(HashMap& opt, StringView str) } template -HashMap option_from_string(Meta::Type>, StringView str) +HashMap option_from_strings(Meta::Type>, ConstArrayView str) { HashMap res; - option_add(res, str); + option_add_from_strings(res, str); return res; } @@ -304,29 +315,32 @@ EnableIfWithBitOps option_add(Flags& opt, StringView str) return res != (Flags)0; } +template +inline Vector option_to_strings(const PrefixedList& opt) +{ + Vector res{option_to_string(opt.prefix)}; + auto list = option_to_strings(opt.list); + res.insert(res.end(), std::make_move_iterator(list.begin()), std::make_move_iterator(list.end())); + return res; +} + template inline String option_to_string(const PrefixedList& opt) { - if (opt.list.empty()) - return format("{}", escape(option_to_string(opt.prefix), list_separator, '\\')); - else - return format("{}{}{}", escape(option_to_string(opt.prefix), list_separator, '\\'), - list_separator, option_to_string(opt.list)); + return option_to_string(opt.prefix) + " " + option_to_string(opt.list); } template -inline PrefixedList option_from_string(Meta::Type>, StringView str) +inline PrefixedList option_from_strings(Meta::Type>, ConstArrayView strs) { - using VecType = Vector; - auto it = find(str, list_separator); - return {option_from_string(Meta::Type

{}, StringView{str.begin(), it}), - it != str.end() ? option_from_string(Meta::Type{}, {it+1, str.end()}) : VecType{}}; + return {option_from_string(Meta::Type

{}, strs[0]), + option_from_strings(Meta::Type>{}, strs.subrange(1))}; } template -inline bool option_add(PrefixedList& opt, StringView str) +inline bool option_add_from_strings(PrefixedList& opt, ConstArrayView str) { - return option_add(opt.list, str); + return option_add_from_strings(opt.list, str); } } diff --git a/src/parameters_parser.hh b/src/parameters_parser.hh index 8cbaf0fd..0f9f83dc 100644 --- a/src/parameters_parser.hh +++ b/src/parameters_parser.hh @@ -6,6 +6,7 @@ #include "meta.hh" #include "array_view.hh" #include "optional.hh" +#include "flags.hh" #include "string.hh" #include "string_utils.hh" @@ -115,6 +116,12 @@ struct ParametersParser return m_params[m_positional_indices[index]]; } + ConstArrayView positionals_from(size_t first) const + { + kak_assert(m_desc.flags & (ParameterDesc::Flags::SwitchesOnlyAtStart | ParameterDesc::Flags::SwitchesAsPositional)); + return m_params.subrange(m_positional_indices[first]); + } + iterator begin() const { return iterator(*this, 0); } iterator end() const { return iterator(*this, m_positional_indices.size()); }