-
Notifications
You must be signed in to change notification settings - Fork 8.4k
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
Add support for the DECAC escape sequence #13058
Conversation
You cool if this misses the 1.14 release train? We're trying to stabilize that build up right now for release in about a weekish. That might mean a little bit delayed review on this one 😬 |
@zadjii-msft Not a problem at all. I only did this now because I thought it might be a quick route to replacing the PowerShell VT quirk, and then we wouldn't have to worry about fixing #13037. But it turns out that's going to be a little more complicated than I had thought, so for now this is just a fun feature. |
You sure got that right |
The border color can be implemented in conhost with |
That's a nice idea. I'll have a play around with that when I get a chance and see how it looks. |
FYI, I tried to get the border color working in conhost, only to discover that those particular DWM attributes are only supported on Windows 11, and I'm still on Windows 10. So I'm going to leave that for someone else to follow up on. |
} | ||
else | ||
{ | ||
_tabColor = settings.TabColor().Value(); | ||
_renderSettings.SetColorTableEntry(TextColor::FRAME_BACKGROUND, til::color{ settings.TabColor().Value() }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't 100% understand the color table vs alias thing right now, but should this use SetColorAlias
to both (1) point FrameBackground
at FRAME_BACKGROUND
and (2) set FRAME_BACKGROUND
to the new tab color?
Or is the intent that once redirected via DECAC
, user-specified tab color cannot override application-specified tab color?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(And keeping with my (potential mis-)understanding of color aliases:) If that is the intent, then updating only the FRAME_BACKGROUND
color but not re-pointing FrameBackground
back to it is in line with that intent (?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm glad you raised this, because I had meant to bring it up myself. I'm open to changing this, but I did it this way because it lets you map the frame background alias to the default background color table (using DECAC 2;261;262
), and then switch your color scheme in the settings and still have both remain in sync. If we reset the frame alias here then we lose that ability.
But further down the line I'm assuming you might add an option for users to explicitly request that behavior (to support #702), and then the decision as to what should happen here would probably be controlled by that setting.
Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does that make sense?
100%. We can refine this if/when we add this part of 702. For now, I like how it is. Thanks!
Mike may be changing the default s.t. Terminal always acts like DECAC 2;261;262
over in #12992, or shortly after. /cc @zadjii-msft for this discussion.
If you wouldn't mind documenting these in an issue, I'd appreciate it! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again!
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
Playing around with this now and I love it! I also somewhat wonder if there's a place for a terminal emulator to extend it. Like, eventually we'll have customizable cursor colors (akin to xterm OSC 12 or whichever it is), but DECAC only specifies text and frame. It puts me in mind of DECSET/DECRST (and I must admit that I don't understand why DECSET's modes aren't just part of SM...) |
Possibly, but I'm not sure the cursor color is a good example of something you'd want to link to an existing color index, rather than just giving it an RGB value. In general, though, I'm not opposed to the idea if there was a good use case for it.
Technically There are only a few possible prefixes, so there were bound to be conflicts, but modern terminal emulators didn't even make an attempt to segregate their modes. They just lumped everything into the DEC category, often overwriting existing DEC modes, so it's a bit of mess at this point. So coming back to the idea of extending |
🎉 Handy links: |
This PR adds support for querying the color indices set by the `DECAC` control, using the existing `DECRQSS` implementation. ## References and Relevant Issues The initial `DECRQSS` support was added in PR #11152. The `DECAC` functionality was added in PR #13058, but at the time we didn't know how to format the associated `DECRQSS` query. ## Detailed Description of the Pull Request / Additional comments For most `DECRQSS` queries, the setting being requested is identified by the final characters of its escape sequence. However, for the `DECAC` settings, you also need to include a parameter value, to indicate which color item you're querying. This meant we needed to extend the `DECRQSS` parser, so I also took this opportunity to ensure we correctly parsed any parameter prefix chars. We don't yet support any setting requiring a prefix, but this makes sure we don't respond incorrectly if an app does query such a setting. ## Validation Steps Performed Thanks to @al20878, we've been able to test how these queries are parsed on a real VT525 terminal, and I've manually verified our implementation matches that behavior. I've also extended the existing `DECRQSS` unit test to confirm that we are responding to the `DECAC` queries as expected. Closes #13091
The
DECAC
(Assign Colors) escape sequence controls which color tableentries are associated with the default foreground and background
colors. This is how you would change the default colors on the the
original DEC VT525 terminals.
But
DECAC
also allows you to assign the color table entries for the"window frame", which in our case is mapped to the tab color (just the
background for now). So this now gives us a way to control the tab color
via an escape sequence as well.
DETAILS
The way this works is there are now two new entries in the color table
for the frame colors, and two new aliases in the color alias table that
are mapped to those color table entries. As previously mentioned, only
the background is used for now.
By default, the colors are set to
INVALID_COLOR
, which indicates thatthe system colors should be used. But if the user has set a
tabColor
property in their profile, the frame background will be initialized with
that value instead.
And note that some of the existing color table entries are now
renumbered for compatibility with XTerm, which uses entries 256 to 260
for special colors which we don't yet support. Our default colors are
now at 261 and 262, the frame colors are 263 and 264, and the cursor
color is 265.
So at runtime, you can change the tab color programmatically by setting
the color table entry at index 262 using
OSC 4
(assuming you need aspecific RGB value). Otherwise if you just want to set the tab color to
an existing color index, you can use
DECAC 2
.You can even make the tab color automatically match background color by
mapping the frame background alias to the color table entry for the
default background, using
DECAC 2;261;262
(technically this is mappingboth the the foreground and background).
This PR doesn't include support for querying the color mapping with
DECRQSS
, and doesn't support resetting the colors withRIS
, buthopefully those can be figured out in a future PR - there are some
complications that'll need to be resolved first.
Validation Steps Performed
I've added a basic unit test that confirms the
DECAC
escape sequenceupdates the color aliases in the render settings as expected. I've also
manually confirmed that the tab color in Windows Terminal is updated by
DECAC 2
, and the default colors are updated in both conhost and WTusing
DECAC 1
.Closes #6574