From 4ae738fd805dd87f85dfcd1a5c4f86c2793c7006 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Thu, 22 Dec 2022 14:54:55 +0100 Subject: [PATCH] [custom-completion-sorting] Support opt-in priority field in shell script candidates This is inspired by a recent change in [Helix] that fixes sorting of code actions. We have the same problem because kak-lsp uses ":prompt -shell-script-candidates" to show code actions. For example, on this Rust file: fn main() { let f: FnOnce(HashMap); } with the cursor on "HashMap", a ":lsp-code-actions" will offer two code actions (from rust-analyzer): Extract type as type alias" Import `std::collections::HashMap` The first one is a refactoring and the second one is a quickfix. If fuzzy match scores are equal, Kakoune sorts completions lexicographically, which is suboptimal because the user will almost always want to run the quickfix first. Allow users to influence the order via a new "-priority" switch. When this switch is used, Kakoune expects a second field in shell-script-candidates completions, like so: Extract type as type alias"|2 Import `std::collections::HashMap`|1 The priority field is taken into account when computing fuzzy match scores. Due to the lack of test cases, the math to do so does not have a solid footing yet. Here's how it works for now. - "distance" is the fuzzy match score (lower is better) - "priority" is the new user-specificed ranking, a positive integer (lower is better) - "query_length" is the length of the string that is used to filter completions effective_priority = priority ^ (1 / query_length) if query_length != 0 else priority prioritized_distance = distance * (effective_priority ^ sign(distance)) The ideas are that 1. A priority of 1 is neutral. Higher values increase the distance (making it worse). 2. The longer the query, the lower the impact of "priority". --- Used by https://github.com/kak-lsp/kak-lsp/pull/657 [Helix]: https://github.com/helix-editor/helix/pull/4134 Part of #1709 --- src/commands.cc | 63 +++++++++++++++++++++++++++++++++++---------- src/completion.hh | 3 ++- src/ranked_match.cc | 30 +++++++++++++++++---- src/ranked_match.hh | 6 ++--- 4 files changed, 79 insertions(+), 23 deletions(-) diff --git a/src/commands.cc b/src/commands.cc index b48a17fc64..ad0529bd97 100644 --- a/src/commands.cc +++ b/src/commands.cc @@ -299,16 +299,34 @@ struct ShellCandidatesCompleter m_candidates.clear(); for (auto c : output | split('\n') | filter([](auto s) { return not s.empty(); })) - m_candidates.emplace_back(c.str(), used_letters(c)); + { + String candidate; + Optional priority; + if (m_flags & Completions::Flags::Priority) + { + priority.emplace(); + std::tie(candidate, *priority) = option_from_string(Meta::Type>{}, c); + if (m_flags & Completions::Flags::Priority and (int)*priority <= 0) + { + String error_message = "error computing shell-script-candidates: priority must be a positive integer"; + write_to_debug_buffer(error_message); + throw runtime_error(std::move(error_message)); + } + } + else + candidate = c.str(); + UsedLetters letters = used_letters(candidate); + m_candidates.push_back(Candidate{std::move(candidate), letters, priority}); + } m_token = token_to_complete; } StringView query = params[token_to_complete].substr(0, pos_in_token); RankedMatchQuery q{query, used_letters(query)}; Vector matches; - for (const auto& candidate : m_candidates) + for (const auto& c : m_candidates) { - if (RankedMatch match{candidate.first, candidate.second, q}) + if (RankedMatch match{c.candidate, c.used_letters, q, c.priority}) matches.push_back(match); } @@ -328,7 +346,12 @@ struct ShellCandidatesCompleter private: String m_shell_script; - Vector, MemoryDomain::Completion> m_candidates; + struct Candidate { + String candidate; + UsedLetters used_letters; + Optional priority; + }; + Vector m_candidates; int m_token = -1; Completions::Flags m_flags; }; @@ -1208,6 +1231,8 @@ Vector params_to_shell(const ParametersParser& parser) CommandCompleter make_command_completer(StringView type, StringView param, Completions::Flags completions_flags) { + if (completions_flags & Completions::Flags::Priority and type != "shell-script-candidates") + throw runtime_error("-priority requires shell-script-candidates"); if (type == "file") { return [=](const Context& context, CompletionFlags flags, @@ -1283,6 +1308,8 @@ CommandCompleter make_command_completer(StringView type, StringView param, Compl } static CommandCompleter parse_completion_switch(const ParametersParser& parser, Completions::Flags completions_flags) { + if (completions_flags & Completions::Flags::Priority and not parser.get_switch("shell-script-candidates")) + throw runtime_error("-priority requires -shell-script-candidates"); for (StringView completion_switch : {"file-completion", "client-completion", "buffer-completion", "shell-script-completion", "shell-script-candidates", "command-completion", "shell-completion"}) @@ -1298,6 +1325,16 @@ static CommandCompleter parse_completion_switch(const ParametersParser& parser, return {}; } +static Completions::Flags make_completions_flags(const ParametersParser& parser) +{ + Completions::Flags flags = Completions::Flags::None; + if (parser.get_switch("menu")) + flags |= Completions::Flags::Menu; + if (parser.get_switch("priority")) + flags |= Completions::Flags::Priority; + return flags; +} + void define_command(const ParametersParser& parser, Context& context, const ShellContext&) { const String& cmd_name = parser[0]; @@ -1313,10 +1350,6 @@ void define_command(const ParametersParser& parser, Context& context, const Shel if (parser.get_switch("hidden")) flags = CommandFlags::Hidden; - bool menu = (bool)parser.get_switch("menu"); - const Completions::Flags completions_flags = menu ? - Completions::Flags::Menu : Completions::Flags::None; - const String& commands = parser[1]; CommandFunc cmd; ParameterDesc desc; @@ -1350,8 +1383,9 @@ void define_command(const ParametersParser& parser, Context& context, const Shel }; } + const Completions::Flags completions_flags = make_completions_flags(parser); CommandCompleter completer = parse_completion_switch(parser, completions_flags); - if (menu and not completer) + if (completions_flags & Completions::Flags::Menu and not completer) throw runtime_error(format("menu switch requires a completion switch", cmd_name)); auto docstring = trim_indent(parser.get_switch("docstring").value_or(StringView{})); @@ -1369,6 +1403,7 @@ const CommandDesc define_command_cmd = { { "hidden", { {}, "do not display the command in completion candidates" } }, { "docstring", { ArgCompleter{}, "define the documentation string for command" } }, { "menu", { {}, "treat completions as the only valid inputs" } }, + { "priority", { {}, "shell script candidates have candidate|priority syntax" } }, { "file-completion", { {}, "complete parameters using filename completion" } }, { "client-completion", { {}, "complete parameters using client name completion" } }, { "buffer-completion", { {}, "complete parameters using buffer name completion" } }, @@ -1436,14 +1471,15 @@ const CommandDesc complete_command_cmd = { "complete-command [] []\n" "define command completion", ParameterDesc{ - { { "menu", { {}, "treat completions as the only valid inputs" } }, }, + { { "menu", { {}, "treat completions as the only valid inputs" } }, + { "priority", { {}, "shell script candidates have candidate|priority syntax" } }, }, ParameterDesc::Flags::None, 2, 3}, CommandFlags::None, CommandHelper{}, make_completer(complete_command_name), [](const ParametersParser& parser, Context& context, const ShellContext&) { - const Completions::Flags flags = parser.get_switch("menu") ? Completions::Flags::Menu : Completions::Flags::None; + const Completions::Flags flags = make_completions_flags(parser); CommandCompleter completer = make_command_completer(parser[1], parser.positional_count() >= 3 ? parser[2] : StringView{}, flags); CommandManager::instance().set_command_completer(parser[0], std::move(completer)); } @@ -2186,6 +2222,7 @@ const CommandDesc prompt_cmd = { { { "init", { ArgCompleter{}, "set initial prompt content" } }, { "password", { {}, "Do not display entered text and clear reg after command" } }, { "menu", { {}, "treat completions as the only valid inputs" } }, + { "priority", { {}, "shell script candidates have candidate|priority syntax" } }, { "file-completion", { {}, "use file completion for prompt" } }, { "client-completion", { {}, "use client completion for prompt" } }, { "buffer-completion", { {}, "use buffer completion for prompt" } }, @@ -2205,9 +2242,7 @@ const CommandDesc prompt_cmd = { const String& command = parser[1]; auto initstr = parser.get_switch("init").value_or(StringView{}); - const Completions::Flags completions_flags = parser.get_switch("menu") ? - Completions::Flags::Menu : Completions::Flags::None; - PromptCompleterAdapter completer = parse_completion_switch(parser, completions_flags); + PromptCompleterAdapter completer = parse_completion_switch(parser, make_completions_flags(parser)); const auto flags = parser.get_switch("password") ? PromptFlags::Password : PromptFlags::None; diff --git a/src/completion.hh b/src/completion.hh index ec1702e51e..c5ae4e4ee8 100644 --- a/src/completion.hh +++ b/src/completion.hh @@ -23,7 +23,8 @@ struct Completions None = 0, Quoted = 0b1, Menu = 0b10, - NoEmpty = 0b100 + NoEmpty = 0b100, + Priority = 0b1000 }; constexpr friend bool with_bit_ops(Meta::Type) { return true; } diff --git a/src/ranked_match.cc b/src/ranked_match.cc index 00164acc8d..fad446de7a 100644 --- a/src/ranked_match.cc +++ b/src/ranked_match.cc @@ -234,7 +234,7 @@ RankedMatchQuery::RankedMatchQuery(StringView input, UsedLetters used_letters) }) | gather()) {} template -RankedMatch::RankedMatch(StringView candidate, const RankedMatchQuery& query, TestFunc func) +RankedMatch::RankedMatch(StringView candidate, const RankedMatchQuery& query, Optional priority, TestFunc func) { if (query.input.length() > candidate.length()) return; @@ -243,6 +243,8 @@ RankedMatch::RankedMatch(StringView candidate, const RankedMatchQuery& query, Te { m_candidate = candidate; m_matches = true; + if (priority) + m_distance = *priority; return; } @@ -265,18 +267,25 @@ RankedMatch::RankedMatch(StringView candidate, const RankedMatchQuery& query, Te m_distance = distance[query_length(query) % 2][bounded_candidate.char_length()].distance + (int)distance.max_index * max_index_weight; + if (priority) + { + double effective_priority = *priority; + if (auto query_len = query.input.char_length(); query_len != 0) + effective_priority = std::pow(effective_priority, 1.0 / (double)(size_t)query_len); + m_distance = m_distance >= 0 ? (m_distance * effective_priority) : (m_distance / effective_priority); + } } RankedMatch::RankedMatch(StringView candidate, UsedLetters candidate_letters, - const RankedMatchQuery& query) - : RankedMatch{candidate, query, [&] { + const RankedMatchQuery& query, Optional priority) + : RankedMatch{candidate, query, priority, [&] { return matches(to_lower(query.used_letters), to_lower(candidate_letters)) and matches(query.used_letters & upper_mask, candidate_letters & upper_mask); }} {} -RankedMatch::RankedMatch(StringView candidate, const RankedMatchQuery& query) - : RankedMatch{candidate, query, [] { return true; }} +RankedMatch::RankedMatch(StringView candidate, const RankedMatchQuery& query, Optional priority) + : RankedMatch{candidate, query, priority, [] { return true; }} { } @@ -435,6 +444,17 @@ UnitTest test_ranked_match{[] { kak_assert(ranked_match_order("fooo", "foo.o", "fo.o.o")); kak_assert(ranked_match_order("evilcorp-lint/bar.go", "scripts/evilcorp-lint/foo/bar.go", "src/evilcorp-client/foo/bar.go")); kak_assert(ranked_match_order("lang/haystack/needle.c", "git.evilcorp.com/language/haystack/aaa/needle.c", "git.evilcorp.com/aaa/ng/wrong-haystack/needle.cpp")); + + auto ranked_match_order_with_priority = [&](StringView query, StringView better, Priority better_priority, StringView worse, Priority worse_priority) -> bool { + q = RankedMatchQuery{query}; + distance_better = subsequence_distance(*q, better); + distance_worse = subsequence_distance(*q, worse); + return RankedMatch{better, *q, better_priority} < RankedMatch{worse, *q, worse_priority}; + }; + + kak_assert(ranked_match_order_with_priority("", "Qualify as `std::collections::HashMap`", 1, "Extract type as type alias", 2)); + kak_assert(ranked_match_order_with_priority("as", "Qualify as `std::collections::HashMap`", 1, "Extract type as type alias", 2)); + }}; UnitTest test_used_letters{[]() diff --git a/src/ranked_match.hh b/src/ranked_match.hh index 1ccc56883a..98ab9f477d 100644 --- a/src/ranked_match.hh +++ b/src/ranked_match.hh @@ -36,9 +36,9 @@ using Priority = size_t; struct RankedMatch { - RankedMatch(StringView candidate, const RankedMatchQuery& query); + RankedMatch(StringView candidate, const RankedMatchQuery& query, Optional priority = {}); RankedMatch(StringView candidate, UsedLetters candidate_letters, - const RankedMatchQuery& query); + const RankedMatchQuery& query, Optional priority = {}); const StringView& candidate() const { return m_candidate; } bool operator<(const RankedMatch& other) const; @@ -48,7 +48,7 @@ struct RankedMatch private: template - RankedMatch(StringView candidate, const RankedMatchQuery& query, TestFunc test); + RankedMatch(StringView candidate, const RankedMatchQuery& query, Optional priority, TestFunc test); StringView m_candidate{}; bool m_matches = false;