Open PRs should not be assigned to milestone, so they are not assigned to the wrong milestone after they are merged. For backport consideration, use a backport label.

This PR has 6 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Label      : Extra Small
Size       : +4 -2
Percentile : 2.4%

Total files changed: 1

Change summary by file extension:
.cs : +4 -2

Was this comment helpful? 👍  :ok_hand:  :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

I cherry-picked the changes to 7.2.6 branch trying to make sure the changes work on 7.2.next. It turns out this works fine on Windows but not so well on Linux:

As shown in the GIF, the first prompt after write-error is in default color, but the following ones are in red color. So, it looks like writing out Reset VT sequence doesn't correctly reset the color change done by Console.ForegroundColor = xxx.

I also tried the 7.3 private build, and it works fine on both Windows and Linux. There are big changes in .NET 7 in the Console area, so I guess that was fixed in .NET 7. However, we need it to work well on 7.2.6 (.NET 6) if we want to backport this change.

Given what I found above, I think we should not rely on setting Console.ForegroundColor/BackgroundColor for NormalView/CategoryView. Instead, as long as VT is supported, we can translate ConsoleColor to VT sequences using [System.Management.Automation.VTUtility]::GetEscapeSequence and explicitly use VT sequences to set the color attribute, so that Reset at the end will reliably work on 7.2.6.

One caveat is that [VTUtility]::GetEscapeSequence seems to only convert foreground color, not background color, but that can be added pretty easily, I think.

Actually, I have question: why do we have to use colors defined in $Host.PrivateData for the NormalView error rendering?
In my opinion, $ErrorView just defines how to render ErrorRecord, but not necessarily coupled with where the color should come from.

After introducing $PSStyle, we say this in the doc:

It replaces $Host.PrivateData as the way to manage colors for formatting rendering.

So, it feels to me we should just use $PSStyle.Formatting.Error regardless of $ErrorView as long as VT is supported.

@daxian-dbw the feedback was that it broke existing users of NormalView who set the color in their $profile. Perhaps we should have a WG review.

There may have been some other changes that would need to be cherry-picked to 7.2 to make it work completely.

the feedback was that it broke existing users of NormalView who set the color in their $profile. Perhaps we should have a WG review.

It's already broken for other formatting color settings defined in $host.PrivateData, unless we want to make them all working.

Waiting on the WG-Interactive-Console group to decide what to do with PrivateData.

The Interactive WG discussed $Host.PrivateData vs. $PSStyle.Formatting, and agreed that all formatting color settings should be in the same location, and ErrorRecord formatting should use $PSStyle instead of $host. See #17886 (comment) for details.

This PR is superseded by #17987