chrisant996 / clink

Bash's powerful command line editing in cmd.exe
https://chrisant996.github.io/clink/
GNU General Public License v3.0
3.63k stars 143 forks source link

If color.histexpand is empty, the "History expansion!" info in line below isn't shown #419

Closed Destroy666x closed 1 year ago

Destroy666x commented 1 year ago

I had color.histexpand empty for some reason, I don't recall setting that myself, but maybe I did by accident. Regardless, I don't think displaying what the expansion will be should rely on coloring the expansion in input line when history.auto_expand is set to True. "color" type of setting should not alter a non-color functionality, perhaps another setting like history.show_expansion_at_cursor would be suitable.

chrisant996 commented 1 year ago

I had color.histexpand empty for some reason, I don't recall setting that myself, but maybe I did by accident. Regardless, I don't think displaying what the expansion will be should rely on coloring the expansion in input line when history.auto_expand is set to True. "color" type of setting should not alter a non-color functionality, perhaps another setting like history.show_expansion_at_cursor would be suitable.

How would you propose for it to convey to the user which characters will be expanded, without applying any color/styling to the input line?

Destroy666x commented 1 year ago

The cursor position does it. The styling does not as there can be multiple expansions in a line and they're all styled while only one expansion is explained below. And user can also just set it to regular either by mistake or on purpose and that will have the same styling effect as empty value, but the line will be shown.

chrisant996 commented 1 year ago

The cursor position does it. The styling does not as there can be multiple expansions in a line and they're all styled while only one expansion is explained below.

I wonder if you may have missed that is it contextual?

The color.histexpand is applied only to the specific text under the cursor. If there are 20 pieces of text that will be expanded by history expansion, only the one currently under the cursor is highlighted. UPDATE: oh, haha, I forgot that I made it highlight all of them. But I still don't agree that cursor position sufficiently identifies which text will be expanded. But maybe when color.histexpand is blank then the comment row could say History expansion for "!1": blahblahblah.

The primary intended purpose is to show when what you've just typed will turn into something else. The reason it was added was primarily to help users who don't know history expansion even exists to realize that the ! (or etc) they just typed might have an unexpected effect (and you'll notice that ! doesn't necessarily cause expansion; the text around it matters).

And if (and only if) it shows which text will get expanded, then it can also show what it will be expanded to.

And user can also just set it to regular either by mistake or on purpose and that will have the same styling effect as empty value, but the line will be shown.

"But the line will be shown" -- are you talking about showing a preview of the entire line? The longer the typed line and expanded line become, the less functional that would become. But that's a very different kind of feature than showing a contextual expansion of what's under the cursor.

Think of it as a toolip that's anchored to some text. Since it cannot be a floating tooltip "near the anchor", there needs to be a way to indicate what text the tooltip is anchored to. The color.histexpand is used to show what the tooltip is anchored to.

chrisant996 commented 1 year ago

My concern about adding history.show_expansion_at_cursor is that it takes what was conceptually a single feature, with a specific purpose, and breaks it into multiple separate features that each have to be enabled individually to get the original intended feature.

chrisant996 commented 1 year ago

...but I guess that history.show_expansion_at_cursor could be enabled by default, and the preview "tooltip" could say History expansion for "!1": whatever when color.histexpand is empty.

Ok fine, I'll add that mainly because of Accessibility reasons (e.g. when colors can't be used for vision or neurological reasons or etc). I'm still not convinced by the stated argument, but talking through it has helped me recognize other reasons exist, which I find more compelling.

Destroy666x commented 1 year ago

I still don't see how it's contextual. This is how I setup the color - 208 light orange image

This is when cursor is in the middle - all expansions are highlighted, but the middle one shows the command from above. image

This is when the cursor is on the 1st one. image

This is on latest clink. If it's supposed to be contextual then that's broken. But I think it's fine that it's not contextual and that the syntax is highlighted, as a user I would expect that rather than the behavior you're describing, to not miss any expansions to highlight/check.

Destroy666x commented 1 year ago

Oh, I see you edited they're supposed to be all highlighted. Good then - I think that's a lot more useful and the cursor does its job of showing the current one much better.

chrisant996 commented 1 year ago

Commit 7192e28908dda500a843f327888a20ce2999c25b.