From 580652362d7186401a4830bf68f06f0996a3283e Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 18 Nov 2023 09:12:05 +0100 Subject: [PATCH] ranked match: prefer input order over alphabetical order for user-specified completions When using either of set-option g completers option=my_option prompt -shell-script-candidates ... While the search text is empty, the completions will be sorted alphabetically. This is bad because it means the most important entries are not listed first, making them harder to select or even spot. Let's compare input order before sorting alphabetically. In theory there is a more elegant solution: sort candidates (except if they're user input) before passing them to RankedMatch, and only use stable sort. However that doesn't work because we use a heap which doesn't support stable sort. Closes #1709, #4813 --- src/commands.cc | 10 +++++----- src/insert_completer.cc | 18 +++++++++--------- src/insert_completer.hh | 3 ++- src/ranked_match.cc | 16 +++++++++++----- src/ranked_match.hh | 7 ++++--- 5 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/commands.cc b/src/commands.cc index 1c6ea09adc..bd4fa5765c 100644 --- a/src/commands.cc +++ b/src/commands.cc @@ -168,7 +168,7 @@ static Completions complete_buffer_name(const Context& context, CompletionFlags StringView query = prefix.substr(0, cursor_pos); Vector filename_matches; Vector matches; - for (const auto& buffer : BufferManager::instance()) + for (auto&& [i, buffer] : BufferManager::instance() | enumerate()) { if (ignore_current and buffer.get() == &context.buffer()) continue; @@ -176,13 +176,13 @@ static Completions complete_buffer_name(const Context& context, CompletionFlags StringView bufname = buffer->display_name(); if (buffer->flags() & Buffer::Flags::File) { - if (RankedMatch match{split_path(bufname).second, query}) + if (RankedMatch match{split_path(bufname).second, query, i}) { filename_matches.emplace_back(match, buffer.get()); continue; } } - if (RankedMatch match{bufname, query}) + if (RankedMatch match{bufname, query, i}) matches.emplace_back(match, buffer.get()); } std::sort(filename_matches.begin(), filename_matches.end()); @@ -337,9 +337,9 @@ struct ShellCandidatesCompleter { UsedLetters query_letters = used_letters(query); Vector matches; - for (auto&& candidate : m_candidates) + for (auto&& [i, candidate] : m_candidates | enumerate()) { - if (RankedMatch m{candidate.first, candidate.second, query, query_letters}) + if (RankedMatch m{candidate.first, candidate.second, query, query_letters, i}) matches.push_back(m); } diff --git a/src/insert_completer.cc b/src/insert_completer.cc index 967131d94e..f9af291932 100644 --- a/src/insert_completer.cc +++ b/src/insert_completer.cc @@ -181,7 +181,7 @@ InsertCompletion complete_word(const SelectionList& sels, else menu_entry.push_back({ m.candidate().str(), {} }); - candidates.push_back({m.candidate().str(), "", std::move(menu_entry)}); + candidates.push_back({0, m.candidate().str(), "", std::move(menu_entry)}); return true; }); @@ -221,7 +221,7 @@ InsertCompletion complete_filename(const SelectionList& sels, { for (auto& filename : Kakoune::complete_filename(prefix, options["ignored_files"].get())) - candidates.push_back({ filename, "", {filename, {}} }); + candidates.push_back({ 0, filename, "", {filename, {}} }); } else { @@ -241,7 +241,7 @@ InsertCompletion complete_filename(const SelectionList& sels, options["ignored_files"].get())) { StringView candidate = filename.substr(dir.length()); - candidates.push_back({ candidate.str(), "", {candidate.str(), {}} }); + candidates.push_back({ 0, candidate.str(), "", {candidate.str(), {}} }); } visited_dirs.push_back(std::move(dir)); @@ -298,9 +298,9 @@ InsertCompletion complete_option(const SelectionList& sels, StringView query = buffer.substr(coord, cursor_pos); Vector matches; - for (auto& candidate : opt.list) + for (auto&& [i, candidate] : opt.list | enumerate()) { - if (RankedMatchAndInfo match{std::get<0>(candidate), query}) + if (RankedMatchAndInfo match{std::get<0>(candidate), query, i}) { match.on_select = std::get<1>(candidate); auto& menu = std::get<2>(candidate); @@ -324,8 +324,8 @@ InsertCompletion complete_option(const SelectionList& sels, { if (candidates.empty() or candidates.back().completion != first->candidate() or candidates.back().on_select != first->on_select) - candidates.push_back({ first->candidate().str(), first->on_select.str(), - std::move(first->menu_entry) }); + candidates.push_back({ candidates.size(), first->candidate().str(), + first->on_select.str(), std::move(first->menu_entry) }); std::pop_heap(first, last--, greater); } @@ -374,7 +374,7 @@ InsertCompletion complete_line(const SelectionList& sels, if (prefix == candidate.substr(0_byte, prefix.length())) { - candidates.push_back({candidate.str(), "", {expand_tabs(candidate, tabstop, column), {}} }); + candidates.push_back({0, candidate.str(), "", {expand_tabs(candidate, tabstop, column), {}} }); // perf: it's unlikely the user intends to search among >10 candidates anyway if (candidates.size() == 100) break; @@ -610,7 +610,7 @@ bool InsertCompleter::try_complete(Func complete_func) kak_assert(m_completions.begin <= sels.main().cursor()); m_current_candidate = m_completions.candidates.size(); menu_show(); - m_completions.candidates.push_back({sels.buffer().string(m_completions.begin, m_completions.end), "", {}}); + m_completions.candidates.push_back({m_completions.candidates.size(), sels.buffer().string(m_completions.begin, m_completions.end), "", {}}); return true; } diff --git a/src/insert_completer.hh b/src/insert_completer.hh index cce433a265..d36a481269 100644 --- a/src/insert_completer.hh +++ b/src/insert_completer.hh @@ -53,12 +53,13 @@ struct InsertCompletion { struct Candidate { + size_t input_order; String completion; String on_select; DisplayLine menu_entry; bool operator==(const Candidate& other) const { return completion == other.completion; } - auto operator<=>(const Candidate& other) const { return completion <=> other.completion; } + auto operator<=>(const Candidate& other) const { return std::tie(completion, input_order) <=> std::tie(other.completion, other.input_order); } }; using CandidateList = Vector; diff --git a/src/ranked_match.cc b/src/ranked_match.cc index a0eedda436..e2ea094001 100644 --- a/src/ranked_match.cc +++ b/src/ranked_match.cc @@ -110,11 +110,13 @@ static Optional subsequence_match_smart_case(StringView str, StringVi } template -RankedMatch::RankedMatch(StringView candidate, StringView query, TestFunc func) +RankedMatch::RankedMatch(StringView candidate, StringView query, TestFunc func, size_t input_order) { if (query.length() > candidate.length()) return; + m_input_order = input_order; + if (query.empty()) { m_candidate = candidate; @@ -169,15 +171,15 @@ RankedMatch::RankedMatch(StringView candidate, StringView query, TestFunc func) } RankedMatch::RankedMatch(StringView candidate, UsedLetters candidate_letters, - StringView query, UsedLetters query_letters) + StringView query, UsedLetters query_letters, size_t input_order) : RankedMatch{candidate, query, [&] { return matches(to_lower(query_letters), to_lower(candidate_letters)) and matches(query_letters & upper_mask, candidate_letters & upper_mask); - }} {} + }, input_order} {} -RankedMatch::RankedMatch(StringView candidate, StringView query) - : RankedMatch{candidate, query, [] { return true; }} +RankedMatch::RankedMatch(StringView candidate, StringView query, size_t input_order) + : RankedMatch{candidate, query, [] { return true; }, input_order} { } @@ -205,6 +207,9 @@ bool RankedMatch::operator<(const RankedMatch& other) const if (m_max_index != other.m_max_index) return m_max_index < other.m_max_index; + if (m_input_order != other.m_input_order) + return m_input_order < other.m_input_order; + // Reorder codepoints to improve matching behaviour auto order = [](Codepoint cp) { return cp == '/' ? 0 : cp; }; @@ -260,6 +265,7 @@ UnitTest test_ranked_match{[] { kak_assert(preferred("so", "source", "source_data")); kak_assert(not preferred("so", "source_data", "source")); kak_assert(not preferred("so", "source", "source")); + kak_assert(RankedMatch{"b", "", 0} < RankedMatch{"a", "", 1}); // Use input order for user input kak_assert(preferred("wo", "single/word", "multiw/ord")); kak_assert(preferred("foobar", "foo/bar/foobar", "foo/bar/baz")); kak_assert(preferred("db", "delete-buffer", "debug")); diff --git a/src/ranked_match.hh b/src/ranked_match.hh index 01afe9cdfc..6d2b0a1d37 100644 --- a/src/ranked_match.hh +++ b/src/ranked_match.hh @@ -21,9 +21,9 @@ inline UsedLetters to_lower(UsedLetters letters) struct RankedMatch { - RankedMatch(StringView candidate, StringView query); + RankedMatch(StringView candidate, StringView query, size_t input_order = 0); RankedMatch(StringView candidate, UsedLetters candidate_letters, - StringView query, UsedLetters query_letters); + StringView query, UsedLetters query_letters, size_t input_order = 0); const StringView& candidate() const { return m_candidate; } bool operator<(const RankedMatch& other) const; @@ -33,7 +33,7 @@ struct RankedMatch private: template - RankedMatch(StringView candidate, StringView query, TestFunc test); + RankedMatch(StringView candidate, StringView query, TestFunc test, size_t input_order); enum class Flags : int { @@ -54,6 +54,7 @@ private: Flags m_flags = Flags::None; int m_word_boundary_match_count = 0; int m_max_index = 0; + size_t m_input_order; }; }