Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
don't insert linebreaks to output (except for tables) #5193
Conversation
SteveL-MSFT
assigned
daxian-dbw
Oct 21, 2017
SteveL-MSFT
requested a review
from
lzybkr
Oct 21, 2017
SteveL-MSFT
requested review from
anmenaga,
dantraMSFT,
daxian-dbw,
JamesWTruher and
PaulHigin
as
code owners
Oct 21, 2017
SteveL-MSFT
added some commits
Oct 21, 2017
SteveL-MSFT
added
the
Breaking-Change
label
Oct 22, 2017
| @@ -921,6 +921,17 @@ internal override void Initialize() | ||
| } | ||
| int columnsOnTheScreen = this.InnerCommand._lo.ColumnNumber; | ||
| + if (columnsOnTheScreen == int.MaxValue) |
| + pwsh -c "& { [Console]::Error.WriteLine('$longtext') }" 2>&1 > $testdrive\error.txt | ||
| + $e = Get-Content -Path $testdrive\error.txt | ||
| + $e.Count | Should BeExactly 1 | ||
| + $e | Should Match $longtext |
SteveL-MSFT
Oct 23, 2017
Owner
The problem currently is that redirection writes with a BOM, so this would fail
iSazonov
Oct 23, 2017
Collaborator
We read with Get-Content -Path $testdrive\error.txt - I believe there is not BOM if we don't use -Byte.
| + $origDebugPref = $DebugPreference | ||
| + $DebugPreference = "Continue" | ||
| + try { | ||
| + $out = Write-Debug $text *>&1 |
SteveL-MSFT
Oct 23, 2017
Owner
Debug isn’t stream number 2 and it seems you can only redirect debug if you redirect all streams
iSazonov
Oct 23, 2017
•
Collaborator
It seems works:
PS> $a=Write-Verbose "ext " -Verbose 4>&1
PS> $a
VERBOSE: ext
PS> $b=Write-Debug "qwerty " -Debug 5>&1
Confirm
Continue with this operation?
[Y] Yes [A] Yes to All [H] Halt Command [S] Suspend [?] Help (default is "Y"): y
PS C:\Users\sie> $b
DEBUG: qwerty| + $DebugPreference = "Continue" | ||
| + try { | ||
| + $out = Write-Debug $text *>&1 | ||
| + $out | Should Match $text |
| + $DebugPreference = $origDebugPref | ||
| + } | ||
| + } | ||
| +} |
| + pwsh -c Write-Error -Message $longtext 2>&1 > $testdrive\error.txt | ||
| + $e = Get-Content -Path $testdrive\error.txt | ||
| + $e.Count | Should BeExactly 4 | ||
| + $e[0] | Should Match $longtext |
iSazonov
Oct 23, 2017
Collaborator
Can we use more strong match pattern? I'm again worried about false positives.
SteveL-MSFT
Oct 23, 2017
Owner
Write-Error adds extra info prefixed to the text. The important bits for this test is the long text not getting choppedby a linebreak
| + $origVerbosePref = $VerbosePreference | ||
| + $VerbosePreference = "continue" | ||
| + try { | ||
| + $out = Write-Verbose $text *>&1 |
SteveL-MSFT
Oct 23, 2017
Owner
Verbose isn’t stream 2, I believe it’s 3, but redirection only worked when using all streams
| + $VerbosePreference = "continue" | ||
| + try { | ||
| + $out = Write-Verbose $text *>&1 | ||
| + $out | Should Match $text |
| @@ -1248,7 +1206,7 @@ public override void WriteDebugLine(string message) | ||
| else | ||
| { | ||
| // NTRAID#Windows OS Bugs-1061752-2004/12/15-sburns should read a skin setting here... |
SteveL-MSFT
Oct 24, 2017
Owner
I believe the comment is saying that if we decide to support color themes (aka skins), this is one place to put the code for Write-Debug. Did a quick search for "read a skin" and found four places for Debug, Verbose, Warning, and ProgressPane.
| @@ -921,6 +921,18 @@ internal override void Initialize() | ||
| } | ||
| int columnsOnTheScreen = this.InnerCommand._lo.ColumnNumber; | ||
| + // for tables, we want to make sure we don't end up adding padding to int.MaxValue and default to 120 columns instead |
iSazonov
Oct 24, 2017
•
Collaborator
Can we rephrase it for easy reading?
And why 120? Can we use current windows width? Have we tests for this?
SteveL-MSFT
Oct 24, 2017
Owner
Let me try to add a bit more detail to make it easier to understand. The code tries to use window width, but if it can't be read (like implicit remoting where there's no console window), we have to default to something. 120 was the original default without my changes.
lzybkr
approved these changes
Oct 27, 2017
I tried out these changes - it looks like a big improvement for Format-List and no change for Format-Table or Format-Wide and error records aren't wrapped, so that's great.
Maybe in some future work we can detect when the output is to a file and be smarter about Format-Table and Format-Wide.
|
Given that @iSazonov's comments have been addressed, I will merge this PR. |


SteveL-MSFT commentedOct 21, 2017
don't use current console window width to add linebreaks to output (including debug, error, and verbose)
explicitly change errorrecord formatter to not add linebreaks
set width to int.maxvalue if not specified, in some special cases if width is int.maxvalue, set to console.windowwidth
so that you don't get int.maxvalue padding (like tables)
Related: #3813