From 5aa02411242468963b8b445a4977f0f4884eba4a Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Tue, 19 Jul 2022 11:26:56 +0200 Subject: [PATCH 1/5] Use and extract functions for completing scope arguments Not really attached to this but it enables the next commit to use menu() for completing scopes. This refactoring is possible because we always have params[token_to_complete].length() == pos_in_token --- Instead of three separate functions, I originally tried to add template arguments to complete_scope(). That worked fine with g++ 12.1 but clang 14.0 complained when wrapping a menu() around a complete_scope() that relied on defaulted template arguments: commands.cc:1087:20: error: no matching function for call to 'menu' make_completer(menu(complete_scope), menu(complete_hooks), complete_nothing, ^~~~ commands.cc:116:6: note: candidate template ignored: couldn't infer template argument 'Completer' auto menu(Completer completer) ^ --- src/commands.cc | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/commands.cc b/src/commands.cc index f8b9f9f9..ba65e57f 100644 --- a/src/commands.cc +++ b/src/commands.cc @@ -190,11 +190,24 @@ const ParameterDesc single_param{ {}, ParameterDesc::Flags::None, 1, 1 }; const ParameterDesc single_optional_param{ {}, ParameterDesc::Flags::None, 0, 1 }; const ParameterDesc double_params{ {}, ParameterDesc::Flags::None, 2, 2 }; -static constexpr auto scopes = { "global", "buffer", "window" }; - static Completions complete_scope(const Context&, CompletionFlags, StringView prefix, ByteCount cursor_pos) { + static constexpr StringView scopes[] = { "global", "buffer", "window", }; + return { 0_byte, cursor_pos, complete(prefix, cursor_pos, scopes) }; +} + +static Completions complete_scope_including_current(const Context&, CompletionFlags, + StringView prefix, ByteCount cursor_pos) +{ + static constexpr StringView scopes[] = { "global", "buffer", "window", "current" }; + return { 0_byte, cursor_pos, complete(prefix, cursor_pos, scopes) }; +} + +static Completions complete_scope_no_global(const Context&, CompletionFlags, + StringView prefix, ByteCount cursor_pos) +{ + static constexpr StringView scopes[] = { "buffer", "window", "current" }; return { 0_byte, cursor_pos, complete(prefix, cursor_pos, scopes) }; } @@ -1120,8 +1133,7 @@ const CommandDesc remove_hook_cmd = { ByteCount pos_in_token) -> Completions { if (token_to_complete == 0) - return { 0_byte, params[0].length(), - complete(params[0], pos_in_token, scopes) }; + return complete_scope(context, flags, params[0], pos_in_token); else if (token_to_complete == 1) { if (auto scope = get_scope_ifp(params[0], context)) @@ -1662,15 +1674,12 @@ const CommandDesc set_option_cmd = { }, CommandFlags::None, option_doc_helper, - [](const Context& context, CompletionFlags, + [](const Context& context, CompletionFlags flags, CommandParameters params, size_t token_to_complete, ByteCount pos_in_token) -> Completions { - static constexpr auto scopes = { "global", "buffer", "window", "current" }; - if (token_to_complete == 0) - return { 0_byte, params[0].length(), - complete(params[0], pos_in_token, scopes) }; + return complete_scope_including_current(context, flags, params[0], pos_in_token); else if (token_to_complete == 1) return { 0_byte, params[1].length(), GlobalScope::instance().option_registry().complete_option_name(params[1], pos_in_token) }; @@ -1701,15 +1710,12 @@ const CommandDesc set_option_cmd = { } }; -Completions complete_option(const Context& context, CompletionFlags, +Completions complete_option(const Context& context, CompletionFlags flags, CommandParameters params, size_t token_to_complete, ByteCount pos_in_token) { if (token_to_complete == 0) - { - static constexpr auto scopes = { "buffer", "window", "current" }; - return { 0_byte, params[0].length(), complete(params[0], pos_in_token, scopes) }; - } + return complete_scope_no_global(context, flags, params[0], pos_in_token); else if (token_to_complete == 1) return { 0_byte, params[1].length(), GlobalScope::instance().option_registry().complete_option_name(params[1], pos_in_token) }; @@ -1828,8 +1834,7 @@ static Completions map_key_completer(const Context& context, CompletionFlags fla ByteCount pos_in_token) { if (token_to_complete == 0) - return { 0_byte, params[0].length(), - complete(params[0], pos_in_token, scopes) }; + return complete_scope(context, flags, params[0], pos_in_token); if (token_to_complete == 1) { auto& user_modes = get_scope(params[0], context).keymaps().user_modes(); From a1715a0c4151884a2e12f78d10f4c4820902f1e3 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Mon, 18 Jul 2022 19:11:10 +0200 Subject: [PATCH 2/5] Use menu behavior when completing scope arguments We allow to abbreviate scopes ("set g" means the same thing as "set global") but that feature is a bit obscure. Users might figure out the menu completion behavior faster, so let's maybe use it here as well? --- src/commands.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/commands.cc b/src/commands.cc index ba65e57f..199d2d92 100644 --- a/src/commands.cc +++ b/src/commands.cc @@ -1094,7 +1094,7 @@ const CommandDesc add_hook_cmd = { }, CommandFlags::None, CommandHelper{}, - make_completer(complete_scope, complete_hooks, complete_nothing, + make_completer(menu(complete_scope),complete_hooks, complete_nothing, [](const Context& context, CompletionFlags flags, StringView prefix, ByteCount cursor_pos) { return CommandManager::instance().complete( @@ -1133,7 +1133,7 @@ const CommandDesc remove_hook_cmd = { ByteCount pos_in_token) -> Completions { if (token_to_complete == 0) - return complete_scope(context, flags, params[0], pos_in_token); + return menu(complete_scope)(context, flags, params[0], pos_in_token); else if (token_to_complete == 1) { if (auto scope = get_scope_ifp(params[0], context)) @@ -1361,7 +1361,7 @@ const CommandDesc alias_cmd = { ParameterDesc{{}, ParameterDesc::Flags::None, 3, 3}, CommandFlags::None, CommandHelper{}, - make_completer(complete_scope, complete_alias_name, complete_command_name), + make_completer(menu(complete_scope), complete_alias_name, complete_command_name), [](const ParametersParser& parser, Context& context, const ShellContext&) { if (not CommandManager::instance().command_defined(parser[2])) @@ -1380,7 +1380,7 @@ const CommandDesc unalias_cmd = { ParameterDesc{{}, ParameterDesc::Flags::None, 2, 3}, CommandFlags::None, CommandHelper{}, - make_completer(complete_scope, complete_alias_name, complete_command_name), + make_completer(menu(complete_scope), complete_alias_name, complete_command_name), [](const ParametersParser& parser, Context& context, const ShellContext&) { AliasRegistry& aliases = get_scope(parser[0], context).aliases(); @@ -1679,7 +1679,7 @@ const CommandDesc set_option_cmd = { ByteCount pos_in_token) -> Completions { if (token_to_complete == 0) - return complete_scope_including_current(context, flags, params[0], pos_in_token); + return menu(complete_scope_including_current)(context, flags, params[0], pos_in_token); else if (token_to_complete == 1) return { 0_byte, params[1].length(), GlobalScope::instance().option_registry().complete_option_name(params[1], pos_in_token) }; @@ -1715,7 +1715,7 @@ Completions complete_option(const Context& context, CompletionFlags flags, ByteCount pos_in_token) { if (token_to_complete == 0) - return complete_scope_no_global(context, flags, params[0], pos_in_token); + return menu(complete_scope_no_global)(context, flags, params[0], pos_in_token); else if (token_to_complete == 1) return { 0_byte, params[1].length(), GlobalScope::instance().option_registry().complete_option_name(params[1], pos_in_token) }; @@ -1834,7 +1834,7 @@ static Completions map_key_completer(const Context& context, CompletionFlags fla ByteCount pos_in_token) { if (token_to_complete == 0) - return complete_scope(context, flags, params[0], pos_in_token); + return menu(complete_scope)(context, flags, params[0], pos_in_token); if (token_to_complete == 1) { auto& user_modes = get_scope(params[0], context).keymaps().user_modes(); @@ -2448,7 +2448,7 @@ const CommandDesc set_face_cmd = { ParameterDesc{{}, ParameterDesc::Flags::None, 3, 3}, CommandFlags::None, face_doc_helper, - make_completer(complete_scope, complete_face, complete_face), + make_completer(menu(complete_scope), complete_face, complete_face), [](const ParametersParser& parser, Context& context, const ShellContext&) { get_scope(parser[0], context).faces().add_face(parser[1], parser[2], true); @@ -2465,7 +2465,7 @@ const CommandDesc unset_face_cmd = { double_params, CommandFlags::None, face_doc_helper, - make_completer(complete_scope, complete_face), + make_completer(menu(complete_scope), complete_face), [](const ParametersParser& parser, Context& context, const ShellContext&) { get_scope(parser[0], context).faces().remove_face(parser[1]); From 69053d96239e5ef1f97522949d8c712fa5846eab Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Tue, 19 Jul 2022 13:55:50 +0200 Subject: [PATCH 3/5] Use menu behavior when completing change-directory This makes "cd" change to the first completion, not to $HOME. This might be surprising but it should make sense. I don't have a concrete motivation but this should save a Tab press in some scenarios. --- src/commands.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/commands.cc b/src/commands.cc index 199d2d92..a854ea33 100644 --- a/src/commands.cc +++ b/src/commands.cc @@ -2555,7 +2555,8 @@ const CommandDesc change_directory_cmd = { return { 0_byte, cursor_pos, complete_filename(prefix, context.options()["ignored_files"].get(), - cursor_pos, FilenameFlags::OnlyDirectories) }; + cursor_pos, FilenameFlags::OnlyDirectories), + Completions::Flags::Menu }; }), [](const ParametersParser& parser, Context&, const ShellContext&) { From 0b5dcf062fd9af87428d599d26acad2dcecb3d30 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Fri, 29 Jul 2022 20:47:57 +0200 Subject: [PATCH 4/5] Use menu behavior when completing doc We can complete every valid argument. --- rc/tools/doc.kak | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rc/tools/doc.kak b/rc/tools/doc.kak index d5227ee4..1fe5b577 100644 --- a/rc/tools/doc.kak +++ b/rc/tools/doc.kak @@ -160,7 +160,7 @@ define-command -params 0..2 \ ' < $page | tr '[A-Z ]' '[a-z-]' fi;; esac - } \ + } -menu \ doc -docstring %{ doc []: open a buffer containing documentation about a given topic An optional keyword argument can be passed to the function, which will be automatically selected in the documentation From 031de6d28ca7279989d431c2e0225803c9389887 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Tue, 19 Jul 2022 12:58:14 +0200 Subject: [PATCH 5/5] Use menu behavior for completing builtins where appropriate This allows to select completions without pressing Tab. There are two different obvious ways to add the menu bit. 1. When creating the "Completions" object, pass the Completions::Flags::Menu parameter. 2. If there is a completer function like "complete_scope", wrap it, e.g. "menu(complete_scope)". I have settled on always using 2 if there is a completer function and 1 otherwise. The advantage of 2 over 1 is that it allows to use the completer function in a context where we don't want the menu behavior (e.g. "complete-command"). --- Now the only* completion type where we usually don't use menu behavior is file completion. Unfortunately, menu behavior has poor interaction with directories' trailing slashes. Consider this (contrived) example: define-command ls -docstring "list directory contents" -params .. %{ echo -- %sh{ls "$@"} } complete-command -menu ls file Run ":ls kakoun". The prompt expands to ":ls kakoune/" before executing. Next, run ":". This recalls ":ls kakoune/" and immediately selects the first completion, so on validation, the command will be ":ls kakoune/colors/", which is weird. [*] Also, expansions like %val{bufname} also don't use menu behavior. It wouldn't add value since validation doesn't add a closing delimiter. I have an experimental patch that adds closing delimiters automatically but I'm not sure if that's the right direction. --- src/commands.cc | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/commands.cc b/src/commands.cc index a854ea33..132aa34e 100644 --- a/src/commands.cc +++ b/src/commands.cc @@ -972,7 +972,7 @@ const CommandDesc arrange_buffers_cmd = { CommandHelper{}, [](const Context& context, CompletionFlags flags, CommandParameters params, size_t, ByteCount cursor_pos) { - return complete_buffer_name(context, flags, params.back(), cursor_pos); + return menu(complete_buffer_name)(context, flags, params.back(), cursor_pos); }, [](const ParametersParser& parser, Context&, const ShellContext&) { @@ -1094,7 +1094,7 @@ const CommandDesc add_hook_cmd = { }, CommandFlags::None, CommandHelper{}, - make_completer(menu(complete_scope),complete_hooks, complete_nothing, + make_completer(menu(complete_scope), menu(complete_hooks), complete_nothing, [](const Context& context, CompletionFlags flags, StringView prefix, ByteCount cursor_pos) { return CommandManager::instance().complete( @@ -1138,7 +1138,8 @@ const CommandDesc remove_hook_cmd = { { if (auto scope = get_scope_ifp(params[0], context)) return { 0_byte, params[0].length(), - scope->hooks().complete_hook_group(params[1], pos_in_token) }; + scope->hooks().complete_hook_group(params[1], pos_in_token), + Completions::Flags::Menu }; } return {}; }, @@ -1478,7 +1479,7 @@ const CommandDesc debug_cmd = { StringView prefix, ByteCount cursor_pos) -> Completions { auto c = {"info", "buffers", "options", "memory", "shared-strings", "profile-hash-maps", "faces", "mappings", "regex", "registers"}; - return { 0_byte, cursor_pos, complete(prefix, cursor_pos, c) }; + return { 0_byte, cursor_pos, complete(prefix, cursor_pos, c), Completions::Flags::Menu }; }), [](const ParametersParser& parser, Context& context, const ShellContext&) { @@ -1682,7 +1683,8 @@ const CommandDesc set_option_cmd = { return menu(complete_scope_including_current)(context, flags, params[0], pos_in_token); else if (token_to_complete == 1) return { 0_byte, params[1].length(), - GlobalScope::instance().option_registry().complete_option_name(params[1], pos_in_token) }; + GlobalScope::instance().option_registry().complete_option_name(params[1], pos_in_token), + Completions::Flags::Menu }; else if (token_to_complete == 2 and params[2].empty() and GlobalScope::instance().option_registry().option_exists(params[1])) { @@ -1718,7 +1720,8 @@ Completions complete_option(const Context& context, CompletionFlags flags, return menu(complete_scope_no_global)(context, flags, params[0], pos_in_token); else if (token_to_complete == 1) return { 0_byte, params[1].length(), - GlobalScope::instance().option_registry().complete_option_name(params[1], pos_in_token) }; + GlobalScope::instance().option_registry().complete_option_name(params[1], pos_in_token), + Completions::Flags::Menu }; return Completions{}; } @@ -1786,7 +1789,7 @@ const CommandDesc declare_option_cmd = { [](const Context& context, CompletionFlags flags, StringView prefix, ByteCount cursor_pos) -> Completions { auto c = {"int", "bool", "str", "regex", "int-list", "str-list", "completions", "line-specs", "range-specs", "str-to-str-map"}; - return { 0_byte, cursor_pos, complete(prefix, cursor_pos, c) }; + return { 0_byte, cursor_pos, complete(prefix, cursor_pos, c), Completions::Flags::Menu }; }), [](const ParametersParser& parser, Context& context, const ShellContext&) { @@ -1839,7 +1842,8 @@ static Completions map_key_completer(const Context& context, CompletionFlags fla { auto& user_modes = get_scope(params[0], context).keymaps().user_modes(); return { 0_byte, params[1].length(), - complete(params[1], pos_in_token, concatenated(modes, user_modes)) }; + complete(params[1], pos_in_token, concatenated(modes, user_modes)), + Completions::Flags::Menu }; } if (unmap and token_to_complete == 2) { @@ -1850,7 +1854,8 @@ static Completions map_key_completer(const Context& context, CompletionFlags fla return { 0_byte, params[2].length(), complete(params[2], pos_in_token, keys | transform([](Key k) { return key_to_str(k); }) - | gather>()) }; + | gather>()), + Completions::Flags::Menu }; } return {}; } @@ -2465,7 +2470,7 @@ const CommandDesc unset_face_cmd = { double_params, CommandFlags::None, face_doc_helper, - make_completer(menu(complete_scope), complete_face), + make_completer(menu(complete_scope), menu(complete_face)), [](const ParametersParser& parser, Context& context, const ShellContext&) { get_scope(parser[0], context).faces().remove_face(parser[1]); @@ -2653,7 +2658,8 @@ const CommandDesc enter_user_mode_cmd = { if (token_to_complete == 0) { return { 0_byte, params[0].length(), - complete(params[0], pos_in_token, context.keymaps().user_modes()) }; + complete(params[0], pos_in_token, context.keymaps().user_modes()), + Completions::Flags::Menu }; } return {}; }, @@ -2698,10 +2704,10 @@ const CommandDesc require_module_cmd = { single_param, CommandFlags::None, CommandHelper{}, - make_completer( + make_completer(menu( [](const Context&, CompletionFlags, StringView prefix, ByteCount cursor_pos) { return CommandManager::instance().complete_module_name(prefix.substr(0, cursor_pos)); - }), + })), [](const ParametersParser& parser, Context& context, const ShellContext&) { CommandManager::instance().load_module(parser[0], context);