-
Notifications
You must be signed in to change notification settings - Fork 8.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Improve conpty rendering of default colors in legacy apps (#6698)
Essentially what this does is map the default legacy foreground and background attributes (typically white on black) to the `IsDefault` color type in the `TextColor` class. As a result, we can now initialize the buffer for "legacy" shells (like PowerShell and cmd.exe) with default colors, instead of white on black. This fixes the startup rendering in conpty clients, which expect an initial default background color. It also makes these colors update appropriately when the default palette values change. One complication in getting this to work, is that the console permits users to change which color indices are designated as defaults, so we can't assume they'll always be white on black. This means that the legacy-to-`TextAttribute` conversion will need access to those default values. Unfortunately the defaults are stored in the conhost `Settings` class (the `_wFillAttribute` field), which isn't easily accessible to all the code that needs to construct a `TextAttribute` from a legacy value. The `OutputCellIterator` is particularly problematic, because some iterator types need to generate a new `TextAttribute` on every iteration. So after trying a couple of different approaches, I decided that the least worst option would be to add a pair of static properties for the legacy defaults in the `TextAttribute` class itself, then refresh those values from the `Settings` class whenever the defaults changed (this only happens on startup, or when the conhost _Properties_ dialog is edited). And once the `TextAttribute` class had access to those defaults, it was fairly easy to adapt the constructor to handle the conversion of default values to the `IsDefault` color type. I could also then simplify the `TextAttribute::GetLegacyAttributes` method which does the reverse mapping, and which previously required the default values to be passed in as a parameter VALIDATION I had to make one small change to the `TestRoundtripExhaustive` unit test which assumed that all legacy attributes would convert to legacy color types, which is no longer the case, but otherwise all the existing tests passed as is. I added a new unit test verifying that the default legacy attributes correctly mapped to default color types, and the default color types were mapped back to the correct legacy attributes. I've manually confirmed that this fixed the issue raised in #5952, namely that the conhost screen is cleared with the correct default colors, and also that it is correctly refreshed when changing the palette from the properties dialog. And I've combined this PR with #6506, and confirmed that the PowerShell and the cmd shell renderings in Windows Terminal are at least improved, if not always perfect. This is a prerequisite for PR #6506 Closes #5952
- Loading branch information
Showing
12 changed files
with
102 additions
and
68 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters