Change default to OutputRendering.Host and remove OutputRendering.Automatic#15882
Change default to OutputRendering.Host and remove OutputRendering.Automatic#15882daxian-dbw merged 9 commits intoPowerShell:masterfrom
OutputRendering.Host and remove OutputRendering.Automatic#15882Conversation
iSazonov
left a comment
There was a problem hiding this comment.
I found follow pattern twice in DecoratedString.cs:
if (outputRendering == OutputRendering.Automatic)
{
outputRendering = OutputRendering.Ansi;Should we fix these too?
|
Formally it is a breaking change. |
|
The other usage of OutputRendering.Ansi is expected. |
|
@SteveL-MSFT Since now I may not fully understand the purpose of this change :) So, it looks like |
|
Yeah, we should probably get rid of |
OutputRendering.Host and remove OutputRendering.Automatic
|
What if we want to change the default behavior for different systems after receiving feedback? |
|
@iSazonov perhaps not "ideal", but we can always add a |
src/System.Management.Automation/FormatAndOutput/common/StringDecorated.cs
Outdated
Show resolved
Hide resolved
|
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 |
| if (outputRendering == OutputRendering.Host) | ||
| { | ||
| return _text; | ||
| throw new ArgumentException(StringDecoratedStrings.RequireExplicitRendering); |
There was a problem hiding this comment.
It is not good to throw in public ToString().
There was a problem hiding this comment.
If you take a closer look, you will find it won't throw in ToString(). The exception could happen only when user calling ToString(OutputRendering.Host).
|
🎉 Handy links: |
|
🎉 Handy links: |
PR Summary
The original behavior of
Automaticwas borrowed from Linux where redirection kept ANSI codes and you would need to explicitly suppress it. macOS, on the other hand, did the opposite. I think for most users, particularly Windows users, it would be better to default toHostso that ANSI is only written to the host and never to the pipeline. This will reduce confusion for users when they redirect text to a file and see ANSI codes in it. Users who want to keep the ANSI codes can always use$PSStyle.OutputRendering = 'Ansi'to force it.PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.(which runs in a different PS Host).