don't insert linebreaks to output (except for tables) #5193

SteveL-MSFT · 2017-10-21T21:18:25Z

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

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)

wrap calls to get WindowWidth and default to 120 columns if it fails

iSazonov · 2017-10-23T05:19:34Z

test/powershell/Language/Scripting/NativeExecution/NativeStreams.Tests.ps1

+            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


Maybe use "BeExactly"? It seems better for testing a formatting.

The problem currently is that redirection writes with a BOM, so this would fail

We read with Get-Content -Path $testdrive\error.txt - I believe there is not BOM if we don't use -Byte.

Good point about Get-Content, I'll change it

iSazonov · 2017-10-23T05:20:14Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Write-Debug.Tests.ps1

+        $origDebugPref = $DebugPreference
+        $DebugPreference = "Continue"
+        try {
+            $out = Write-Debug $text *>&1


Maybe use exactly "2>&1"

Debug isn’t stream number 2 and it seems you can only redirect debug if you redirect all streams

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

Maybe I did something wrong when I tried it. Will update.

iSazonov · 2017-10-23T05:21:18Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Write-Debug.Tests.ps1

+        $DebugPreference = "Continue"
+        try {
+            $out = Write-Debug $text *>&1
+            $out | Should Match $text


Maybe use "$out | Should BeExactly $longtext"?

Same problem as above in that redirection currently adds a BOM

iSazonov · 2017-10-23T05:21:33Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Write-Debug.Tests.ps1

+            $DebugPreference = $origDebugPref
+        }
+    }
+}


Please add new line at EOF.

iSazonov · 2017-10-23T05:32:36Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Write-Error.Tests.ps1

+        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


Can we use more strong match pattern? I'm again worried about false positives.

Write-Error adds extra info prefixed to the text. The important bits for this test is the long text not getting choppedby a linebreak

I see - no false positives can be.

Closed.

iSazonov · 2017-10-23T05:33:02Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Write-Verbose.Tests.ps1

+        $origVerbosePref = $VerbosePreference
+        $VerbosePreference = "continue"
+        try {
+            $out = Write-Verbose $text *>&1


The same "2>&1".

Verbose isn’t stream 2, I believe it’s 3, but redirection only worked when using all streams

iSazonov · 2017-10-23T05:34:27Z

test/powershell/Modules/Microsoft.PowerShell.Utility/Write-Verbose.Tests.ps1

+        $VerbosePreference = "continue"
+        try {
+            $out = Write-Verbose $text *>&1
+            $out | Should Match $text


The same about "BeExactly".

Same as above, BOM gets added

iSazonov · 2017-10-23T05:35:41Z

...ystem.Management.Automation/commands/utility/FormatAndOutput/common/BaseOutputtingCommand.cs

                }

                int columnsOnTheScreen = this.InnerCommand._lo.ColumnNumber;
+                if (columnsOnTheScreen == int.MaxValue)


Could you please add comments about the special cases?

Yes, will add

address iSazonov's comments

iSazonov · 2017-10-24T13:00:27Z

src/Microsoft.PowerShell.ConsoleHost/host/msh/ConsoleHostUserInterface.cs

            }
            else
            {
                // NTRAID#Windows OS Bugs-1061752-2004/12/15-sburns should read a skin setting here...


Is the comment still actual?

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.

Thanks for clarify.

Closed.

iSazonov · 2017-10-24T13:02:47Z

...ystem.Management.Automation/commands/utility/FormatAndOutput/common/BaseOutputtingCommand.cs

                }

                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


Can we rephrase it for easy reading?

And why 120? Can we use current windows width? Have we tests for this?

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.

expanded on comment for computing width for tables

lzybkr

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.

daxian-dbw · 2017-10-30T16:53:34Z

Given that @iSazonov's comments have been addressed, I will merge this PR.

SteveL-MSFT assigned daxian-dbw Oct 21, 2017

SteveL-MSFT requested a review from lzybkr October 21, 2017 21:18

SteveL-MSFT requested review from JamesWTruher, PaulHigin, anmenaga, dantraMSFT and daxian-dbw as code owners October 21, 2017 21:18

SteveL-MSFT force-pushed the no-linebreak-output branch from d27bd62 to 1b9c754 Compare October 22, 2017 00:35

SteveL-MSFT force-pushed the no-linebreak-output branch from 1b9c754 to 57c8c6f Compare October 22, 2017 01:20

[feature]

98d3c1f

wrap calls to get WindowWidth and default to 120 columns if it fails

SteveL-MSFT added the Breaking-Change breaking change that may affect users label Oct 22, 2017

iSazonov reviewed Oct 23, 2017

View reviewed changes

[feature]

5c1a1b4

address iSazonov's comments

iSazonov reviewed Oct 24, 2017

View reviewed changes

[feature]

a08b22e

expanded on comment for computing width for tables

lzybkr approved these changes Oct 27, 2017

View reviewed changes

daxian-dbw merged commit a384c6e into PowerShell:master Oct 30, 2017

SteveL-MSFT deleted the no-linebreak-output branch November 12, 2017 05:17

This was referenced Mar 23, 2018

"Get-ChildItem | Format-Wide -AutoSize | Out-String" can't work on Powershell 6.0.1 & 6.0.2 #6471

Closed

Fix error when 'Format-Wide -AutoSize | Out-String' is called. #6491

Merged

don't insert linebreaks to output (except for tables) #5193

don't insert linebreaks to output (except for tables) #5193

Uh oh!

Conversation

SteveL-MSFT commented Oct 21, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov Oct 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov Oct 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

iSazonov Oct 24, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lzybkr left a comment

Choose a reason for hiding this comment

Uh oh!

daxian-dbw commented Oct 30, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

iSazonov Oct 23, 2017 •

edited

Loading

iSazonov Oct 23, 2017 •

edited

Loading

iSazonov Oct 24, 2017 •

edited

Loading