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

Stop setting the background color when erasing #830

Closed
Jaykul opened this issue Dec 11, 2018 · 4 comments · Fixed by #1626
Closed

Stop setting the background color when erasing #830

Jaykul opened this issue Dec 11, 2018 · 4 comments · Fixed by #1626

Comments

@Jaykul
Copy link

Jaykul commented Dec 11, 2018

As of Windows 10 build 18298, the new “Terminal” settings allow setting default ForegroundColor and BackgroundColor values which are separate from the 16 color "ConsoleColor" palette.

PSReadLine doesn't use the ANSI/VT escape sequence for default when erasing text, but instead overwrites the line with $Host.UI.RawUI.BackgroundColor (e.g. when pressing backspace, or when using the arrow keys to navigate through the history buffer with commands of different lengths -- worse when some have multiple lines).

PSReadLine needs to start using the "default" color, instead of specifying a color.

@lzybkr
Copy link
Member

lzybkr commented Dec 11, 2018

Interesting, nobody has complained about this on Linux or Mac where this should also be a problem.

Have you tried removing this line? That might be all it takes.

@Jaykul
Copy link
Author

Jaykul commented Dec 11, 2018

Yeah, that's weird. I'm tempted to suggest that most people probably set their background = black(ish) and foreground = white(ish) (or vice-versa) and don't use an extra color ...

However, there are certainly a lot of, say iTerm color schemes, where the default background is not one of the base16 colors. E.g. BirdsOfParadise, Borland, Chalk, Desert ...

I tried a couple of those in the new build last night but (as we've discussed elsewhere) I can't find a way to programmatically set the new color values yet.

@SteveL-MSFT SteveL-MSFT added this to the Future milestone Jun 18, 2019
@DHowett
Copy link
Contributor

DHowett commented Jun 27, 2020

This is going to become critically important for a future Terminal update (1.2, 1.3).

Since it launched, Terminal has been operating in a compatibility mode where the backing console acts as though the terminal setting @Jaykul mentioned is turned off.

This has been the cause of a great number of issues, chief among them our inability to distinguish \e[40m from \e[49m and \e[37m from \e[39m. For a representative sample, look at duplicates of microsoft/terminal#293 and microsoft/terminal#2661.

We're finally getting that fixed.

We can differentiate colors set through the Win32 console API, by VT with the 0-8 color set (±brightness), and by VT with the 0-256 color set.

old behavior

color type stored as translated as
vt 0-8 (±brightness) indexed 0-16 mangled
vt 0-256 rgb mangled
vt rgb rgb mangled
vt defaults (39, 49) "default" flag mangled
win32 1-6, 8-16 indexed 0-16 mangled
win32 0, 7 indexed 0-16 mangled, but into vt 39, 49

(I marked every translation as "mangled" here because we would first resolve the index to an rgb color known to the conhost in the middle of the transaction, then resolve the rgb color back to an index, and if that RGB color matched the defaults turn it into 39/49, and if it didn't match an index turn it into a \e[38;2;x;y;zm RGB sequence)

new behavior

color type stored as translated as notes
vt 0-8 (±brightness) indexed 0-16 passed through unchanged totally normal VT color
vt 0-256 indexed 0-256 passed through unchanged cannot be brightened with SGR 1 (!)
vt defaults (39, 49) "default" flag passed through unchanged
win32 0-16 indexed 0-256 vt 0-256 cannot be brightened with SGR 1 (!)
win32 index matching console properties sheet default for bg/fg "default" flag vt defaults (39, 49) this to stop all legacy console applications from forcing a black background

Problem is, though, that due to the line @lzybkr pointed out PSReadline takes a ConsoleColor and maps it to a VT color. This means that we're reading out the foreground and background from win32 format (bottom of the table) and transforming it to vt (top of the table).

Joel's assertion about black backgrounds unfortunately doesn't hold for our userbase. People love background images and acrylic, though I cannot quite determine why.

The result looks like this (with my transparency exaggerated for effect):

image

@DHowett
Copy link
Contributor

DHowett commented Jun 27, 2020

And just to confirm, removing that line works great:

image

😄

ghost pushed a commit to microsoft/terminal that referenced this issue Jun 23, 2022
In #6810, we introduced a "quirk" for all known versions of PowerShell
that suppressed their requests for black background/gray foreground.
This was done to avoid an [issue in PSReadline] where it would paint
black bars all over the screen if the default background color wasn't
the same as the ANSI black color.

Years have passed since that quirk was introduced. The underlying bug
was fixed, and the fix was released broadly long ago. It's time for us
to remove the quirk... almost.

Terminal still runs on versions of Windows that ship a broken version of
PSReadline. We must maintain the quirk there -- the user can't do
anything about it, and we would make their experience worse if we
removed the quirk entirely.

PowerShell 7.0 also ships a broken version of PSReadline. It is still in
support for another 6 months, but updates have been available for some
time. We can encourage users to update.

Therefore, we only need the quirk for Windows PowerShell, and then only
for specific versions of Windows.

_Inside Windows_, we don't even need that: we're guaranteed to be built
alongside a fixed version of PowerShell!

Closes #6807

[issue in PSReadline]: PowerShell/PSReadLine#830 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants