-
Notifications
You must be signed in to change notification settings - Fork 298
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
Incorrect color handling / color handling improvements #1110
Comments
I believe that Actually that's not entirely true, XTPUSHSGR/XTPOPSGR is precisely what should be used here, but I don't think there is critical mass in support yet that we can use those. But I have to ask - is it really a good idea to have PowerShell specific terminal settings? |
Ah ... if I set the background RGB of 64,64,0 - a sort of muddy brown Well ... that's a real pain. Because you have to choose between either honouring a temporary change made by setting
I see Did you have any thoughts on the enhancements ? |
It seems Windows Terminal has chosen a background color for PowerShell that is not used in normal color table, see here I also recall seeing the same in a default Ubuntu desktop - but it's been awhile so I might be mistaken. As the use of As for themes - I think that maybe belongs in PowerShell rather than PSReadLine. There may be some interest in making PSReadLine agnostic to PowerShell so that it can be used in other tools. |
This is both a bug and a feature request, which could be better as single issues.
It is connected to PowerShell/PowerShell#10811
First the bug.
Currently the code assumes default colors are in force in the console. A simple example is
$host.ui.RawUI.foregroundColor = "cyan"
changes the color of the output text but
get-psreadlineOption
changes it back to the terminal default. This is because after setting the color with an escape sequence there is reset to console defaults
\x1b[0m
. To fix this in cmdlets.cs, inside formatcolor() line 1059"\x1b[0m";
should be the escape sequence for host.ui.rawui.foregroundcolorAdditionally the error color should be the one set in host.private data, In the same file in PSConsoleReadLineOptions line 79
DefaultErrorColor = ConsoleColor.Red;
should set the value to host.privateData.errorforegroundColor if it is a console color, and only if it is not set ConsoleColor.Red.and in resetcolors () line 427
fg = console.ForegroundColor
-- should set the value to host.ui.rawui.foregroundcolor if it is a console color otherwise -1and in resetcolors () line 442
bg = console.backgroundColor
-- should set the value to host.ui.rawui.backgroundcolor if it is a console color otherwise -1Now the improvements.
The thinking
Much of the work for this has already been done in psreadline
Two enhancements are needed.
first in cmdlets.cs after the public read/write properties for the psreadline colors. (after about line 404)
Insert read only properties for formatAccentColor, verboseColor, warningColor, debugcolor, to return values from Host.PrivateData and for foregroundColor & backgroundColor to return from host.ui.rawui.
Rationale
PSConsoleReadLine.GetOptions()
then returns all theme colors in one place.Second
VTColorUtils
should have an addition static method which takes a string to apply formatting to and an enum whose members are the theme colors, reverse, and underline, and applies that formatting to the string and ends with set-to-foreground-color / underline-off / reverse-off. Ideally this would check a preference variable and host.ui.SupportsVirtualTerminal and return the text unformatted where appropriate.Rationale instead of encoding
"\x1b[32m"+<some value>+" \x1b[0m"
either in a format.ps1xml file or in compiled code the formatting is[vtColourUtils]::ApplyFormat(<someValue>,formatAccentColor)
Which can be promoted as good practice. I understand these are not "read line" things but as the owner of VT Color utils, in my view it would be silly to do them anywhere else.
The text was updated successfully, but these errors were encountered: