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

Can't change background color in Powershell after updating to 1809 #276

Closed
johanlindfors opened this issue Oct 8, 2018 · 8 comments
Closed
Assignees
Labels
Product-Powershell Issues in the modern command interpreter, Powershell.exe. Work-Item It's being tracked by an actual work item internally. (to be removed soon)

Comments

@johanlindfors
Copy link

After the latest update to Windows 10 1809 I can't change the background color of the PowerShell console from within the actual console.

This used to work:

[console]::BackgroundColor = ”black”
Clear-Host
(the window was now black)

But it doesn't anymore.

@zadjii-msft zadjii-msft added Work-Item It's being tracked by an actual work item internally. (to be removed soon) Product-Conhost For issues in the Console codebase labels Oct 8, 2018
@zadjii-msft zadjii-msft self-assigned this Oct 8, 2018
@zadjii-msft zadjii-msft added this to the 19H1 milestone Oct 8, 2018
@zadjii-msft
Copy link
Member

I'll take a look at it. There's a good chance I regressed this. Internally filed as MSFT:19234722

@bitcrazed
Copy link
Contributor

Thanks for filing @johanlindfors.

@zadjii-msft - I created bug 19221878 to track this.

@DHowett-MSFT
Copy link
Contributor

[Linking] This was also reported at PowerShell/PSReadLine#774.

@lzybkr
Copy link
Member

lzybkr commented Oct 23, 2018

This happens because PSReadLine is writing CSI 0 m, you can reproduce w/o PSReadLine like this:

function prompt { "PS> " } # Ensure we aren't using posh-git
[Console]::BackgroundColor = 'Yellow'
cls
"$([char]0x1b)[34mHello$([char]0x1b)[0m"
"$([char]0x1b)[34mHello$([char]0x1b)[0m"

After the cls, I have the follow

powershellcolors

So it looks like writing CSI 0 m reverts the background color to the initial background color, not the current background color (which may have been set via an escape sequence or via a Win32 Console api).

It also looks like this behavior is not specific to conhost, I see the same thing in the CentOS Terminal.

So is it safe to say that any tool that writes CSI 0 m should save the current foreground/background colors before writing anything and restore them afterwards? Assuming this is correct, Posh-Git and maybe Powerline also need to be fixed.

@mjoyce6500
Copy link

Experiencing same issue after OS update. Custom $PROFILE which was working now reverts back to system default background color at the prompt as you type ….

@miniksa miniksa added Product-Powershell Issues in the modern command interpreter, Powershell.exe. and removed Product-Conhost For issues in the Console codebase labels Jan 18, 2019
@miniksa miniksa removed this from the 19H1 milestone Jan 18, 2019
@miniksa
Copy link
Member

miniksa commented Jan 18, 2019

This was moved to Powershell as MSFT: 19221878 to track an inbox release. It has been fixed in PSReadLine on GitHub according to @SteveL-MSFT.

@DHowett-MSFT
Copy link
Contributor

I never really weighed in on this, but I'd like to double-click on this:

So is it safe to say that any tool that writes CSI 0 m should save the current foreground/background colors before writing anything and restore them afterwards?

I think it's unsafe to say that. We're moving to a world where the user's current foreground/background color is a non-machine-manipulable user preference, and one that cannot be represented by an index into the standard color table.

If the user's "default" colors aren't the same as their current runtime colors, that is a user configuration issue. No tool should try to divine what the background and foreground are, because those colors may not be representable in any known color space. Attempts to do so (like PSReadline here and MSYS/MinGW/Cygwin for git) result in a poor user experience.

The correct solution is that any tool that would like to play nicely with other tools and terminals should use CSI 0 m to mean "restore your default colors." It was a mistake to allow commandline applications to know the "active" and "default" colors.

@lzybkr
Copy link
Member

lzybkr commented Apr 1, 2019

@DHowett-MSFT - I like the idea that user color preferences are more static. I'm sure you're well aware that configuration of said preferences needs to be easy though, as easy as what folks do today in their PowerShell profile.

I do think that we need to make it easier for colorful command line applications to choose appropriate defaults though. Today if you use a white/light background, PSReadLine is essentially unusable. This is only fixable if you can query the RGB value of the background - which you can do with a Win32 api or some terminals with the appropriate VT sequence.

DHowett pushed a commit that referenced this issue Nov 17, 2020
This fixes a number of exceptions that can cause conhost to crash when
the buffer is resized in such a way that the viewport or cursor position
end up out of bounds.

Technically this is a fix for issue #256, although that has been closed
as "needs-repro".

The main fix was to add checks in the `SetConsoleScreenBufferSizeImpl`
and `SetConsoleScreenBufferInfoExImpl` methods, to make sure the
viewport doesn't extend past the bottom or right of the buffer after a
resize. If it has overflowed, we move the viewport back up or left until
it's back within the buffer boundaries. We also check if the cursor
position has ended up out of bounds, and if so, clamp it back inside the
buffer.

The `SCREEN_INFORMATION::SetViewport` was also a source of viewport
overflow problems, because it was mistakenly using inclusive coordinates
in its range checks, which resulted in them being off by one. That has
now been corrected to use exclusive coordinates.

Finally, the `IsCursorDoubleWidth` method was incorrectly marked as
`noexcept`, which was preventing its exceptions from being caught.
Ideally it shouldn't be throwing exceptions at all any more, but I've
removed the `noexcept` specifier, so if it does throw an exception,
it'll at least have more chance of recovering without a crash.

## Validation Steps Performed

I put together a few test cases (based on the reports in issues #276 and
#1976) which consistently caused conhost to crash, or to generate an
exception visible in the debug output. With this PR applied, those test
cases are no longer crashing or triggering exceptions.

Closes #1976
DHowett pushed a commit that referenced this issue Jan 25, 2021
This fixes a number of exceptions that can cause conhost to crash when
the buffer is resized in such a way that the viewport or cursor position
end up out of bounds.

Technically this is a fix for issue #256, although that has been closed
as "needs-repro".

The main fix was to add checks in the `SetConsoleScreenBufferSizeImpl`
and `SetConsoleScreenBufferInfoExImpl` methods, to make sure the
viewport doesn't extend past the bottom or right of the buffer after a
resize. If it has overflowed, we move the viewport back up or left until
it's back within the buffer boundaries. We also check if the cursor
position has ended up out of bounds, and if so, clamp it back inside the
buffer.

The `SCREEN_INFORMATION::SetViewport` was also a source of viewport
overflow problems, because it was mistakenly using inclusive coordinates
in its range checks, which resulted in them being off by one. That has
now been corrected to use exclusive coordinates.

Finally, the `IsCursorDoubleWidth` method was incorrectly marked as
`noexcept`, which was preventing its exceptions from being caught.
Ideally it shouldn't be throwing exceptions at all any more, but I've
removed the `noexcept` specifier, so if it does throw an exception,
it'll at least have more chance of recovering without a crash.

## Validation Steps Performed

I put together a few test cases (based on the reports in issues #276 and
#1976) which consistently caused conhost to crash, or to generate an
exception visible in the debug output. With this PR applied, those test
cases are no longer crashing or triggering exceptions.

Closes #1976

(cherry picked from commit 9a07049)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product-Powershell Issues in the modern command interpreter, Powershell.exe. Work-Item It's being tracked by an actual work item internally. (to be removed soon)
Projects
None yet
Development

No branches or pull requests

7 participants