-
Notifications
You must be signed in to change notification settings - Fork 197
Allow overriding suggestion display value #1002
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
base: main
Are you sure you want to change the base?
Conversation
|
That lolcat demo is great! 🤣 I'm up for giving this a try! |
|
Great! Do you want to merge it or ask someone else to look over it? |
|
If there's someone else you can get to review it that would be great. Otherwise we could just land it and see what happens. We have a release this weekend so we could also wait until after that depending on your comfort level. |
|
Yeah, let's wait. There's a good chance this'll break stuff. I'll dogfood it for a bit before merging. @maxomatic458 and @blindFS If you have time, could you guys review this PR and/or try it out? Would appreciate any feedback you have since I know you've both spent a bunch of time on completions and menu stuff |
I'm confused at this part. Do you mean display value (ansi striped) based partial completion is not good enough? Haven't looked at those ansi related changes, I can't find any issue except for the per-completer partial completion mentioned. BTW, with matching indices enabled, I think we are already safe to remove quotes stripping in reedline menus. But the idea of display-value certainly works better in those cases. |
This PR adds an optional
display_overridefield toSuggestion. If set, the string set in this field will be displayed to users rather than the actualvalueof the suggestion. However, when a suggestion is chosen, itsvaluewill be put into the buffer, not itsdisplay_override.Idea came from this comment by @qfel on an older PR.
Why?
Reason 1: On the Nushell side, we often have to provide completions wrapped in quotes. This can cause problems on the Reedline side. We strip quotes in the columnar and IDE menus to account for this when figuring out which substring to highlight. I'm not sure if the quote stripping is necessary anymore since we're doing substring matching anyway, but it doesn't handle escapes in strings. With
display_override, we can show suggestions that don't contain any quotes or backslashes while still keeping the suggestions'values the same.Relatedly,
find_common_string, which is used for partial completions, is unable to compare file names wrapped in quotes to file names not wrapped in quotes. But here, the best solution here would be to allow completers to provide their own partial completions, which I plan to implement soon. Right now, partial completions are still based onvalue, not display valueReason 2: Allowing Nushell users to customize their completions. The new field allows including ANSI highlights, so we can now let users create custom/external completers that provide suggestions with
display_overrides styled however they want. They can get styled strings from some other source and provide those to Nushell.Testing
As an example, here's a custom completer that uses lolcat. I just used lolcat for a laugh, but the lolcat colors look surprisingly good:
https://asciinema.org/a/CTmdQR9nboLjm5zn?t=4
Completer:
def comp [] { let completions = ^ls crates/*/*/*.rs | lolcat -f | lines | each {|file| { value: ($file | ansi strip), display_override: $file } } { completions: $completions, options: { completion_algorithm: "fuzzy" } } } def foo [ arg: string@comp ] {}