PowerShell icon indicating copy to clipboard operation
PowerShell copied to clipboard

$Host.PrivateData colors are completely broken in PowerShell 7.2

Open Jaykul opened this issue 4 years ago • 15 comments

Prerequisites

Steps to reproduce

$PSStyle has completely broken the PrivateData colors. Setting them no longer changes anything.

image

Expected behavior

The colors on `$Host.PrivateData` should affect the output

Actual behavior

Changes to `$Host.PrivateData` don't do anything anymore

Error details

No response

Environment data

Name                           Value
----                           -----
PSVersion                      7.2.0
PSEdition                      Core
GitCommitId                    7.2.0
OS                             Microsoft Windows 10.0.22000
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Visuals

This is what I expected: image

Jaykul avatar Nov 11 '21 04:11 Jaykul

/cc @SteveL-MSFT

iSazonov avatar Nov 11 '21 04:11 iSazonov

$Host.PrivateData works if there is no virtual terminal or it is disabled.

iSazonov avatar Nov 11 '21 05:11 iSazonov

Setting them no longer changes anything.

If I change $PSStyle.Progress.View to Classic then the progress colors are from $Host.PrivateData.Progress*

237dmitry avatar Nov 11 '21 08:11 237dmitry

Hm. Error output obeys $Host.PrivateData.ErrorForegroundColor but not $Host.PrivateData.ErrorBackgroundColor for me, and only when $ErrorView = ConciseView. For any other value of $ErrorView, no coloration is applied.

Meanwhile, the Get-Error cmdlet applies $PSStyle.Formatting.Error coloration to the value of the returned object's Exception.Message property, as well as to the position-marking tildes of InvocationInfo.PositionMessage

dwtaber avatar Nov 11 '21 22:11 dwtaber

So WITH NO NOTICE a bunch of code was broken that customized error and verbose output colors, and there is now no way to customize them in a compatible fashion, except to feature-sniff for types/versions and do it both ways.

I raised this MONTHS AGO as a REGRESSION BUG and I feel like it was completely brushed off and basically ignored.

What is going on with prioritization of development over there?!?

Jaykul avatar Jul 30 '22 20:07 Jaykul

@dwtaber Error for classic view was a bug that was fixed via https://github.com/PowerShell/PowerShell/pull/17705

@Jaykul the WGs are working through issues, but haven't gotten to this one. I would propose to have a new $PSStyle.Formatting.UsePrivateDataColors set to $true by default which would have the old stream output use the old settings. We could automatically flip this to $false if any of the new formatting colors are set.

SteveL-MSFT avatar Jul 30 '22 21:07 SteveL-MSFT

This did not (and does not) need to be one or the other. The new thing could have been implemented on top of the old. If you implement it as automatic switching, you risk total confusion, when all the colors change when someone meant to change only one...

Perhaps the more helpful thing would be to use the new $PSStyle but update it automatically when the PrivateData values are set. That is, treat PrivateData as a [ConsoleColor] limited setter for $PSStyle.Formatting.

So if something runs: $Host.PrivateData.ErrorForeground = "Red" then in addition to updating PrivateData, you could also set $PSStyle.Formatting.Error += $PSStyle.Foreground.Red

Or if they run $Host.PrivateData.ErrorForeground = "Black"; $Host.PrivateData.ErrorBackground = "Red" you could also set $PSStyle.Formatting.Error = $PSStyle.Foreground.Black + $PSStyle.Background.Red

Although right now, I'm not sure if the current formatting supports setting background colors. It seems broken to me.

Jaykul avatar Jul 30 '22 21:07 Jaykul

@Jaykul that is also an option. The only problem is if then the user updates $PSStyle, it would not be possible to reflect that back into PrivateData unless they happen to use one of the 8 colors only. It might just be something that gets documented and could be ok.

SteveL-MSFT avatar Jul 30 '22 21:07 SteveL-MSFT

Actually, there's a problem with your proposal. $Host.PrivateData is managed by the host (ConsoleHost, EditorServices, etc...), so every host would have to adopt that pattern. PrivateData is just a PSObject so a host can have whatever they want in there. The colors are just a de facto convention.

SteveL-MSFT avatar Jul 30 '22 22:07 SteveL-MSFT

I think going one way would probably be good enough. I mean, I have code for converting from 16bit colors to 16 colors.

Jaykul avatar Jul 30 '22 22:07 Jaykul

Actually, there's a problem with your proposal. $Host.PrivateData is managed by the host (ConsoleHost, EditorServices, etc...), so every host would have to adopt that pattern. PrivateData is just a PSObject so a host can have whatever they want in there. The colors are just a de facto convention.

So what you're saying is that the HOST is still trying to set the colors, but the engine is injecting escape sequences into the text that override the host's settings?

Jaykul avatar Jul 30 '22 22:07 Jaykul

Write-Error, Write-Verbose, etc... are host callbacks. It's up to the host to decide how to display them. Here's the relevant section for ConsoleHost: https://github.com/PowerShell/PowerShell/blob/edac193cf05a540b1f5f59f23fd1eb29c2776ce1/src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHostUserInterface.cs#L1395-L1402. Currently, if VT is supported, it goes the VT route and by-passes the older code path that relies on [Console] APIs that accept [ConsoleColor] and overrides the color codes in the string (the string value still contains them).

So in the end, it's up to the host to decide what to do. This was an unfortunate design choice early on.

SteveL-MSFT avatar Jul 30 '22 22:07 SteveL-MSFT

So, bottom line: you added a new, non-host way of injecting virtual terminal escape sequences for the same purposes that already existed in the host, and then changed the default host so that if it thinks it's running in a terminal that supports escape sequences ... it just ignores it's own settings.

Is that right? Is backward compatibility just out the window forever?

Jaykul avatar Jul 31 '22 02:07 Jaykul

I've proposed a solution due to the limitations of the existing design. You had a different proposal which seemed good at first until I looked into it and it's not feasible. If you have an alternate proposal, let's discuss it.

SteveL-MSFT avatar Jul 31 '22 03:07 SteveL-MSFT

I'm frustrated, but I don't know how I didn't understand that you'd deliberately deprecated PrivateData. Honestly, I don't think there's any point in putting a switch in. It would basically be the same as setting $ENV:TERM = "dumb" right?

Writing a warning that they're deprecated when someone sets those properties (in a host that SupportsVirtualTerminal?) would be nice, so people can be reminded those things are now useless ... but unless we can do that, I think we might as well just close this "as designed". 😣

Jaykul avatar Aug 01 '22 03:08 Jaykul

I've just started reading about this and I can't believe in 7 years nothing has been done.

The time spent arguing about this since @Jaykul first asked about it on Sep 29, 2016, could have been used to implement his rather straight forward request.

There does need to be any breaking changes, just override the Write-Host to allow for HEX values for ForegroundColor.

The terminal window is not a graphic design tool, the RGB space is perfectly sufficient and slight display variations are the least of a coder's problems.

We just want a way of distinguishing one thing from another in the terminal - this doesn't have to made into rocket science.

Interlectual wars aside, this is would benefit everyone - Support for RGB console colors

poidah avatar Feb 06 '23 22:02 poidah

The Interactive WG has reviewed this long standing issue. We don't think that PrivateData can be linked well to PSStyle to remove a poor affect from PrivateData. We think the current behavior of PSStyle was the intended design change, however we want to review this committee before closing this issue.

theJasonHelmick avatar Sep 27 '23 20:09 theJasonHelmick

Thank you to all that commented on this issue.  The PowerShell committee has reviewed and concurs with the Interactive WG that this feature is by designed.  We will investigate in the future to emit a depreciation warning if any private data is changed.

theJasonHelmick avatar Oct 18 '23 21:10 theJasonHelmick