-
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 "faint" graphic rendition attribute #6873
Conversation
…nt on the color calculations.
…and resetting the faint attribute.
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.
This is great.
I wonder if, if we ever do get perceptual grading going, there's value in making faint more faint (=harder to perceive) rather than just darker.
Given that darkening is comparable to other terminal emulators, it's more of a curiosity than a desire. 😄
Thanks for doing this.
Btw, here are a couple of screenshots comparing how other terminals handle this. From left to right, top to bottom: XTerm, VTE (Gnome Terminal), Mintty, and then us. In each case I've got a color test pattern rendered with normal attributes first, and with a faint version below. I think XTerm and VTE are a bit brighter than us, but Mintty looks almost identical. The overall effect is about the same though. |
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.
Fantastic as always. Thanks!
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 (
|
🎉 Handy links: |
Summary of the Pull Request
This PR adds support for the
SGR 2
escape sequence, which enables theANSI faint graphic rendition attribute. When a character is output
with this attribute set, it uses a dimmer version of the active
foreground color.
PR Checklist
Detailed Description of the Pull Request / Additional comments
There was already an
ExtendedAttributes::Faint
flag in theTextAttribute
class, but I needed to addSetFaint
andIsFaint
methods to access that flag, and update the
SetGraphicsRendition
methods of the two dispatchers to set the attribute on receipt of the
SGR 2
sequence. I also had to update the existingSGR 22
handler toreset Faint in addition to Bold, since they share the same reset
sequence. For that reason, I thought it a good idea to change the name
of the
SGR 22
enum toNotBoldOrFaint
.For the purpose of rendering, I've updated the
TextAttribute::CalculateRgbColors
method to return a dimmer version ofthe foreground color when the Faint attribute is set. This is simply
achieved by dividing each color component by two, which produces a
reasonable effect without being too complicated. Note that the Faint
effect is applied before Reverse Video, so if the output it reversed,
it's the background that will be faint.
The only other complication was the update of the
Xterm256Engine
inthe VT renderer. As mentioned above, Bold and Faint share the same
reset sequence, so to forward that state over conpty we have to go
through a slightly more complicated process than with other attributes.
We first check whether either attribute needs to be turned off to send
the reset sequence, and then check if the individual attributes need to
be turned on again.
Validation
I've extended the existing SGR unit tests to cover the new attribute in
the
AdapterTest
, theScreenBufferTests
, and theVtRendererTest
,and added a test to confirm the color calculations when Faint is set
in the
TextAttributeTests
.I've also done a bunch of manual testing with all the different VT color
types and confirmed that our output is comparable to most other
terminals.