Skip to content
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 "concealed" graphic rendition attribute #6907

Merged
3 commits merged into from
Jul 14, 2020

Conversation

j4james
Copy link
Collaborator

@j4james j4james commented Jul 13, 2020

Summary of the Pull Request

This PR adds support for the SGR 8 and SGR 28 escape sequences,
which enable and disable the concealed/invisible graphic rendition
attribute. When a character is output with this attribute set, it is
rendered with the same foreground and background colors, so the text is
essentially invisible.

PR Checklist

Detailed Description of the Pull Request / Additional comments

Most of the framework for this attribute was already implemented, so it
was just a matter of updating the TextAttribute::CalculateRgbColors
method to make the foreground the same as the background when the
Invisible flag was set. Note that this has to happen after the
Reverse Video attribute is applied, so if you have white-on-black text
that is reversed and invisible, it should be all white, rather than all
black.

Validation Steps Performed

There were already existing SGR unit tests covering this attribute in
the ScreenBufferTests, and the VtRendererTest. But I've added to the
AdapterTest which verifies the SGR sequences for setting and resetting
the attribute, and I've extended the TextAttributeTests to verify that
the color calculations return the correct values when the attribute is
set.

I've also manually confirmed that we now render the concealed text
values correctly in the ISO 6429 tests in Vttest. And I've manually
tested the output of concealed when combined with other attributes,
and made sure that we're matching the behaviour of most other terminals.

@ghost ghost added Area-VT Virtual Terminal sequence support Issue-Task It's a feature request, but it doesn't really need a major design. Priority-2 A description (P2) Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal. labels Jul 13, 2020
@j4james j4james marked this pull request as ready for review July 13, 2020 23:19
@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jul 13, 2020
@ghost
Copy link

ghost commented Jul 13, 2020

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 8 hours, a condition that will be fulfilled in about 7 hours 15 minutes. No worries though, I will be back when the time is right! 😉

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 (@msftbot) and give me an instruction to get started! Learn more here.

@DHowett DHowett added the Needs-Second It's a PR that needs another sign-off label Jul 13, 2020
Copy link
Member

@zadjii-msft zadjii-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

@ghost ghost merged commit b2973eb into microsoft:master Jul 14, 2020
@ghost
Copy link

ghost commented Jul 22, 2020

🎉Windows Terminal Preview v1.2.2022.0 has been released which incorporates this pull request.:tada:

Handy links:

@j4james j4james deleted the feature-sgr-conceal branch July 30, 2020 10:06
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-VT Virtual Terminal sequence support AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Second It's a PR that needs another sign-off Priority-2 A description (P2) Product-Conhost For issues in the Console codebase Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for the "concealed" graphic rendition attribute
3 participants