Use shell specific quoting for env vars

Add a test case to validate roundtrips between Kakoune and the
shell.
This commit is contained in:
Maxime Coste 2018-08-27 21:52:57 +10:00
parent 373858f9bf
commit 68aba9e353
15 changed files with 98 additions and 58 deletions

View File

@ -688,7 +688,7 @@ void Buffer::on_option_changed(const Option& option)
m_flags &= ~Flags::ReadOnly; m_flags &= ~Flags::ReadOnly;
} }
run_hook_in_own_context("BufSetOption", 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) void Buffer::run_hook_in_own_context(StringView hook_name, StringView param, String client_name)

View File

@ -253,7 +253,7 @@ Token parse_percent_token(Reader& reader, bool throw_on_unterminated)
auto expand_option(Option& opt, std::true_type) 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) 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); auto it = shell_context.env_vars.find(content);
if (it != shell_context.env_vars.end()) if (it != shell_context.env_vars.end())
return {it->value}; return {it->value};
return {ShellManager::instance().get_val(content, context)}; return {ShellManager::instance().get_val(content, context, Quoting::Kakoune)};
} }
case Token::Type::ArgExpand: case Token::Type::ArgExpand:
{ {

View File

@ -1223,7 +1223,8 @@ const CommandDesc debug_cmd = {
{ {
write_to_debug_buffer("Options:"); write_to_debug_buffer("Options:");
for (auto& option : context.options().flatten_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") else if (parser[0] == "memory")
{ {
@ -1366,7 +1367,7 @@ const CommandDesc set_option_cmd = {
GlobalScope::instance().option_registry().option_exists(params[start + 1])) GlobalScope::instance().option_registry().option_exists(params[start + 1]))
{ {
OptionManager& options = get_scope(params[start], context).options(); 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{}; return Completions{};
}, },

View File

@ -113,131 +113,132 @@ String config_directory()
static const EnvVarDesc builtin_env_vars[] = { { static const EnvVarDesc builtin_env_vars[] = { {
"bufname", false, "bufname", false,
[](StringView name, const Context& context) -> String [](StringView name, const Context& context, Quoting quoting) -> String
{ return context.buffer().display_name(); } { return context.buffer().display_name(); }
}, { }, {
"buffile", false, "buffile", false,
[](StringView name, const Context& context) -> String [](StringView name, const Context& context, Quoting quoting) -> String
{ return context.buffer().name(); } { return context.buffer().name(); }
}, { }, {
"buflist", false, "buflist", false,
[](StringView name, const Context& context) [](StringView name, const Context& context, Quoting quoting)
{ return join(BufferManager::instance() | { return join(BufferManager::instance() |
transform(&Buffer::display_name) | transform(quote), ' ', false); } transform(&Buffer::display_name) | transform(quoter(quoting)), ' ', false); }
}, { }, {
"buf_line_count", 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()); } { return to_string(context.buffer().line_count()); }
}, { }, {
"timestamp", false, "timestamp", false,
[](StringView name, const Context& context) -> String [](StringView name, const Context& context, Quoting quoting) -> String
{ return to_string(context.buffer().timestamp()); } { return to_string(context.buffer().timestamp()); }
}, { }, {
"history_id", false, "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()); } { return to_string((size_t)context.buffer().current_history_id()); }
}, { }, {
"selection", false, "selection", false,
[](StringView name, const Context& context) [](StringView name, const Context& context, Quoting quoting)
{ const Selection& sel = context.selections().main(); { const Selection& sel = context.selections().main();
return content(context.buffer(), sel); } return content(context.buffer(), sel); }
}, { }, {
"selections", false, "selections", false,
[](StringView name, const Context& context) [](StringView name, const Context& context, Quoting quoting)
{ return join(context.selections_content() | transform(quote), ' ', false); } { return join(context.selections_content() | transform(quoter(quoting)), ' ', false); }
}, { }, {
"runtime", false, "runtime", false,
[](StringView name, const Context& context) [](StringView name, const Context& context, Quoting quoting)
{ return runtime_directory(); } { return runtime_directory(); }
}, { }, {
"config", false, "config", false,
[](StringView name, const Context& context) [](StringView name, const Context& context, Quoting quoting)
{ return config_directory(); } { return config_directory(); }
}, { }, {
"version", false, "version", false,
[](StringView name, const Context& context) -> String [](StringView name, const Context& context, Quoting quoting) -> String
{ return version; } { return version; }
}, { }, {
"opt_", true, "opt_", true,
[](StringView name, const Context& context) [](StringView name, const Context& context, Quoting quoting)
{ return context.options()[name.substr(4_byte)].get_as_string(); } { return context.options()[name.substr(4_byte)].get_as_string(quoting); }
}, { }, {
"main_reg_", true, "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(); } { return context.main_sel_register_value(name.substr(9_byte)).str(); }
}, { }, {
"reg_", true, "reg_", true,
[](StringView name, const Context& context) [](StringView name, const Context& context, Quoting quoting)
{ return join(RegisterManager::instance()[name.substr(4_byte)].get(context) | transform(quote), ' ', false); } { return join(RegisterManager::instance()[name.substr(4_byte)].get(context) |
transform(quoter(quoting)), ' ', false); }
}, { }, {
"client_env_", true, "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(); } { return context.client().get_env_var(name.substr(11_byte)).str(); }
}, { }, {
"session", false, "session", false,
[](StringView name, const Context& context) -> String [](StringView name, const Context& context, Quoting quoting) -> String
{ return Server::instance().session(); } { return Server::instance().session(); }
}, { }, {
"client", false, "client", false,
[](StringView name, const Context& context) -> String [](StringView name, const Context& context, Quoting quoting) -> String
{ return context.name(); } { return context.name(); }
}, { }, {
"client_pid", false, "client_pid", false,
[](StringView name, const Context& context) -> String [](StringView name, const Context& context, Quoting quoting) -> String
{ return to_string(context.client().pid()); } { return to_string(context.client().pid()); }
}, { }, {
"client_list", false, "client_list", false,
[](StringView name, const Context& context) -> String [](StringView name, const Context& context, Quoting quoting) -> String
{ return join(ClientManager::instance() | { return join(ClientManager::instance() |
transform([](const std::unique_ptr<Client>& c) -> const String& transform([](const std::unique_ptr<Client>& c) -> const String&
{ return c->context().name(); }), ' ', false); } { return c->context().name(); }), ' ', false); }
}, { }, {
"modified", false, "modified", false,
[](StringView name, const Context& context) -> String [](StringView name, const Context& context, Quoting quoting) -> String
{ return context.buffer().is_modified() ? "true" : "false"; } { return context.buffer().is_modified() ? "true" : "false"; }
}, { }, {
"cursor_line", 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); } { return to_string(context.selections().main().cursor().line + 1); }
}, { }, {
"cursor_column", false, "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); } { return to_string(context.selections().main().cursor().column + 1); }
}, { }, {
"cursor_char_value", false, "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 coord = context.selections().main().cursor();
auto& buffer = context.buffer(); auto& buffer = context.buffer();
return to_string((size_t)utf8::codepoint(buffer.iterator_at(coord), buffer.end())); } return to_string((size_t)utf8::codepoint(buffer.iterator_at(coord), buffer.end())); }
}, { }, {
"cursor_char_column", false, "cursor_char_column", false,
[](StringView name, const Context& context) -> String [](StringView name, const Context& context, Quoting quoting) -> String
{ auto coord = context.selections().main().cursor(); { auto coord = context.selections().main().cursor();
return to_string(context.buffer()[coord.line].char_count_to(coord.column) + 1); } return to_string(context.buffer()[coord.line].char_count_to(coord.column) + 1); }
}, { }, {
"cursor_byte_offset", false, "cursor_byte_offset", false,
[](StringView name, const Context& context) -> String [](StringView name, const Context& context, Quoting quoting) -> String
{ auto cursor = context.selections().main().cursor(); { auto cursor = context.selections().main().cursor();
return to_string(context.buffer().distance({0,0}, cursor)); } return to_string(context.buffer().distance({0,0}, cursor)); }
}, { }, {
"selection_desc", false, "selection_desc", false,
[](StringView name, const Context& context) [](StringView name, const Context& context, Quoting quoting)
{ return selection_to_string(context.selections().main()); } { return selection_to_string(context.selections().main()); }
}, { }, {
"selections_desc", false, "selections_desc", false,
[](StringView name, const Context& context) [](StringView name, const Context& context, Quoting quoting)
{ return selection_list_to_string(context.selections()); } { return selection_list_to_string(context.selections()); }
}, { }, {
"window_width", false, "window_width", false,
[](StringView name, const Context& context) -> String [](StringView name, const Context& context, Quoting quoting) -> String
{ return to_string(context.window().dimensions().column); } { return to_string(context.window().dimensions().column); }
}, { }, {
"window_height", false, "window_height", false,
[](StringView name, const Context& context) -> String [](StringView name, const Context& context, Quoting quoting) -> String
{ return to_string(context.window().dimensions().line); } { return to_string(context.window().dimensions().line); }
}, { }, {
"user_modes", false, "user_modes", false,
[](StringView name, const Context& context) -> String [](StringView name, const Context& context, Quoting quoting) -> String
{ return join(context.keymaps().user_modes(), ' ', false); } { return join(context.keymaps().user_modes(), ' ', false); }
} }
}; };

View File

@ -53,7 +53,7 @@ public:
template<typename T> void set(const T& val, bool notify=true); template<typename T> void set(const T& val, bool notify=true);
template<typename T> bool is_of_type() const; template<typename T> bool is_of_type() const;
virtual String get_as_string() const = 0; virtual String get_as_string(Quoting quoting) const = 0;
virtual Vector<String> get_as_strings() const = 0; virtual Vector<String> get_as_strings() const = 0;
virtual void set_from_strings(ConstArrayView<String> strs) = 0; virtual void set_from_strings(ConstArrayView<String> strs) = 0;
virtual void add_from_strings(ConstArrayView<String> strs) = 0; virtual void add_from_strings(ConstArrayView<String> strs) = 0;
@ -147,9 +147,9 @@ public:
return option_to_strings(m_value); 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<String> strs) override void set_from_strings(ConstArrayView<String> strs) override

View File

@ -18,6 +18,13 @@
namespace Kakoune namespace Kakoune
{ {
template<typename T>
std::enable_if_t<std::is_same<decltype(option_to_string(std::declval<T>())), String>::value, String>
option_to_string(const T& value, Quoting)
{
return option_to_string(value);
}
template<typename T, typename... Rest> template<typename T, typename... Rest>
constexpr bool option_needs_quoting(Meta::Type<T> type, Meta::Type<Rest>... rest) constexpr bool option_needs_quoting(Meta::Type<T> type, Meta::Type<Rest>... rest)
{ {
@ -25,10 +32,10 @@ constexpr bool option_needs_quoting(Meta::Type<T> type, Meta::Type<Rest>... rest
} }
template<typename... Ts> template<typename... Ts>
String quote_ifn(String str) String quote_ifn(String str, Quoting quoting)
{ {
if (option_needs_quoting(Meta::Type<Ts>{}...)) if (option_needs_quoting(Meta::Type<Ts>{}...))
return quote(std::move(str)); return quoter(quoting)(std::move(str));
return str; return str;
} }
@ -89,9 +96,9 @@ Vector<String> option_to_strings(const Vector<T, domain>& opt)
} }
template<typename T, MemoryDomain domain> template<typename T, MemoryDomain domain>
String option_to_string(const Vector<T, domain>& opt) String option_to_string(const Vector<T, domain>& opt, Quoting quoting)
{ {
return join(opt | transform([](const T& t) { return quote_ifn<T>(option_to_string(t)); }), ' ', false); return join(opt | transform([=](const T& t) { return quote_ifn<T>(option_to_string(t), quoting); }), ' ', false);
} }
template<typename T, MemoryDomain domain> template<typename T, MemoryDomain domain>
@ -135,13 +142,13 @@ Vector<String> option_to_strings(const HashMap<Key, Value, domain>& opt)
} }
template<typename Key, typename Value, MemoryDomain domain> template<typename Key, typename Value, MemoryDomain domain>
String option_to_string(const HashMap<Key, Value, domain>& opt) String option_to_string(const HashMap<Key, Value, domain>& opt, Quoting quoting)
{ {
return join(opt | transform([](auto&& item) { return join(opt | transform([=](auto&& item) {
return quote_ifn<Key, Value>( return quote_ifn<Key, Value>(
format("{}={}", format("{}={}",
escape(option_to_string(item.key), '=', '\\'), escape(option_to_string(item.key), '=', '\\'),
escape(option_to_string(item.value), '=', '\\'))); escape(option_to_string(item.value), '=', '\\')), quoting);
}), ' ', false); }), ' ', false);
} }
@ -335,9 +342,9 @@ inline Vector<String> option_to_strings(const PrefixedList<P, T>& opt)
} }
template<typename P, typename T> template<typename P, typename T>
inline String option_to_string(const PrefixedList<P, T>& opt) inline String option_to_string(const PrefixedList<P, T>& 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<typename P, typename T> template<typename P, typename T>

View File

@ -19,7 +19,7 @@ void GlobalScope::on_option_changed(const Option& option)
{ {
Context empty_context{Context::EmptyContextFlag{}}; Context empty_context{Context::EmptyContextFlag{}};
hooks().run_hook("GlobalSetOption", hooks().run_hook("GlobalSetOption",
format("{}={}", option.name(), option.get_as_string()), format("{}={}", option.name(), option.get_as_string(Quoting::Kakoune)),
empty_context); empty_context);
} }

View File

@ -138,7 +138,7 @@ Vector<String> generate_env(StringView cmdline, const Context& context, const Sh
try try
{ {
const String& value = var_it != shell_context.env_vars.end() ? 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)); kak_env.push_back(format("kak_{}={}", name, value));
} catch (runtime_error&) {} } catch (runtime_error&) {}
@ -312,7 +312,7 @@ std::pair<String, int> ShellManager::eval(
return { std::move(stdout_contents), WIFEXITED(status) ? WEXITSTATUS(status) : -1 }; 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) { auto env_var = find_if(m_env_vars, [name](const EnvVarDesc& desc) {
return desc.prefix ? prefix_match(name, desc.str) : name == desc.str; 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()) if (env_var == m_env_vars.end())
throw runtime_error("no such env var: " + name); 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, CandidateList ShellManager::complete_env_var(StringView prefix,

View File

@ -18,10 +18,11 @@ struct ShellContext
EnvVarMap env_vars; EnvVarMap env_vars;
}; };
enum class Quoting;
struct EnvVarDesc struct EnvVarDesc
{ {
using Retriever = String (*)(StringView name, const Context&); using Retriever = String (*)(StringView name, const Context&, Quoting quoting);
StringView str; StringView str;
bool prefix; bool prefix;
@ -45,7 +46,7 @@ public:
Flags flags = Flags::WaitForStdout, Flags flags = Flags::WaitForStdout,
const ShellContext& shell_context = {}); 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; CandidateList complete_env_var(StringView prefix, ByteCount cursor_pos) const;

View File

@ -135,6 +135,23 @@ inline String quote(StringView s)
return format("'{}'", double_up(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 ? &quote : &shell_quote;
}
} }
#endif // string_utils_hh_INCLUDED #endif // string_utils_hh_INCLUDED

View File

@ -340,7 +340,7 @@ void Window::clear_display_buffer()
void Window::on_option_changed(const Option& option) void Window::on_option_changed(const Option& option)
{ {
run_hook_in_own_context("WinSetOption", format("{}={}", option.name(), 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 // an highlighter might depend on the option, so we need to redraw
force_redraw(); force_redraw();
} }

View File

@ -0,0 +1 @@

View File

@ -0,0 +1 @@

View File

@ -0,0 +1,4 @@
foo
bar
'foo'bar'

View File

@ -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)<ret><esc>'"
done
}