-
Notifications
You must be signed in to change notification settings - Fork 389
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
Issues with PSAvoidUsingWriteHost #267
Comments
I thought we all agreed that Write-Host doesn't kill kittens after all ;-) |
The original purpose of this rule was to enable scripts to be used in an automated environment. Write-Host emits to the display and will interfere when used during a pipeline scenario. Using Write-Host is ok when only UX is involved - in this case suppress the rule application using -ExcludeRule parameter/SuppressRule attribute/Profile Write-Verbose would be a good candidate if Write-Output is confusing users with pipeline data. |
You might also mention that Write-Information is the most direct replacement but that it is only available on PowerShell v5. |
No, @rkeithhill -- Write-Information is not a replacement for Write-Host -- EVER? It doesn't have any output anywhere.
In fact, as far as I have seen, there still isn't a (documented) way for third-party hosts to distinguish Write-Host output from other console text output (Out-Default), unless they completely replace the formatting sub-system starting with Out-Default. |
The bottom line here is that Write-Host is THE ONLY way to provide anything other than a blue-and-white text for user interface. If you want to highlight something, colorize output, etc., it is not only THE ONLY WAY -- it is THE RECOMMENDED WAY!! Warning scripters to stop using it is just ridiculous. TEACHING NEWBIES the difference between Write-Output and Write-Verbose and Write-Host is not the purpose of a linting tool. |
We will discuss further in the meeting today, but I'd still like to capture my baseline thoughts: Write-Host can be okay to use, but only in specific instances where are you very deliberately trying to build a UI of some kind, and can have a reasonable expectation of the host in which PowerShell is executing. For instance, PSReadline employs colored syntax highlighting as a feature of the module, but it can do some wonky stuff in the ISE (which is why we didn't enable it by default there in Windows 10). In these cases, global rule suppression makes the most sense, but it's still an edge case. Generally speaking, scripts and modules should cater to a non-interactive context by default. (Obviously, there are some exceptions to this that use I'm not exactly sure how to tackle that through our existing rules, but we should talk about it today. |
The guidance was updated in the rule output: |
Here is the current wording after installing the module from the gallery:
If we run
The message has a couple of problems. First it states that "Write-Host because it might not work in all hosts" and that one should use instead
Finally, if my script has @rkeithhill @Jaykul Do you think the wording needs to be re-considered again? |
The message guides folks to use Write-Output but Write-Output has no equivalent of -BackgroundColor/-ForegroundColor/-NoNewLine. Perhaps this rule shouldn't fire when any of these parameters are present? However when it does fire, shouldn't it be pointing folks to use Write-Information instead - at least when they are on PowerShell V5?
I have helped quite a few folks with PowerShell functions where they were using Write-Output (implicitly) in their function to notify users of what's going on in the function. Then they were confused by the fact that their function returned a collection instead of the single value they intended. They're usually even more surprised because they used "return $result" and expected that to be the only return value.
Telling a script analyzer user to use Write-Output instead of Write-Host, significantly changes semantics in a way that could break their script. Seems like this tool should obey that tenant of the Hippocratic oath - do no harm. :-)
The text was updated successfully, but these errors were encountered: