Support EnableColorSelection in Terminal

[jazzdelightsme]

Support EnableColorSelection feature in Terminal

The legacy conhost has a (somewhat hidden, I think) feature called "color selection", which allows you to select text and then use some keystrokes to change the fg/bg color of the selected text (including optional search). (for a description, see here)

This feature is not on by default, and must be enabled by setting a registry value.

I LOVE this feature and use it all the time (and when people see me use it, they all want to know how to turn it on). Hence I would love to port this feature to Terminal as well.

Proposed technical implementation details

I think this would probably need to be disabled by default, and only enabled by a profile setting. Based on a recent comment by Dustin, I believe he has an aversion to things that manipulate the buffer, but I don't know why that might be considered undesirable, or what other options might be--if you want this particular selection to be green, then you have to store that information somewhere--why not in the buffer; if not in the buffer, then where?

If the team is not opposed to having optional support for this feature, I'd be interested in taking a stab at it. I would appreciate any pointers to help me get a toe-hold in the code.

[zadjii-msft]

For my future reference: Selection::_HandleColorSelection

I'm definitely of the same mind as Dustin in this case - I'm extremely reluctant to allow the user to modify the buffer contents like this.

• If the region being changed is in the mutable viewport (at the "bottom" of the buffer), then we'd need to communicate these changes back to the underlying conpty as well. I'm not sure there's a VT sequence currently for "set the attributes in this region to this value". We could co-opt that as an input sequence, but it's definitely extremely hacky.
  • This of course wouldn't work if the thing on the other side wasn't a conpty. I guess in that case we wouldn't have to worry too much - if it's not a conpty, then the client app won't be reading the contents of the buffer anyways.
  • Though, if the terminal had like, a direct connection to a linux machine that was then ssh'ing to a Windows machine, then it's
    Terminal <--direct connection --> linux <-- ssh --> Windows sshd (using conpty)
    and then the Terminal is connected to a conpty, even if it doesn't know it.
• If the region being changed is in the scrollback, then I suppose we don't have to worry too much - that won't be in the conpty buffer, so we don't need to worry about it round-tripping

It might make more sense to not handle this in the conpty buffer itself, but maybe as something else entirely - like another layer of attributes applied on top of the ones in the buffer. We had discussed in the past something like having "overlay" buffers - maybe we could do something like that to "overlay" these color regions on the Terminal buffer contents. Then we wouldn't need to worry about conpty at all.

The original UX in conhost is also incredibly undiscoverable. I'd also think that if we implement this in the Terminal, we could come up with a better UI for this, not necessarily implement it exactly the same.

[j4james]

• If the region being changed is in the mutable viewport (at the "bottom" of the buffer), then we'd need to communicate these changes back to the underlying conpty as well.

If we ever get around to reimplementing conpty so it's purely passing through the VT sequences, then it would never need to "redraw" the mutable viewport, so we wouldn't need to communicate the changes back to the conhost side. I think that's the only way a feature like this would be practical (unless of course you went with a completely different approach - like that overlay idea).

• I'm not sure there's a VT sequence currently for "set the attributes in this region to this value". We could co-opt that as an input sequence, but it's definitely extremely hacky.

There is such a sequence (DECCARA), but I wouldn't think it appropriate to use as an input sequence.

For the record, I'm neither for or against this feature. I've used it a few times as a party-trick to amaze friends and family, but it's not something I really need.

[jazzdelightsme]

It is a good party trick, right? :D

then we'd need to communicate these changes back to the underlying conpty as well.

For sake of discussion (and my education): what if we didn't?

I'm guessing the "bad thing" that could happen is an app asks about buffer contents at some (now-modified by the user) location, Terminal gives back the [modified] content, and the app says "what the????". Is that the problem/concern? (Or is it something else?)

My mental model of this feature is that it really is a UI feature "on top", so maybe the "overlay" idea really is the way to go. On the other hand, if we had an "export the entire buffer" feature (to file, or HTML, or whatever), I think I would want the user's modifications to be included... so it seems like actually modifying the buffer would be most straightforward for that.

Maybe most apps just don't really need to know that the user is coloring on their buffer? And if you have some apps that ARE sensitive to this, you just don't enable this feature? (or use it when using these apps)

Re: discoverability: I don't think there is anything so much worse about this feature than most. Just to figure out tabs and panes I had to read the documentation like for any other feature (and was frustrated and confused when the documentation was wrong, ha), so I don't see how EnableColorSelection is any different.

[zadjii-msft]

what if we didn't?

Conpty tries it's absolute best to not repaint things that haven't actually changed, but it isn't always the best at that. If you only change the terminal buffer and not the underlying conpty one, then it's absolutely possible that conpty might decide to repaint a section that had the attributes changed, and blow away the modifications that only reside in the terminal. Hence why we might want to go with the overlay route - just leave the underlying buffer out of the question.

[j4james]

It just occurred to me that the overlay concept doesn't really solve this problem. Imagine you've got some VT margins set, and you highlight a piece of text somewhere within the margin area, then you also scroll that area. Conpty is just going to redraw that area of the screen, but with all the content moved up one line. There's no way for the terminal to know that a scroll operation occurred, so it'll either draw the overlay in the wrong place, or maybe just erase it. Either way it's not what you want.

[zadjii-msft]

Hack project number 6:

Relevant diff:

d364eb4...dev/migrie/fhl/9583-colorSelection

I'm probably gonna leave it at this for now, and tag it up as help wanted. This is definitely the "just change the Terminal buffer contents and don't worry about conpty" version. Works.... well enough.

• I had a mind to make SelectionColor accept either a index in the color table or a RGB color, but only got so far as a color. Hence the weird uint32_t in there
• I didn't really validate the string parsing
• I wanted FG/BG to be optional/nullable, but I don't think that's gonna be possible, a TextAttribute needs both. So we should validate they're both not null, or maybe just use the Default FG/BG when omitted.
• Didn't bother with formatting the action name at all. Clearly, it's just got random garbage numbers in it.

There's also a bunch of other stuff that's in Selection::_HandleColorSelection that I didn't do yet.

If someone rounded out the corners, we'd probably take this as it is, as the experimental.colorSelection action, without having to worry about all the overlays, etc. Especially considering the scroll margins issue.

[jazzdelightsme]

I had a mind to make SelectionColor accept either a index in the color table or a RGB color

Yes, I think this is important, because if I select some text and type alt+6, then I expect the text to become red, and not just any red, but the same red as is used by my current color theme.

I think this would be doable by sneaking a flag in the alpha channel (because we don't have access to TextColor or such things from the settings layer). And furthermore, I think it could be done in a way that basically only affects "color selection". And indexed colors would use a different syntax: so you'd say "#RRGGBB" for RGB, or "iNN" for an index.

Like so: see dev/migrie/fhl/9583-colorSelection...jazzdelightsme:user/danthom/ecs_demo_indexedColors

This allows perfect reproduction of the ctrl/alt combos from legacy conhost, with keybindings like:

    {
        "keys": "alt+1",
        "command": { "action": "experimental.colorSelection",
                     "foreground": "i07", "background": "i00" }
    },
    {
        "keys": "alt+2",
        "command": { "action": "experimental.colorSelection",
                     "foreground": "i08", "background": "i00" }
    },
    {
        "keys": "alt+3",
        "command": { "action": "experimental.colorSelection",
                     "foreground": "i09", "background": "i00" }
    },
    {
        "keys": "alt+4",
        "command": { "action": "experimental.colorSelection",
                     "foreground": "i0a", "background": "i00" }
    },
    {
        "keys": "alt+5",
        "command": { "action": "experimental.colorSelection",
                     "foreground": "i0b", "background": "i00" }
    },
    {
        "keys": "alt+6",
        "command": { "action": "experimental.colorSelection",
                     "foreground": "i0c", "background": "i00" }
    },
    // etc.
    // background
    {
        "keys": "ctrl+1",
        "command": { "action": "experimental.colorSelection",
                     "background": "i07", "foreground": "i00" }
    },
    {
        "keys": "ctrl+2",
        "command": { "action": "experimental.colorSelection",
                     "background": "i08", "foreground": "i00" }
    },
    // etc.

@zadjii-msft: any objection to this approach?

[ghost]

🎉This issue was addressed in #13429, which has now been successfully released as Windows Terminal Preview v1.16.252.:tada:

Handy links:

• Release Notes
• Store Download