nushell
suggestion: added display
Full inserted value:
Display value:
TODO
- [x] fix column width calculation
related https://github.com/nushell/nushell/issues/5292 needs https://github.com/nushell/reedline/pull/691
@stormasm this one can be tested with https://github.com/nushell/nushell/pull/11444
@rsteube @fdncred Thanks for bringing this one up to speed too... Unfortunately we are waiting on this PR to land https://github.com/sharkdp/lscolors/pull/81 [As I am erroring on the side of caution and don't want to get too far ahead of ourselves] before we can move forward on landing any more Reedline PRs... I want to be able to test on the nushell side of the world and nushell is currently blocked from updating Reedline 😢
Yeah, i ran into that as well (duh!). Didn't expect that the patching does not work for the transitive dependency.
Now that I think about this some more I am hesitant to add another field to Suggestion... I don't want Suggestion to get bigger than it needs to be...
Why not just use the
pub extra: Option<Vec<String>>,
extra instead ?
It can be viewed as kind of an additional (generic) location to grab stuff from ?
And then you can just grab your display value from the first item in the extra vector.
And the end user of Carapace has no idea where that data is coming from anyway right ?
Thoughts on this... Maybe I am not understanding the concept completely...
Yeah I think there might be some misunderstanding.
The issue is that reedline doesn't discern between what is inserted and what is shown during completion.
When you cd /some/super/long/path/with/many long segments the menu becomes confusting at first and broken when it exceeds the available space.
Parent directories also don't really matter to the user (as those are visible on the command line), just the last completed segment is relevant.
Truncating isn't really an option. Fish does this and it gives awful results as it can't know where to make the cut. Completion scripts however do know this and just need a way to pass this to reedline.
The string vector extra seems pretty cryptic to be honest.
Completion scripts however do know this and just need a way to pass this to Reedline.
Thanks for helping me understand the problem better...
So if "extra" was called "display" but we kept it a Vector like it is right now would that work ?
instead of this...
/// Optional display value for the replacement
pub display: Option<String>,
do it this way...
pub display: Option<Vec<String>>,
Or to phrase it another way
Completion scripts however do know this and just need a way to pass this to Reedline.
Do the completion scripts care whether they are passing a String or a Vector of Strings ?
I kind of like the idea of having a display and a full path for certain situations. it makes it more intuitive like the screenshots show. i'd like to see an new example to demonstrate this better though.
I tried the nushell PR and expected to not see crates in all the suggestions.
@fdncred cd is the nushell completer which doesn't make use of this (yet?). Try bat <TAB> or something else.
cd is the nushell completer which doesn't make use of this (yet?). Try bat <TAB> or something else.
I guess what I'm saying is, if we're going to land this feature, nushell should support it, maybe with an option.
I'm not getting much fun with bat either.
Can't really make much sense of the vector here. What is it for?
I guess what I am saying is that if you think about the Suggestion as a Container in which to pass more generic type of information...
Where the end user of the Suggestion may have different needs in their application
The Style PR that we landed yesterday and this Display PR currently could have been represented in the "extra" Vector of Strings where the first field of the Vector was the "Style" and the second field of the Vector is the "Display"...
I believe that was the intention of the "extra" field in the first place.....
Because other applications will come along later and want other stuff in their Suggestions possibly...
But I could be wrong --- why is extra there anyway ---- what can it be used for ? Is it named properly ?
I think it's fine with the display member of the suggestion structure, but we don't want to have breaking changes either. @stormasm may have different suggestions.
@rsteube I am fine with moving forward with this PR as well... Please go ahead and resolve the conflicts... Thank you !
I'll probably be ok with this PR once I can see it work and understand if there are bugs or not. I can't get the nushell PR that references this PR to work. So, I'm glad these are draft. I'm also a tiny bit concerned that this is another breaking change.