Remove extra newlines from formatting which resulted in necessary double newlines #8247

SteveL-MSFT · Nov 13, 2018

PR Summary

When formatting, the Group start and end added unnecessary newlines which resulted in double newlines in many cases. Formatting changes are not considered breaking.

(get-module | out-string).replace("`n","\n`n")

Before:

\n
ModuleType Version    Name                                ExportedCommands\n
---------- -------    ----                                ----------------\n
Manifest   6.1.0.0    Microsoft.PowerShell.Management     {Add-Content, Clear-Content, Clear-Item, Clear-ItemProperty...}\n
Manifest   6.1.0.0    Microsoft.PowerShell.Utility        {Add-Member, Add-Type, Clear-Variable, Compare-Object...}\n
Script     2.0.0      PSReadLine                          {Get-PSReadLineKeyHandler, Get-PSReadLineOption, Remove-PSReadLineKeyHandler, Set-PSReadLineKeyHandler...}\n
\n
\n

After:

\n             
ModuleType Version    Name                                ExportedCommands\n
---------- -------    ----                                ----------------\n
Manifest   6.1.0.0    Microsoft.PowerShell.Management     {Add-Content, Clear-Content, Clear-Item, Clear-ItemProperty...}\n
Manifest   6.1.0.0    Microsoft.PowerShell.Utility        {Add-Member, Add-Type, Clear-Variable, Compare-Object...}\n
Script     2.0.0      PSReadLine                          {Get-PSReadLineKeyHandler, Get-PSReadLineOption, Remove-PSReadLineKeyHandler, Set-PSReadLineKeyHandler...}\n
\n

There are still a few cases where the final output has two newlines (but not three anymore!). This is a bit more complicated to fix and would need to track state that either a newline was the last thing output or the item is the last in a collection and doesn't need a separator.

Fix #7186

PR Checklist

iSazonov · Nov 13, 2018

What we get with:

dir | Format-Table -GroupBy Name

SteveL-MSFT · Nov 13, 2018

(get-process pwsh | format-list | Out-String).Replace("`n","\n`n")

Before:

\n
\n
Id      : 1626\n
Handles : 0\n
CPU     : 5697.7865177\n
SI      : 422\n
Name    : pwsh\n
\n
Id      : 18927\n
Handles : 0\n
CPU     : 480.0566228\n
SI      : 18920\n
Name    : pwsh\n
\n
Id      : 18928\n
Handles : 0\n
CPU     : 771.2722798\n
SI      : 422\n
Name    : pwsh\n
\n
\n
\n

After:

\n
Id      : 1626\n
Handles : 0\n
CPU     : 5696.2817117\n
SI      : 422\n
Name    : pwsh\n
\n
Id      : 18927\n
Handles : 0\n
CPU     : 474.75417\n
SI      : 18920\n
Name    : pwsh\n
\n
Id      : 18928\n
Handles : 0\n
CPU     : 769.8305049\n
SI      : 422\n
Name    : pwsh\n
\n
\n

SteveL-MSFT · Nov 13, 2018

(dir | select -first 3 |Format-Table -GroupBy Name | out-string).replace("`n","\n`n")

Before:

\n
\n
    Directory: /Users/steve/repos/PowerShell\n
\n
\n
Mode                LastWriteTime         Length Name\n
----                -------------         ------ ----\n
d-----           11/4/18  7:47 AM                assets\n
\n
\n
    Directory: /Users/steve/repos/PowerShell\n
\n
\n
Mode                LastWriteTime         Length Name\n
----                -------------         ------ ----\n
d-----          10/26/18 10:24 AM                demos\n
\n
\n
    Directory: /Users/steve/repos/PowerShell\n
\n
\n
Mode                LastWriteTime         Length Name\n
----                -------------         ------ ----\n
d-----          10/26/18 10:24 AM                docker\n
\n
\n

After:

\n
\n
    Directory: /Users/steve/repos/PowerShell\n
\n
Mode                LastWriteTime         Length Name\n
----                -------------         ------ ----\n
d-----           11/4/18  7:47 AM                assets\n
\n
    Directory: /Users/steve/repos/PowerShell\n
\n
Mode                LastWriteTime         Length Name\n
----                -------------         ------ ----\n
d-----          10/26/18 10:24 AM                demos\n
\n
    Directory: /Users/steve/repos/PowerShell\n
\n
Mode                LastWriteTime         Length Name\n
----                -------------         ------ ----\n
d-----          10/26/18 10:24 AM                docker\n
\n

iSazonov · Nov 13, 2018

Is it ok to have one '\n' before "Directory ..."?

SteveL-MSFT · Nov 13, 2018

@iSazonov you are asking if it's ok to separate the groups with just a single newline vs two? Seems fine to me and just as readable visually. In the future, we should leverage vt100 as a way to provide visual indicators rather than rely on whitespace only.

iSazonov · Nov 13, 2018

iSazonov approved these changes Nov 13, 2018

View changes

anmenaga · Nov 13, 2018

anmenaga approved these changes Nov 13, 2018

View changes

anmenaga · Nov 13, 2018

test/powershell/Language/Classes/Scripting.Classes.Attributes.Tests.ps1

@@ -401,7 +401,7 @@ Describe 'ValidateSet support a dynamically generated set' -Tag "CI" {
                Import-Module -Name $moduleFile -Force
                Test-ValidateSet 'Hello' | Should -BeExactly 'Hello'
            } finally {
-                Remove-Module -Name $moduleFile -Force
+                Remove-Module -Name $moduleFile -Force -ErrorAction SilentlyContinue


This is not related to this formatting code change, right?

Correct. Was running the tests locally and this line was outputting unncessarily

adityapatwardhan · Nov 15, 2018

@iSazonov Reminder to add CL-XXX labels.

SteveL-MSFT assigned iSazonov Nov 13, 2018

SteveL-MSFT requested review from anmenaga and iSazonov Nov 13, 2018

[feature]
remove extra whitespace that is unnecessary and results in double newlines

Loading status checks…

49f7c17

SteveL-MSFT force-pushed the SteveL-MSFT:format-whitespace branch to 49f7c17 Nov 13, 2018

SteveL-MSFT deleted the SteveL-MSFT:format-whitespace branch Nov 14, 2018

adityapatwardhan added the CL-Engine label Nov 15, 2018

iSazonov referenced this pull request Dec 28, 2018
Closed
while help an alias with *ending,like help dir* or help ls*，the output is in confusion #8541

PowerShell/PowerShell

Remove extra newlines from formatting which resulted in necessary double newlines #8247

Remove extra newlines from formatting which resulted in necessary double newlines #8247

SteveL-MSFT commented Nov 13, 2018 •

edited

SteveL-MSFT assigned iSazonov Nov 13, 2018

SteveL-MSFT requested review from anmenaga and iSazonov Nov 13, 2018

SteveL-MSFT force-pushed the SteveL-MSFT:format-whitespace branch to `49f7c17` Nov 13, 2018

This comment has been minimized.

iSazonov commented Nov 13, 2018

This comment has been minimized.

SteveL-MSFT commented Nov 13, 2018 •

edited

This comment has been minimized.

SteveL-MSFT commented Nov 13, 2018 •

edited

This comment has been minimized.

iSazonov commented Nov 13, 2018

This comment has been minimized.

SteveL-MSFT commented Nov 13, 2018

iSazonov approved these changes Nov 13, 2018

View changes

anmenaga approved these changes Nov 13, 2018

View changes

This comment has been minimized.

This comment has been minimized.

iSazonov merged commit `43e0394` into PowerShell:master Nov 14, 2018
6 checks passed

6 checks passed

SteveL-MSFT deleted the SteveL-MSFT:format-whitespace branch Nov 14, 2018

adityapatwardhan added the CL-Engine label Nov 15, 2018

This comment has been minimized.

adityapatwardhan commented Nov 15, 2018

iSazonov referenced this pull request Dec 28, 2018

while help an alias with ending,like help dir or help ls*，the output is in confusion #8541

PowerShell/PowerShell

Join GitHub today

Remove extra newlines from formatting which resulted in necessary double newlines #8247

Conversation

SteveL-MSFT commented Nov 13, 2018 • edited

PR Summary

PR Checklist

SteveL-MSFT assigned iSazonov Nov 13, 2018

SteveL-MSFT requested review from anmenaga and iSazonov Nov 13, 2018

SteveL-MSFT force-pushed the SteveL-MSFT:format-whitespace branch to 49f7c17 Nov 13, 2018

This comment has been minimized.

iSazonov commented Nov 13, 2018

This comment has been minimized.

SteveL-MSFT commented Nov 13, 2018 • edited

This comment has been minimized.

SteveL-MSFT commented Nov 13, 2018 • edited

This comment has been minimized.

iSazonov commented Nov 13, 2018

This comment has been minimized.

SteveL-MSFT commented Nov 13, 2018

iSazonov approved these changes Nov 13, 2018 View changes

anmenaga approved these changes Nov 13, 2018 View changes

This comment has been minimized.

anmenaga Nov 13, 2018

This comment has been minimized.

SteveL-MSFT Nov 13, 2018

Hide details View details iSazonov merged commit 43e0394 into PowerShell:master Nov 14, 2018 6 checks passed

6 checks passed

SteveL-MSFT deleted the SteveL-MSFT:format-whitespace branch Nov 14, 2018

adityapatwardhan added the CL-Engine label Nov 15, 2018

This comment has been minimized.

adityapatwardhan commented Nov 15, 2018

iSazonov referenced this pull request Dec 28, 2018

while help an alias with *ending,like help dir* or help ls*，the output is in confusion #8541

SteveL-MSFT commented Nov 13, 2018 •

edited

SteveL-MSFT force-pushed the SteveL-MSFT:format-whitespace branch to `49f7c17` Nov 13, 2018

SteveL-MSFT commented Nov 13, 2018 •

edited

SteveL-MSFT commented Nov 13, 2018 •

edited

iSazonov approved these changes Nov 13, 2018

View changes

anmenaga approved these changes Nov 13, 2018

View changes

iSazonov merged commit `43e0394` into PowerShell:master Nov 14, 2018
6 checks passed

while help an alias with ending,like help dir or help ls*，the output is in confusion #8541