From d6215dc25d02d1f8d958d3f5ee5c2d8b39d48e77 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 2 Dec 2023 09:31:48 +0100 Subject: [PATCH 1/2] Reuse for_n_best when sorting values from complete options While at it, remove a needless reserve() call and reserve an extra slot because "InsertCompleter::try_complete" might add one more element. --- src/insert_completer.cc | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/insert_completer.cc b/src/insert_completer.cc index 967131d9..c43d9761 100644 --- a/src/insert_completer.cc +++ b/src/insert_completer.cc @@ -164,7 +164,7 @@ InsertCompletion complete_word(const SelectionList& sels, constexpr size_t max_count = 100; // Gather best max_count matches InsertCompletion::CandidateList candidates; - candidates.reserve(std::min(matches.size(), max_count)); + candidates.reserve(std::min(matches.size(), max_count) + 1); for_n_best(matches, max_count, [](auto& lhs, auto& rhs) { return rhs < lhs; }, [&](RankedMatchAndBuffer& m) { @@ -314,20 +314,19 @@ InsertCompletion complete_option(const SelectionList& sels, constexpr size_t max_count = 100; // Gather best max_count matches - auto greater = [](auto& lhs, auto& rhs) { return rhs < lhs; }; - auto first = matches.begin(), last = matches.end(); - std::make_heap(first, last, greater); InsertCompletion::CandidateList candidates; - candidates.reserve(std::min(matches.size(), max_count)); - candidates.reserve(matches.size()); - while (candidates.size() < max_count and first != last) - { - 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) }); - std::pop_heap(first, last--, greater); - } + candidates.reserve(std::min(matches.size(), max_count) + 1); + + for_n_best(matches, max_count, [](auto& lhs, auto& rhs) { return rhs < lhs; }, + [&](RankedMatchAndInfo& m) { + if (not candidates.empty() + and candidates.back().completion == m.candidate() + and candidates.back().on_select == m.on_select) + return false; + candidates.push_back({ m.candidate().str(), m.on_select.str(), + std::move(m.menu_entry) }); + return true; + }); auto end = cursor_pos; if (match[3].matched) From 658c3385a924ddfa784ab3346a56e2d96c9155a3 Mon Sep 17 00:00:00 2001 From: Johannes Altmanninger Date: Sat, 18 Nov 2023 09:12:05 +0100 Subject: [PATCH 2/2] 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 apply input order before resorting to sorting alphabetically. In theory there is a more elegant solution: sort candidates (except if they're user input) before passing them to RankedMatch, and then always use stable sort. However that doesn't work because we use a heap which doesn't support stable sort. Closes #1709, #4813 --- rc/tools/ctags.kak | 2 +- rc/tools/doc.kak | 2 +- rc/tools/git.kak | 32 +++++++++++++++++++++++++++----- rc/tools/man.kak | 5 ++++- src/commands.cc | 5 ++++- src/insert_completer.cc | 3 ++- src/ranked_match.cc | 3 +++ src/ranked_match.hh | 3 +++ 8 files changed, 45 insertions(+), 10 deletions(-) diff --git a/rc/tools/ctags.kak b/rc/tools/ctags.kak index 59992c01..d056fe63 100644 --- a/rc/tools/ctags.kak +++ b/rc/tools/ctags.kak @@ -25,7 +25,7 @@ define-command -params ..1 \ cut -f 1 "$tags" | grep -v '^!' | uniq > "$namecache" fi cat "$namecache" - done} \ + done | sort } \ -docstring %{ ctags-search []: jump to a symbol's definition If no symbol is passed then the current selection is used as symbol name diff --git a/rc/tools/doc.kak b/rc/tools/doc.kak index c1ceef08..5ce9a353 100644 --- a/rc/tools/doc.kak +++ b/rc/tools/doc.kak @@ -189,7 +189,7 @@ complete-command doc shell-script-candidates %{ /^\[\[[^\]]+\]\]/ { sub(/^\[\[/, ""); sub(/\]\].*/, ""); print } ' < $page | tr '[A-Z ]' '[a-z-]' fi;; - esac + esac | sort } alias global help doc diff --git a/rc/tools/git.kak b/rc/tools/git.kak index c6fd7ff5..2f19593e 100644 --- a/rc/tools/git.kak +++ b/rc/tools/git.kak @@ -62,27 +62,49 @@ define-command -params 1.. \ Available commands: add apply (alias for "patch git apply") - rm - reset blame - commit checkout + commit diff + edit + grep hide-blame hide-diff init log next-hunk prev-hunk + reset + rm show show-branch show-diff status update-diff - grep } -shell-script-candidates %{ if [ $kak_token_to_complete -eq 0 ]; then - printf "add\napply\nrm\nreset\nblame\ncommit\ncheckout\ndiff\nhide-blame\nhide-diff\nlog\nnext-hunk\nprev-hunk\nshow\nshow-branch\nshow-diff\ninit\nstatus\nupdate-diff\ngrep\nedit\n" + printf %s\\n \ + apply \ + blame \ + checkout \ + commit \ + diff \ + edit \ + grep \ + hide-blame \ + hide-diff \ + init \ + log \ + next-hunk \ + prev-hunk \ + reset \ + rm \ + show \ + show-branch \ + show-diff \ + status \ + update-diff \ + ; else case "$1" in commit) printf -- "--amend\n--no-edit\n--all\n--reset-author\n--fixup\n--squash\n"; git ls-files -m ;; diff --git a/rc/tools/man.kak b/rc/tools/man.kak index 24207353..949af56d 100644 --- a/rc/tools/man.kak +++ b/rc/tools/man.kak @@ -63,7 +63,10 @@ define-command -hidden -params ..3 man-impl %{ evaluate-commands %sh{ define-command -params ..1 \ -shell-script-candidates %{ - find /usr/share/man/ $(printf %s "${MANPATH}" | sed 's/:/ /') -name '*.[1-8]*' | sed 's,^.*/\(.*\)\.\([1-8][a-zA-Z]*\).*$,\1(\2),' + find /usr/share/man/ $(printf %s "${MANPATH}" | + sed 's/:/ /') -name '*.[1-8]*' | + sed 's,^.*/\(.*\)\.\([1-8][a-zA-Z]*\).*$,\1(\2),' | + sort } \ -docstring %{ man []: manpage viewer wrapper diff --git a/src/commands.cc b/src/commands.cc index 5c3b5f39..3413d095 100644 --- a/src/commands.cc +++ b/src/commands.cc @@ -337,10 +337,13 @@ private: { 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}) + { + m.set_input_sequence_number(i); matches.push_back(m); + } } constexpr size_t max_count = 100; diff --git a/src/insert_completer.cc b/src/insert_completer.cc index c43d9761..af387506 100644 --- a/src/insert_completer.cc +++ b/src/insert_completer.cc @@ -298,10 +298,11 @@ 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}) { + match.set_input_sequence_number(i); match.on_select = std::get<1>(candidate); auto& menu = std::get<2>(candidate); match.menu_entry = not menu.empty() ? diff --git a/src/ranked_match.cc b/src/ranked_match.cc index 2abea8d9..77cf758d 100644 --- a/src/ranked_match.cc +++ b/src/ranked_match.cc @@ -208,6 +208,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_sequence_number != other.m_input_sequence_number) + return m_input_sequence_number < other.m_input_sequence_number; + // Reorder codepoints to improve matching behaviour auto order = [](Codepoint cp) { return cp == '/' ? 0 : cp; }; diff --git a/src/ranked_match.hh b/src/ranked_match.hh index 01afe9cd..5a58defb 100644 --- a/src/ranked_match.hh +++ b/src/ranked_match.hh @@ -31,6 +31,8 @@ struct RankedMatch explicit operator bool() const { return m_matches; } + void set_input_sequence_number(size_t i) { m_input_sequence_number = i; } + private: template RankedMatch(StringView candidate, StringView query, TestFunc test); @@ -54,6 +56,7 @@ private: Flags m_flags = Flags::None; int m_word_boundary_match_count = 0; int m_max_index = 0; + size_t m_input_sequence_number = 0; }; }