Skip to content

Warning stream color #2317

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

Closed
DarkLite1 opened this issue Nov 19, 2019 · 8 comments
Closed

Warning stream color #2317

DarkLite1 opened this issue Nov 19, 2019 · 8 comments

Comments

@DarkLite1
Copy link

DarkLite1 commented Nov 19, 2019

Currently when running the code below the colors for the Verbose and Warning stream are exactly the same. Would it be possible to have a different color for Warning, just like in the PowerShell ISE?

$VerbosePreference = 'Continue'
Write-Verbose 'this is verbose text'
Write-Warning 'this is warning text'
Write-Verbose 'this is verbose text'

Results in:
image

Probably #594 is related.

@ghost ghost added the Needs: Triage Maintainer attention needed! label Nov 19, 2019
@corbob
Copy link
Contributor

corbob commented Nov 19, 2019

I'm not going to reinvent the wheel by reposting a bunch of stuff, instead I'll just link you here: https://learn-powershell.net/2012/08/07/make-your-powershell-errors-less-harsh-by-changing-their-color/

And thank you heavily for this as I'm adding to my AllHosts profile to set the color different because IMHO it was a mistake to make them the same color.

image

Edit because I accidentally posted too early: effectively you can use $host.PrivateData to change the colors. I'm intrigued by this because I recall a talk by @KirkMunro whereby this method didn't work in vscode... Or perhaps that's only for the error stream?

@DarkLite1
Copy link
Author

DarkLite1 commented Nov 19, 2019

Thank you @corbob . I tend to agree with you that not setting the colors for Verbose and Warning different is a design mistake. On top of this I would like to suggest the following as a standard for a new release of the extension:

$VerbosePreference = 'Continue'
$WarningPreference = 'Continue'

$Host.PrivateData.WarningForegroundColor = [System.ConsoleColor]::Yellow
$Host.PrivateData.VerboseForegroundColor = [System.ConsoleColor]::Cyan

Write-Verbose 'Verbose text'
Write-Warning 'Warning tesx'
Write-Verbose 'Verbose text'
Write-Verbose 'Verbose text'

The Cyan is more neutral and the Yellow is better suited for a warning. The same way as it is in the PowerShell ISE. At the devs, would this be a valid option for a future release? It avoids people having to edit files

image

@corbob
Copy link
Contributor

corbob commented Nov 19, 2019

I don't think it would be unreasonable to add that as a default, but that might be a PSES thing. Alternatively, perhaps it's something to add to the ISE Compatibility documents. @TylerLeonhardt was asking about things that could be added on Twitter the other day: https://twitter.com/TylerLeonhardt/status/1195750313203945473

On a related note: I found the video I was talking about, and it was from PSHSummit 2018: https://www.youtube.com/watch?v=zhjU24hbYuI#t=4m30s It looks like ISE has a $psISE variable that has an Options property that is basically just $host.PrivateData.

@SeeminglyScience
Copy link
Collaborator

@corbob

I'm intrigued by this because I recall a talk by @KirkMunro whereby this method didn't work in vscode... Or perhaps that's only for the error stream?

It didn't for awhile, support was added in PowerShell/PowerShellEditorServices#654.

@DarkLite1

On top of this I would like to suggest the following as a standard for a new release of the extension:

Personally I'd prefer we stick to the same thing a standard console does as a default. It's easy enough to customize with a profile imo.

@DarkLite1
Copy link
Author

@SeeminglyScience it depends on what we try to achieve. If all we want is a standard console with default functionality then one could argue that even PSReadLine might not be required. But if we try to get on par with the PowerShell ISE, and we're only talking a color here, then it would be a desired feature/change.

I'm not a developer but if two people already are adjusting their profiles, then it might be a good suggestion and it might help others who are less experienced and don't want to change their configs.

Normally when writing PowerShell code and doing other IT related stuff, I'm always in favor of staying as close to the standards as possible. But in this case I don't think we would break someone's workflow or have a burden of maintenance coming afterwards. So I vote yes :)

@SeeminglyScience
Copy link
Collaborator

@SeeminglyScience it depends on what we try to achieve. If all we want is a standard console with default functionality then one could argue that even PSReadLine might not be required.

When I say "standard console" I mean literally the experience that ConsoleHost provides by default in pwsh/powershell.exe. PSRL is included by default by ConsoleHost, so it fits. Even if it wasn't, I'd still argue for it's inclusion. I'm not saying I would be opposed to any changes in behavior, but my opinion is that any intentional differences should have significant demonstrated value. The benefits of changing the default color of a message stream is mostly subjective and personal preference.

I'm not a developer but if two people already are adjusting their profiles, then it might be a good suggestion and it might help others who are less experienced and don't want to change their configs.

Plenty of folks are customizing the colors, that's definitely an important feature. I was really happy to see the PR that added support for those settings. That said, I don't think the fact that folks are interacting with that API to be a compelling reason to change the defaults. Color choices are personal, what works for me may infuriate you (my error stream is Cyan for instance 🙂).

If you want the defaults changed, I'd suggest starting with PowerShell itself. If they change it up, we'll probably follow suit.

@TylerLeonhardt
Copy link
Member

Yeah I think we have a unique opportunity here where we can finally have the editor and the console have the same behavior. Consistency.

The UI API's are there to give folks the ability to change that consistency both for just PowerShell Integrated Console (via the CurrentUserCurrentHost profile) or for all PowerShell hosts (via CurrentUserAllHosts profile)

@SydneyhSmith SydneyhSmith self-assigned this Nov 19, 2019
@DarkLite1
Copy link
Author

Ok guys, thanks for the feedback. I'm also in favor of consistency so I'll close this one.

@ghost ghost added the Needs: Maintainer Attention Maintainer attention needed! label Nov 20, 2019
@TylerLeonhardt TylerLeonhardt removed the Needs: Maintainer Attention Maintainer attention needed! label Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants