fix: Fuzzy completion with Unicode chars #887
Conversation
|
Temporarily converting this to a draft because I don't think this checks for suggestions shorter than the text being completed. Reported here: reubeno/brush#406. |
|
Just threw in a test to make sure that suggestions shorter than the text being matched don't cause a panic. Should be ready to go now. |
|
I'd like to bring another (probably) related issue nushell/nushell#4595 into the sight here. @uek-1 would know it better. |
@blindFS I'm not sure I understand. Isn't handling quotes in partial completions a related but separate matter? |
Yes, related but separate, just in case we need some top-down redesign to address them at the same time. To be clear, that issue is mainly about common prefix completion (which works if both items are not quoted). I mean we should probably strip the quotes when extracting the common prefix. |
There was a problem hiding this comment.
Thanks for tackling this complexity.
I don't quite follow why this code has to use the vague oracle of UnicodeWidth and can't simply use the grapheme/byte indices etc. which are the relevant unit for where we actually perform the highlighting/string splitting etc.
src/menu/menu_functions.rs
Outdated
| pub fn split_suggestion(sugg: &str, match_width: usize) -> (&str, &str) { | ||
| let mut match_end = sugg.len(); | ||
| let mut curr_width = 0; | ||
| for (offset, grapheme) in sugg.grapheme_indices(true) { | ||
| if curr_width >= match_width { | ||
| match_end = offset; | ||
| break; | ||
| } | ||
| // Strip quotes from the beginning | ||
| if offset == 0 && (grapheme == "`" || grapheme == "'" || grapheme == "\"") { | ||
| continue; | ||
| } | ||
| curr_width += grapheme.width(); |
There was a problem hiding this comment.
I guess with this code doing operations on Unicode width would not panic.
But I have a question if this misbehaves if the user tries to complete starting from " etc. or " being a particular suggestion? (Is this relying on a particular behavior of our Completer implementations to take out all quotes?)
There was a problem hiding this comment.
I guess with this code doing operations on Unicode width would not panic.
But I have a question if this misbehaves if the user tries to complete starting from
"etc. or"being a particular suggestion? (Is this relying on a particular behavior of ourCompleterimplementations to take out all quotes?)
True, that's one of reasons why this is just a temporary fix.
There was a problem hiding this comment.
Yeah, this won't work well if a quote is part of what the user types. As blindFS said, it's a temporary fix. I was planning on allowing adding a separate display value on Suggestions, so that when showing suggestions for files, the displayed values could be unquoted and unescaped. Then, Reedline would no longer have to strip quotes.
This won't solve the problem of getting a common prefix that accounts for quotes, though, so that's going to need some more thought. One obvious thing I can think of is pushing the burden of finding a common prefix onto completers.
This current quote stripping is really for Nushell, and I assume it won't hurt other users of Reedline either.
I just used width so that when displaying, the underlined sections would be (around) the same width. But yeah, looking at graphemes makes more sense, I've updated it now |
This PR fixes a panic that happens when trying to display fuzzy-matched suggestions. Previously, #886 fixed this for prefix matching. Closes nushell/nushell#12680 and nushell/nushell#15302.
This is temporary, until we start highlighting fuzzy-matched suggestions properly (underlining only the graphemes that were matched, rather than underlining a prefix of the suggestion).
I chose to operate on graphemes rather than characters because that's what Nucleo operates on and because they seem to correspond better to a block of text shown on the screen. First we get the width of the shortest string, and then for every suggestion, we highlight the suggestion up to the last grapheme that matches or exceeds this width.