completions: Highlight fuzzy matching #2190
Conversation
| .fuzzy_indices(text, pattern) | ||
| .map(|(score, fuzzy_indices)| (index, score, fuzzy_indices)) |
There was a problem hiding this comment.
This is very slow and expensive for large datasets. We should do what we do in the file picker: use fuzzy_match to score, then use fuzzy_indices inside the render method to only compute the indices for elements on screen.
There was a problem hiding this comment.
In this case the render function relies on Table::render_table function. I can do that, but it would require passing more information to the latter (the highlighted style and the prompt), making it less general. Is it okay?
There was a problem hiding this comment.
Sorry for the delay, https://github.com/helix-editor/helix/pull/3053/files#diff-b6c51d6e6645b52604abc7b9cd90c0200ab42d52be2f7e623929376971e784c6R641-R715 has a similar implementation where it patches the spans while rendering. We should be able to reuse the code once that PR is merged
There was a problem hiding this comment.
I see, reusing the code would definitely be the best. Unfortunately that PR hasn't seen much progress in the last couple of months; if you prefer I can pick it up and continue where it was left.
There was a problem hiding this comment.
@danyspin97 #3053 has been merged! That should help this PR be able to move further.
There was a problem hiding this comment.
@LeoniePhiline Thanks for the heads up :) I have also updated #1905 so this PR should be close to being merged.
the-mikedavis
added
the
S-waiting-on-author
the-mikedavis
added
S-waiting-on-author
the-mikedavis
added
S-waiting-on-pr
the-mikedavis
added
S-waiting-on-review
pascalkuthe
added
S-inactive
|
closing this one out as stale. Thank you for contributing! |
Requires #1905 .
I had to create a vector for every completion to add the spans, but I didn't notice any slowdown (even with
idle-timeout = 0). The loop part is only slightly longer for non consecutive matches, but in this case the matching rows are just few ones, so there is no performance impact.Preview:
