Skip to content

RecommendedAction: Explicitly start and stop ANSI Error Color #24065

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.

Sign up for GitHub

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

Merged
andyleejordan merged 2 commits into from
Jul 29, 2024
Merged

RecommendedAction: Explicitly start and stop ANSI Error Color #24065

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f1670a2
RecommendedAction: Explicitly start and stop ANSI Error Color
JustinGrote Jul 19, 2024
ed4ca90
Merge remote-tracking branch 'upstream/master' into justingrote/fix/r…
JustinGrote Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ...m.Management.Automation/FormatAndOutput/DefaultFormatters/PowerShellCore_format_ps1xml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1351,7 +1351,11 @@ function Get-ConciseViewPositionMessage {
if ($ErrorView -eq 'ConciseView') {
$recommendedAction = $_.ErrorDetails.RecommendedAction
if (-not [String]::IsNullOrWhiteSpace($recommendedAction)) {
$recommendedAction = $newline + ' Recommendation: ' + $recommendedAction
$recommendedAction = $newline +
${errorColor} +
Copy link
Collaborator

@kilasuit kilasuit Jul 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion in line with my other comment is that instead of ${errorColor} this could be ${recommendationColor} to allow for differing colours to be used instead.

Copy link
Contributor Author

@JustinGrote JustinGrote Jul 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kilasuit adding a PSStyle option was discussed in the original PR, there's actually a lot that goes into adding that vs. keeping this a small simple PR, I can't commit I could get all of that done by 7.5 but I structured it such that it could be added in the future.

For now I still think it should be formatted with ErrorColor because it is a related property, and gives the user the ability to still customize with PSStyle (as per the original PR implementation and justification), the current behavior is a bug.

Copy link
Collaborator

@theJasonHelmick theJasonHelmick Jul 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WG Interactive group has reviewed this. We agree to implement as this pr describes using ErrorColor. We would recommend making this customizable in the future based on Recommended Action.

Copy link
Member

@andyleejordan andyleejordan Jul 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, all makes sense to me!

' Recommendation: ' +
$recommendedAction +
${resetcolor}
}

if ($err.PSMessageDetails) {
Expand Down