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

$Host.PrivateData colors are completely broken in PowerShell 7.2 #16441

Closed
5 tasks done
Jaykul opened this issue Nov 11, 2021 · 19 comments
Closed
5 tasks done

$Host.PrivateData colors are completely broken in PowerShell 7.2 #16441

Jaykul opened this issue Nov 11, 2021 · 19 comments
Labels
Committee-Reviewed PS-Committee has reviewed this and made a decision Resolution-No Activity Issue has had no activity for 6 months or more WG-Interactive-Console the console experience

Comments

@Jaykul
Copy link
Contributor

Jaykul commented Nov 11, 2021

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 Jaykul added the Needs-Triage The issue is new and needs to be triaged by a work group. label Nov 11, 2021
@iSazonov
Copy link
Collaborator

/cc @SteveL-MSFT

@iSazonov
Copy link
Collaborator

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

@237dmitry
Copy link

Setting them no longer changes anything.

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

@dwtaber
Copy link
Contributor

dwtaber commented Nov 11, 2021

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

@daxian-dbw daxian-dbw added WG-Interactive-Console the console experience and removed WG-Engine-Format labels Nov 12, 2021
@Jaykul
Copy link
Contributor Author

Jaykul commented Jul 30, 2022

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?!?

@SteveL-MSFT
Copy link
Member

@dwtaber Error for classic view was a bug that was fixed via #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.

@Jaykul
Copy link
Contributor Author

Jaykul commented Jul 30, 2022

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.

@SteveL-MSFT
Copy link
Member

SteveL-MSFT commented Jul 30, 2022

@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
Copy link
Member

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.

@Jaykul
Copy link
Contributor Author

Jaykul commented Jul 30, 2022

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

@Jaykul
Copy link
Contributor Author

Jaykul commented Jul 30, 2022

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?

@SteveL-MSFT
Copy link
Member

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:

if (SupportsVirtualTerminal)
{
WriteLine(value);
}
else
{
WriteLine(ErrorForegroundColor, ErrorBackgroundColor, value);
}
. 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.

@Jaykul
Copy link
Contributor Author

Jaykul commented Jul 31, 2022

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?

@SteveL-MSFT
Copy link
Member

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.

@Jaykul
Copy link
Contributor Author

Jaykul commented Aug 1, 2022

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". 😣

@poidah

This comment was marked as off-topic.

@theJasonHelmick
Copy link
Collaborator

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 theJasonHelmick added Review - Committee The PR/Issue needs a review from the PowerShell Committee and removed Needs-Triage The issue is new and needs to be triaged by a work group. labels Sep 27, 2023
@theJasonHelmick
Copy link
Collaborator

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 theJasonHelmick added Committee-Reviewed PS-Committee has reviewed this and made a decision and removed Review - Committee The PR/Issue needs a review from the PowerShell Committee labels Oct 18, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Resolution-No Activity Issue has had no activity for 6 months or more label Apr 16, 2024
Copy link
Contributor

This issue has not had any activity in 6 months, if there is no further activity in 7 days, the issue will be closed automatically.

Activity in this case refers only to comments on the issue. If the issue is closed and you are the author, you can re-open the issue using the button below. Please add more information to be considered during retriage. If you are not the author but the issue is impacting you after it has been closed, please submit a new issue with updated details and a link to this issue and the original.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committee-Reviewed PS-Committee has reviewed this and made a decision Resolution-No Activity Issue has had no activity for 6 months or more WG-Interactive-Console the console experience
Projects
None yet
Development

No branches or pull requests

8 participants