From 68aba9e353961ccae17183ccdc65c3c89bdcd6c6 Mon Sep 17 00:00:00 2001 From: Maxime Coste Date: Mon, 27 Aug 2018 21:52:57 +1000 Subject: [PATCH] Use shell specific quoting for env vars Add a test case to validate roundtrips between Kakoune and the shell. --- src/buffer.cc | 2 +- src/command_manager.cc | 4 +-- src/commands.cc | 5 +-- src/main.cc | 69 +++++++++++++++++++------------------- src/option_manager.hh | 6 ++-- src/option_types.hh | 25 +++++++++----- src/scope.cc | 2 +- src/shell_manager.cc | 6 ++-- src/shell_manager.hh | 5 +-- src/string_utils.hh | 17 ++++++++++ src/window.cc | 2 +- test/shell/list-syntax/cmd | 1 + test/shell/list-syntax/in | 1 + test/shell/list-syntax/out | 4 +++ test/shell/list-syntax/rc | 7 ++++ 15 files changed, 98 insertions(+), 58 deletions(-) create mode 100644 test/shell/list-syntax/cmd create mode 100644 test/shell/list-syntax/in create mode 100644 test/shell/list-syntax/out create mode 100644 test/shell/list-syntax/rc diff --git a/src/buffer.cc b/src/buffer.cc index 3b636734..8ef76a09 100644 --- a/src/buffer.cc +++ b/src/buffer.cc @@ -688,7 +688,7 @@ void Buffer::on_option_changed(const Option& option) m_flags &= ~Flags::ReadOnly; } run_hook_in_own_context("BufSetOption", - format("{}={}", option.name(), option.get_as_string())); + format("{}={}", option.name(), option.get_as_string(Quoting::Kakoune))); } void Buffer::run_hook_in_own_context(StringView hook_name, StringView param, String client_name) diff --git a/src/command_manager.cc b/src/command_manager.cc index 7537f450..42e7f802 100644 --- a/src/command_manager.cc +++ b/src/command_manager.cc @@ -253,7 +253,7 @@ Token parse_percent_token(Reader& reader, bool throw_on_unterminated) auto expand_option(Option& opt, std::true_type) { - return opt.get_as_string(); + return opt.get_as_string(Quoting::Kakoune); } auto expand_option(Option& opt, std::false_type) @@ -314,7 +314,7 @@ expand_token(const Token& token, const Context& context, const ShellContext& she 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 {ShellManager::instance().get_val(content, context, Quoting::Kakoune)}; } case Token::Type::ArgExpand: { diff --git a/src/commands.cc b/src/commands.cc index a63bc71a..4db5ee30 100644 --- a/src/commands.cc +++ b/src/commands.cc @@ -1223,7 +1223,8 @@ const CommandDesc debug_cmd = { { write_to_debug_buffer("Options:"); for (auto& option : context.options().flatten_options()) - write_to_debug_buffer(format(" * {}: {}", option->name(), option->get_as_string())); + write_to_debug_buffer(format(" * {}: {}", option->name(), + option->get_as_string(Quoting::Kakoune))); } else if (parser[0] == "memory") { @@ -1366,7 +1367,7 @@ const CommandDesc set_option_cmd = { GlobalScope::instance().option_registry().option_exists(params[start + 1])) { OptionManager& options = get_scope(params[start], context).options(); - return { 0_byte, params[start + 2].length(), { options[params[start + 1]].get_as_string() }, true }; + return { 0_byte, params[start + 2].length(), { options[params[start + 1]].get_as_string(Quoting::Kakoune) }, true }; } return Completions{}; }, diff --git a/src/main.cc b/src/main.cc index 15ece332..fbc9fb67 100644 --- a/src/main.cc +++ b/src/main.cc @@ -113,131 +113,132 @@ String config_directory() static const EnvVarDesc builtin_env_vars[] = { { "bufname", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return context.buffer().display_name(); } }, { "buffile", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return context.buffer().name(); } }, { "buflist", false, - [](StringView name, const Context& context) + [](StringView name, const Context& context, Quoting quoting) { return join(BufferManager::instance() | - transform(&Buffer::display_name) | transform(quote), ' ', false); } + transform(&Buffer::display_name) | transform(quoter(quoting)), ' ', false); } }, { "buf_line_count", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return to_string(context.buffer().line_count()); } }, { "timestamp", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return to_string(context.buffer().timestamp()); } }, { "history_id", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return to_string((size_t)context.buffer().current_history_id()); } }, { "selection", false, - [](StringView name, const Context& context) + [](StringView name, const Context& context, Quoting quoting) { const Selection& sel = context.selections().main(); return content(context.buffer(), sel); } }, { "selections", false, - [](StringView name, const Context& context) - { return join(context.selections_content() | transform(quote), ' ', false); } + [](StringView name, const Context& context, Quoting quoting) + { return join(context.selections_content() | transform(quoter(quoting)), ' ', false); } }, { "runtime", false, - [](StringView name, const Context& context) + [](StringView name, const Context& context, Quoting quoting) { return runtime_directory(); } }, { "config", false, - [](StringView name, const Context& context) + [](StringView name, const Context& context, Quoting quoting) { return config_directory(); } }, { "version", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return version; } }, { "opt_", true, - [](StringView name, const Context& context) - { return context.options()[name.substr(4_byte)].get_as_string(); } + [](StringView name, const Context& context, Quoting quoting) + { return context.options()[name.substr(4_byte)].get_as_string(quoting); } }, { "main_reg_", true, - [](StringView name, const Context& context) + [](StringView name, const Context& context, Quoting quoting) { return context.main_sel_register_value(name.substr(9_byte)).str(); } }, { "reg_", true, - [](StringView name, const Context& context) - { return join(RegisterManager::instance()[name.substr(4_byte)].get(context) | transform(quote), ' ', false); } + [](StringView name, const Context& context, Quoting quoting) + { return join(RegisterManager::instance()[name.substr(4_byte)].get(context) | + transform(quoter(quoting)), ' ', false); } }, { "client_env_", true, - [](StringView name, const Context& context) + [](StringView name, const Context& context, Quoting quoting) { return context.client().get_env_var(name.substr(11_byte)).str(); } }, { "session", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return Server::instance().session(); } }, { "client", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return context.name(); } }, { "client_pid", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return to_string(context.client().pid()); } }, { "client_list", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return join(ClientManager::instance() | transform([](const std::unique_ptr& c) -> const String& { return c->context().name(); }), ' ', false); } }, { "modified", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return context.buffer().is_modified() ? "true" : "false"; } }, { "cursor_line", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return to_string(context.selections().main().cursor().line + 1); } }, { "cursor_column", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return to_string(context.selections().main().cursor().column + 1); } }, { "cursor_char_value", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { auto coord = context.selections().main().cursor(); auto& buffer = context.buffer(); return to_string((size_t)utf8::codepoint(buffer.iterator_at(coord), buffer.end())); } }, { "cursor_char_column", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { auto coord = context.selections().main().cursor(); return to_string(context.buffer()[coord.line].char_count_to(coord.column) + 1); } }, { "cursor_byte_offset", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { auto cursor = context.selections().main().cursor(); return to_string(context.buffer().distance({0,0}, cursor)); } }, { "selection_desc", false, - [](StringView name, const Context& context) + [](StringView name, const Context& context, Quoting quoting) { return selection_to_string(context.selections().main()); } }, { "selections_desc", false, - [](StringView name, const Context& context) + [](StringView name, const Context& context, Quoting quoting) { return selection_list_to_string(context.selections()); } }, { "window_width", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return to_string(context.window().dimensions().column); } }, { "window_height", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return to_string(context.window().dimensions().line); } }, { "user_modes", false, - [](StringView name, const Context& context) -> String + [](StringView name, const Context& context, Quoting quoting) -> String { return join(context.keymaps().user_modes(), ' ', false); } } }; diff --git a/src/option_manager.hh b/src/option_manager.hh index b7de0e7e..6c9db2ff 100644 --- a/src/option_manager.hh +++ b/src/option_manager.hh @@ -53,7 +53,7 @@ public: template void set(const T& val, bool notify=true); template bool is_of_type() const; - virtual String get_as_string() const = 0; + virtual String get_as_string(Quoting quoting) const = 0; virtual Vector get_as_strings() const = 0; virtual void set_from_strings(ConstArrayView strs) = 0; virtual void add_from_strings(ConstArrayView strs) = 0; @@ -147,9 +147,9 @@ public: return option_to_strings(m_value); } - String get_as_string() const override + String get_as_string(Quoting quoting) const override { - return option_to_string(m_value); + return option_to_string(m_value, quoting); } void set_from_strings(ConstArrayView strs) override diff --git a/src/option_types.hh b/src/option_types.hh index 0cd7ef7e..7d062deb 100644 --- a/src/option_types.hh +++ b/src/option_types.hh @@ -18,6 +18,13 @@ namespace Kakoune { +template +std::enable_if_t())), String>::value, String> +option_to_string(const T& value, Quoting) +{ + return option_to_string(value); +} + template constexpr bool option_needs_quoting(Meta::Type type, Meta::Type... rest) { @@ -25,10 +32,10 @@ constexpr bool option_needs_quoting(Meta::Type type, Meta::Type... rest } template -String quote_ifn(String str) +String quote_ifn(String str, Quoting quoting) { if (option_needs_quoting(Meta::Type{}...)) - return quote(std::move(str)); + return quoter(quoting)(std::move(str)); return str; } @@ -89,9 +96,9 @@ Vector option_to_strings(const Vector& opt) } template -String option_to_string(const Vector& opt) +String option_to_string(const Vector& opt, Quoting quoting) { - return join(opt | transform([](const T& t) { return quote_ifn(option_to_string(t)); }), ' ', false); + return join(opt | transform([=](const T& t) { return quote_ifn(option_to_string(t), quoting); }), ' ', false); } template @@ -135,13 +142,13 @@ Vector option_to_strings(const HashMap& opt) } template -String option_to_string(const HashMap& opt) +String option_to_string(const HashMap& opt, Quoting quoting) { - return join(opt | transform([](auto&& item) { + return join(opt | transform([=](auto&& item) { return quote_ifn( format("{}={}", escape(option_to_string(item.key), '=', '\\'), - escape(option_to_string(item.value), '=', '\\'))); + escape(option_to_string(item.value), '=', '\\')), quoting); }), ' ', false); } @@ -335,9 +342,9 @@ inline Vector option_to_strings(const PrefixedList& opt) } template -inline String option_to_string(const PrefixedList& opt) +inline String option_to_string(const PrefixedList& opt, Quoting quoting) { - return option_to_string(opt.prefix) + " " + option_to_string(opt.list); + return option_to_string(opt.prefix, quoting) + " " + option_to_string(opt.list, quoting); } template diff --git a/src/scope.cc b/src/scope.cc index f2879356..2930be97 100644 --- a/src/scope.cc +++ b/src/scope.cc @@ -19,7 +19,7 @@ void GlobalScope::on_option_changed(const Option& option) { Context empty_context{Context::EmptyContextFlag{}}; hooks().run_hook("GlobalSetOption", - format("{}={}", option.name(), option.get_as_string()), + format("{}={}", option.name(), option.get_as_string(Quoting::Kakoune)), empty_context); } diff --git a/src/shell_manager.cc b/src/shell_manager.cc index b92edb4c..3683c85f 100644 --- a/src/shell_manager.cc +++ b/src/shell_manager.cc @@ -138,7 +138,7 @@ Vector generate_env(StringView cmdline, const Context& context, const Sh try { const String& value = var_it != shell_context.env_vars.end() ? - var_it->value : ShellManager::instance().get_val(name, context); + var_it->value : ShellManager::instance().get_val(name, context, Quoting::Shell); kak_env.push_back(format("kak_{}={}", name, value)); } catch (runtime_error&) {} @@ -312,7 +312,7 @@ std::pair ShellManager::eval( return { std::move(stdout_contents), WIFEXITED(status) ? WEXITSTATUS(status) : -1 }; } -String ShellManager::get_val(StringView name, const Context& context) const +String ShellManager::get_val(StringView name, const Context& context, Quoting quoting) const { auto env_var = find_if(m_env_vars, [name](const EnvVarDesc& desc) { return desc.prefix ? prefix_match(name, desc.str) : name == desc.str; @@ -321,7 +321,7 @@ String ShellManager::get_val(StringView name, const Context& context) const if (env_var == m_env_vars.end()) throw runtime_error("no such env var: " + name); - return env_var->func(name, context); + return env_var->func(name, context, quoting); } CandidateList ShellManager::complete_env_var(StringView prefix, diff --git a/src/shell_manager.hh b/src/shell_manager.hh index f97935d1..ebfe3cca 100644 --- a/src/shell_manager.hh +++ b/src/shell_manager.hh @@ -18,10 +18,11 @@ struct ShellContext EnvVarMap env_vars; }; +enum class Quoting; struct EnvVarDesc { - using Retriever = String (*)(StringView name, const Context&); + using Retriever = String (*)(StringView name, const Context&, Quoting quoting); StringView str; bool prefix; @@ -45,7 +46,7 @@ public: Flags flags = Flags::WaitForStdout, const ShellContext& shell_context = {}); - String get_val(StringView name, const Context& context) const; + String get_val(StringView name, const Context& context, Quoting quoting) const; CandidateList complete_env_var(StringView prefix, ByteCount cursor_pos) const; diff --git a/src/string_utils.hh b/src/string_utils.hh index 558ba62f..92714a0d 100644 --- a/src/string_utils.hh +++ b/src/string_utils.hh @@ -135,6 +135,23 @@ inline String quote(StringView s) return format("'{}'", double_up(s, "'")); } +inline String shell_quote(StringView s) +{ + return format("'{}'", replace(s, "'", R"('\'')")); +} + +enum class Quoting +{ + Kakoune, + Shell +}; + +inline auto quoter(Quoting quoting) +{ + return quoting == Quoting::Kakoune ? "e : &shell_quote; +} + + } #endif // string_utils_hh_INCLUDED diff --git a/src/window.cc b/src/window.cc index 648d14ee..52279c15 100644 --- a/src/window.cc +++ b/src/window.cc @@ -340,7 +340,7 @@ void Window::clear_display_buffer() void Window::on_option_changed(const Option& option) { run_hook_in_own_context("WinSetOption", format("{}={}", option.name(), - option.get_as_string())); + option.get_as_string(Quoting::Kakoune))); // an highlighter might depend on the option, so we need to redraw force_redraw(); } diff --git a/test/shell/list-syntax/cmd b/test/shell/list-syntax/cmd new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/test/shell/list-syntax/cmd @@ -0,0 +1 @@ + diff --git a/test/shell/list-syntax/in b/test/shell/list-syntax/in new file mode 100644 index 00000000..8b137891 --- /dev/null +++ b/test/shell/list-syntax/in @@ -0,0 +1 @@ + diff --git a/test/shell/list-syntax/out b/test/shell/list-syntax/out new file mode 100644 index 00000000..074fb665 --- /dev/null +++ b/test/shell/list-syntax/out @@ -0,0 +1,4 @@ +foo +bar +'foo'bar' + diff --git a/test/shell/list-syntax/rc b/test/shell/list-syntax/rc new file mode 100644 index 00000000..e60c1ad5 --- /dev/null +++ b/test/shell/list-syntax/rc @@ -0,0 +1,7 @@ +declare-option str-list my_list 'foo' 'bar' '''foo''bar''' +evaluate-commands %sh{ + eval set -- $kak_opt_my_list + for elem; do + echo exec "'i$(printf %s "$elem" | sed -e s/\'/\'\'/g)'" + done +}