fix(autocomplete): compare whole completions when removing duplicates - #6004
Open
boogie wants to merge 1 commit into
Open
fix(autocomplete): compare whole completions when removing duplicates#6004boogie wants to merge 1 commit into
boogie wants to merge 1 commit into
Conversation
Duplicates are dropped by comparing each completion with the one before it, which only holds while equal completions stay adjacent. They are sorted by score first, so a duplicate scoring differently from its twin ends up separated by another completion and both reach the popup. Two completers offering the same word with different scores is enough to see it. The comparison is also too coarse in the other direction. It reads `snippet || caption || value`, so snippets are compared by their body alone and two snippets expanding to the same text collapse into one however differently they are captioned. The html mode ships fourteen `doc` snippets that expand to only eight distinct bodies, and three of them are dropped from the popup today. Compare what the completion reads as together with what it inserts, and track every completion kept rather than only the previous one. Completions that look the same in the popup and insert the same text are still merged, and of a set of duplicates the first after sorting survives, which is the highest scored.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
FilteredList.setFilterremoves duplicates by comparing each completion with the one immediately before it. That is only correct while equal completions stay adjacent, and they do not: the list is sorted by score first, so a duplicate scoring differently from its twin ends up separated by another completion and both reach the popup.Two completers offering the same word with different scores is enough:
The comparison is too coarse in the other direction as well. The key is
snippet || caption || value, so snippets are compared by their body alone, and two snippets expanding to the same text collapse into one however differently they are captioned. Ace's own html mode ships fourteendocsnippets that expand to only eight distinct bodies — three of them are dropped from the popup today, and the survivor of each pair is whichever sorts first alphabetically.Change
Compare what a completion reads as in the popup together with what it inserts, and track every completion kept rather than only the previous one.
docprefixCompletions that look the same in the popup and insert the same text are still merged, which is what keeps the keyword and text completers from offering the same word twice. Of a set of duplicates the first one after sorting survives, which is the highest scored — the same one that survived before.
The
Setreplaces a single variable in a function that already sorts the same array immediately above, so the added cost is not measurable against whatsetFilterdoes per keystroke.Test
test: duplicate completions are removed regardless of their positioncovers both directions at once, and asserts the surviving duplicate is the higher scored one. Against the current code it fails with:—
footwice, andsnippet bmissing.Open kitchen-sink @ 2a773af408f8dfa2c8f61df011b845662b2c4e02