Skip to content

Commit

Permalink
Support opt-in priority field in shell script candidates
Browse files Browse the repository at this point in the history
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<i32, i32>);
	}

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 kakoune-lsp/kakoune-lsp#657

[Helix]: helix-editor/helix#4134
Part of mawww#1709
  • Loading branch information
krobelus committed Dec 27, 2022
1 parent 647de2d commit 7941886
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 23 deletions.
63 changes: 49 additions & 14 deletions src/commands.cc
Original file line number Diff line number Diff line change
Expand Up @@ -278,16 +278,34 @@ struct ShellCandidatesCompleter
m_candidates.clear();
for (auto c : output | split<StringView>('\n')
| filter([](auto s) { return not s.empty(); }))
m_candidates.emplace_back(c.str(), used_letters(c));
{
String candidate;
Optional<Priority> priority;
if (m_flags & Completions::Flags::Priority)
{
priority.emplace();
std::tie(candidate, *priority) = option_from_string(Meta::Type<std::tuple<String, Priority>>{}, 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<RankedMatch> 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);
}

Expand All @@ -307,7 +325,12 @@ struct ShellCandidatesCompleter

private:
String m_shell_script;
Vector<std::pair<String, UsedLetters>, MemoryDomain::Completion> m_candidates;
struct Candidate {
String candidate;
UsedLetters used_letters;
Optional<Priority> priority;
};
Vector<Candidate, MemoryDomain::Completion> m_candidates;
int m_token = -1;
Completions::Flags m_flags;
};
Expand Down Expand Up @@ -1184,6 +1207,8 @@ Vector<String> 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,
Expand Down Expand Up @@ -1259,6 +1284,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"})
Expand All @@ -1274,6 +1301,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];
Expand All @@ -1289,10 +1326,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;
Expand Down Expand Up @@ -1326,8 +1359,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{}));

Expand All @@ -1345,6 +1379,7 @@ const CommandDesc define_command_cmd = {
{ "hidden", { false, "do not display the command in completion candidates" } },
{ "docstring", { true, "define the documentation string for command" } },
{ "menu", { false, "treat completions as the only valid inputs" } },
{ "priority", { false, "shell script candidates have candidate|priority syntax" } },
{ "file-completion", { false, "complete parameters using filename completion" } },
{ "client-completion", { false, "complete parameters using client name completion" } },
{ "buffer-completion", { false, "complete parameters using buffer name completion" } },
Expand Down Expand Up @@ -1412,14 +1447,15 @@ const CommandDesc complete_command_cmd = {
"complete-command [<switches>] <name> <type> [<param>]\n"
"define command completion",
ParameterDesc{
{ { "menu", { false, "treat completions as the only valid inputs" } }, },
{ { "menu", { false, "treat completions as the only valid inputs" } },
{ "priority", { false, "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));
}
Expand Down Expand Up @@ -2158,6 +2194,7 @@ const CommandDesc prompt_cmd = {
{ { "init", { true, "set initial prompt content" } },
{ "password", { false, "Do not display entered text and clear reg after command" } },
{ "menu", { false, "treat completions as the only valid inputs" } },
{ "priority", { false, "shell script candidates have candidate|priority syntax" } },
{ "file-completion", { false, "use file completion for prompt" } },
{ "client-completion", { false, "use client completion for prompt" } },
{ "buffer-completion", { false, "use buffer completion for prompt" } },
Expand All @@ -2177,9 +2214,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;
Expand Down
3 changes: 2 additions & 1 deletion src/completion.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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<Flags>) { return true; }
Expand Down
30 changes: 25 additions & 5 deletions src/ranked_match.cc
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ RankedMatchQuery::RankedMatchQuery(StringView input, UsedLetters used_letters)
}) | gather<decltype(smartcase_alternative_match)>()) {}

template<typename TestFunc>
RankedMatch::RankedMatch(StringView candidate, const RankedMatchQuery& query, TestFunc func)
RankedMatch::RankedMatch(StringView candidate, const RankedMatchQuery& query, Optional<Priority> priority, TestFunc func)
{
if (query.input.length() > candidate.length())
return;
Expand All @@ -243,6 +243,8 @@ RankedMatch::RankedMatch(StringView candidate, const RankedMatchQuery& query, Te
{
m_candidate = candidate;
m_matches = true;
if (priority)
m_distance = *priority;
return;
}

Expand All @@ -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> 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> priority)
: RankedMatch{candidate, query, priority, [] { return true; }}
{
}

Expand Down Expand Up @@ -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<true>(*q, better);
distance_worse = subsequence_distance<true>(*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{[]()
Expand Down
6 changes: 3 additions & 3 deletions src/ranked_match.hh
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ using Priority = size_t;

struct RankedMatch
{
RankedMatch(StringView candidate, const RankedMatchQuery& query);
RankedMatch(StringView candidate, const RankedMatchQuery& query, Optional<Priority> priority = {});
RankedMatch(StringView candidate, UsedLetters candidate_letters,
const RankedMatchQuery& query);
const RankedMatchQuery& query, Optional<Priority> priority = {});

const StringView& candidate() const { return m_candidate; }
bool operator<(const RankedMatch& other) const;
Expand All @@ -46,7 +46,7 @@ struct RankedMatch

private:
template<typename TestFunc>
RankedMatch(StringView candidate, const RankedMatchQuery& query, TestFunc test);
RankedMatch(StringView candidate, const RankedMatchQuery& query, Optional<Priority> priority, TestFunc test);

StringView m_candidate{};
bool m_matches = false;
Expand Down

0 comments on commit 7941886

Please sign in to comment.