Skip to content

Commit

Permalink
ranked match: prefer input order over alphabetical order for user-spe…
Browse files Browse the repository at this point in the history
…cified 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
  • Loading branch information
krobelus committed Nov 20, 2023
1 parent 4499b26 commit 5806523
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 23 deletions.
10 changes: 5 additions & 5 deletions src/commands.cc
Original file line number Diff line number Diff line change
Expand Up @@ -168,21 +168,21 @@ static Completions complete_buffer_name(const Context& context, CompletionFlags
StringView query = prefix.substr(0, cursor_pos);
Vector<RankedMatchAndBuffer> filename_matches;
Vector<RankedMatchAndBuffer> matches;
for (const auto& buffer : BufferManager::instance())
for (auto&& [i, buffer] : BufferManager::instance() | enumerate())
{
if (ignore_current and buffer.get() == &context.buffer())
continue;

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());
Expand Down Expand Up @@ -337,9 +337,9 @@ struct ShellCandidatesCompleter
{
UsedLetters query_letters = used_letters(query);
Vector<RankedMatch> 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);
}

Expand Down
18 changes: 9 additions & 9 deletions src/insert_completer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});

Expand Down Expand Up @@ -221,7 +221,7 @@ InsertCompletion complete_filename(const SelectionList& sels,
{
for (auto& filename : Kakoune::complete_filename(prefix,
options["ignored_files"].get<Regex>()))
candidates.push_back({ filename, "", {filename, {}} });
candidates.push_back({ 0, filename, "", {filename, {}} });
}
else
{
Expand All @@ -241,7 +241,7 @@ InsertCompletion complete_filename(const SelectionList& sels,
options["ignored_files"].get<Regex>()))
{
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));
Expand Down Expand Up @@ -298,9 +298,9 @@ InsertCompletion complete_option(const SelectionList& sels,
StringView query = buffer.substr(coord, cursor_pos);
Vector<RankedMatchAndInfo> 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);
Expand All @@ -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);
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
3 changes: 2 additions & 1 deletion src/insert_completer.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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<Candidate, MemoryDomain::Completion>;

Expand Down
16 changes: 11 additions & 5 deletions src/ranked_match.cc
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,13 @@ static Optional<SubseqRes> subsequence_match_smart_case(StringView str, StringVi
}

template<typename TestFunc>
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;
Expand Down Expand Up @@ -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}
{
}

Expand Down Expand Up @@ -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; };

Expand Down Expand Up @@ -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"));
Expand Down
7 changes: 4 additions & 3 deletions src/ranked_match.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,7 +33,7 @@ struct RankedMatch

private:
template<typename TestFunc>
RankedMatch(StringView candidate, StringView query, TestFunc test);
RankedMatch(StringView candidate, StringView query, TestFunc test, size_t input_order);

enum class Flags : int
{
Expand All @@ -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;
};

}
Expand Down

0 comments on commit 5806523

Please sign in to comment.