Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove sorting of user-supplied insert completions #1709

Closed
aver-d opened this issue Nov 19, 2017 · 5 comments · Fixed by #5035
Closed

Remove sorting of user-supplied insert completions #1709

aver-d opened this issue Nov 19, 2017 · 5 comments · Fixed by #5035

Comments

@aver-d
Copy link
Contributor

aver-d commented Nov 19, 2017

Hi, for a plugin I'm writing I need insert-mode completions to be displayed in a pre-determined order.

The implementation for displaying user-generated completions in complete_option includes a sort

std::sort(matches.begin(), matches.end());

As far as I'm aware, there's no other way I can show the custom-generated completions without this sorting.

Would it be possible to either make the sort optional or simply remove this one line and leave sorting to the user if it's required?

@mawww
Copy link
Owner

mawww commented Nov 20, 2017

It would be tricky to remove/disable this sort, because this is not simply an alphabetical sort, its sorting candidates according to their fuzzy matching relevance. When no text has been written by the user to filter out candidates yet, this will be alphabetically sorted as that is the last condition when comparing two ranked matches, but as soon as some text has been written, the sorting will be based on the number of matching word boundaries, whether the user entered text is a prefix or just a subsequence, wether the matched part is contiguous...

Disabling this behaviour would be possible, after all we could just display candidates that matches the entered text without sorting them at all, but it would require a very strong motivating use case to add that complexity inside the insert completion code.

Can you describe our use case in more details ?

@aver-d
Copy link
Contributor Author

aver-d commented Nov 20, 2017

Hi, and thanks for the response.

In case I wasn't clear, I didn't mean for there to be any change in the way that kakoune currently operates – only that it be useful to have a choice to display a static list of completions candidates (provided by an external process) to be shown as originally supplied.

So, for example, set window my_completions ${header}:d||d:b||b:a||a:c||c could show in order d b a c, not a b c d.

You mention there are two stages: (1) an initial lexicographic sort and (2) subsequent fuzzy matching.

I had been considering the problem in a way that had seemed to me would result in less work for kakoune. Neither stage 1 nor 2 would occur. The user is presented with a list of candidates and can either select an item or make a change to the buffer. Either way, the list is then immediately cleared, so there's no opportunity for stage 2 fuzzy matching to occur – the external process instead just sends a new list which reflects the new buffer state. I'd been able to get new completions to appear inside a few milliseconds, so just recompiling and commenting out that sort line seemed enough to resolve what appears to be the only problem of this extra sort (Though I appreciate you're more familiar with kakoune's implementation and there may be further issues I hadn't encountered during my brief trial).

In terms of use cases, although initially ordering candidates alphabetically is normally a reasonable choice, another way would be likelihood of selection. One simple example might be a dictionary lookup of English words. An external process could provide a list of candidates for an input infor

Displaying with an ordering based on google's n-gram data frequency...

information     285127664
informed         34787210
informal         15413598
inform           12294536

may be more useful than the conversion to a lexicographic ordering...

inform
informal
information
informed

Of course, there's no way for kakoune to know what would be a “best” ordering for each and every situation. So, to give flexibility for all cases it'd be nice to have an extra option which would make kakoune responsible solely for displaying and selecting candidates, with no additional logic.

@mawww
Copy link
Owner

mawww commented Nov 21, 2017

Ok, I think I got a better understanding of what you are trying to achieve.

I see two possible directions to provide that features:

  • Add a new type of completer, say static-option=<option-name> supported in the completers option, which only appears when you are exactly at the position specified in the option, and hence does not need any kind of fuzzy matching/sorting.
  • Add a way to specify a sorting key to completion candidates that would have priority over lexicographical sorting. In the given example, all candidates will match equaly well for the fuzzy matcher, as they are all prefix-matched by infor, if we had a way to pass the n-gram data frequency (or whatever other value the completion script decides to give to the completion candidates) and have that value being compared in priority to lexicographical comparison, we would get the result you want.

However, to be more precise, there arent two stages to completion sorting, we just sort ranked matches against each other, and the comparison operator between two ranked match will compare various properties (number of word boundaries matched, is it a prefix, a full match...), when all those properties are equal, the last comparison that happens is lexicographical. This is all done in the same operation, its just the definition of RankedMatch::operator< which is pretty complex.

@Screwtapello
Copy link
Contributor

I bumped into this the other day, with my predictive-text plugin. I could have sworn that this worked as I expected (candidates presented in given order) when I first made the plugin in 2021-06, but I checked out some revisions from around that time and I couldn't reproduce that behaviour - Kakoune sorted the completions every time.

For the record, here's the test-case I came up with:

echo |
    kak -n -e '
        declare-option completions mycomp
        set-option global completers option=mycomp
        set-option global mycomp 1.1@1 bbbbb||bbbbb zzzzz||zzzzz aaaaa||aaaaa
        exec i
    '

Expected result:

  • An empty buffer with a completion menu listing bbbbb, zzzzz, aaaaa

Actual result:

  • An empty buffer with a completion menu listing aaaaa, bbbbb, zzzzz

I see two possible directions to provide that features...

The most general solution would be to add a "weight" field to the completion list, and let the second-last test compare weights before the last lexicographical comparison. However, the syntax of "completions" options is already pretty dense, I'm not sure if there's a safe way to squeeze an optional "weight" field in there or if we'd want to break compatibility and just make it 4 required fields (text, select command, menu text, weight).

If there were some way to make the weight automatically be "distance from the end of the list" (so the first item has the highest weight and the last item has the lowest), that would basically Do The Right Thing so far as I can tell.

krobelus added a commit to krobelus/kakoune that referenced this issue Dec 22, 2022
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
krobelus added a commit to krobelus/kakoune that referenced this issue Dec 22, 2022
Closes mawww#1709

This also allows supporting LSP's sortText (to a reasonable extent).
@krobelus
Copy link
Contributor

I have an experimental change that might do the trick, see #4813

krobelus added a commit to krobelus/kakoune that referenced this issue Dec 22, 2022
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
krobelus added a commit to krobelus/kakoune that referenced this issue Dec 22, 2022
Closes mawww#1709

This also allows supporting LSP's sortText (to a reasonable extent).
krobelus added a commit to krobelus/kakoune that referenced this issue Dec 30, 2022
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
krobelus added a commit to krobelus/kakoune that referenced this issue Dec 30, 2022
Closes mawww#1709

This also allows supporting LSP's sortText (to a reasonable extent).
krobelus added a commit to krobelus/kakoune that referenced this issue Mar 5, 2023
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
krobelus added a commit to krobelus/kakoune that referenced this issue Mar 5, 2023
Closes mawww#1709

This also allows supporting LSP's sortText (to a reasonable extent).
krobelus added a commit to krobelus/kakoune that referenced this issue Apr 16, 2023
…ript 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<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
krobelus added a commit to krobelus/kakoune that referenced this issue Apr 16, 2023
…options

Closes mawww#1709

This also allows supporting LSP's sortText (to a reasonable extent).
krobelus added a commit to krobelus/kakoune that referenced this issue May 19, 2023
…ript 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<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
krobelus added a commit to krobelus/kakoune that referenced this issue May 19, 2023
…options

Closes mawww#1709

This also allows supporting LSP's sortText (to a reasonable extent).
krobelus added a commit to krobelus/kakoune that referenced this issue Nov 18, 2023
…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.

Closes mawww#1709, mawww#4813

TODO: this does it for all completions, can do for user-specified ones only.
krobelus added a commit to krobelus/kakoune that referenced this issue Nov 19, 2023
…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.

Closes mawww#1709, mawww#4813
krobelus added a commit to krobelus/kakoune that referenced this issue Nov 20, 2023
…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 mawww#1709, mawww#4813
krobelus added a commit to krobelus/kakoune that referenced this issue Dec 2, 2023
…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 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 mawww#1709, mawww#4813
krobelus added a commit to krobelus/kakoune that referenced this issue Dec 2, 2023
…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 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 mawww#1709, mawww#4813
krobelus added a commit to krobelus/kakoune that referenced this issue Dec 2, 2023
…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 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 mawww#1709, mawww#4813
evannjohnson pushed a commit to evannjohnson/kakoune-ev that referenced this issue Jan 25, 2024
…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 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 mawww#1709, mawww#4813
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants