Skip to content

Remove extra line before formatting group - #12163

Merged
Travis Plunk (TravisEz13) merged 4 commits into
PowerShell:masterfrom
iSazonov:remove-extra-line-gci
May 28, 2020
Merged

Remove extra line before formatting group#12163
Travis Plunk (TravisEz13) merged 4 commits into
PowerShell:masterfrom
iSazonov:remove-extra-line-gci

Conversation

@iSazonov

@iSazonov Ilya (iSazonov) commented Mar 20, 2020 •
edited
Loading

Copy link
Copy Markdown
Collaborator

PR Summary

Fix #11846.

Add newline only before each formatting group except first.

PR Context

PR Checklist

@iSazonov Ilya (iSazonov) changed the title WIP: Remove extra line before formatting group Remove extra line before formatting group Mar 20, 2020
@iSazonov Ilya (iSazonov) added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label Mar 20, 2020
@iSazonov Ilya (iSazonov) added this to the 7.1.0-preview.1 milestone Mar 20, 2020
@TravisEz13 Travis Plunk (TravisEz13) added CL-Engine Indicates that a PR should be marked as an engine change in the Change Log and removed CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log labels Apr 14, 2020
JamesWTruher
James Truher (JamesWTruher) reviewed Apr 14, 2020
View reviewed changes

@JamesWTruher James Truher (JamesWTruher) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although we don't usually have validation for formatting, it would be useful to have the discussion include what this looks like now vs your fix with some sample output.

Comment thread src/System.Management.Automation/FormatAndOutput/common/BaseFormattingCommand.cs
@ghost ghost added the Review - Needed The PR is being reviewed label May 27, 2020
@ghost

Deleted user (ghost) commented May 27, 2020

Copy link
Copy Markdown

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

@iSazonov

Ilya (iSazonov) commented May 27, 2020

Copy link
Copy Markdown
Collaborator Author

The PR removes first extra empty line - you can see screenshots in related issue.

@TravisEz13

Travis Plunk (TravisEz13) commented May 27, 2020

Copy link
Copy Markdown
Member

Steve Lee (@SteveL-MSFT) James Truher (@JamesWTruher) Can you update your review?

@TravisEz13 Travis Plunk (TravisEz13) removed the Review - Needed The PR is being reviewed label May 28, 2020
JamesWTruher
James Truher (JamesWTruher) approved these changes May 28, 2020
View reviewed changes

@JamesWTruher James Truher (JamesWTruher) left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks fine, it should probably done for format-list too

TravisEz13
Travis Plunk (TravisEz13) approved these changes May 28, 2020
View reviewed changes

@TravisEz13 Travis Plunk (TravisEz13) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approve as maintainer

@TravisEz13
Travis Plunk (TravisEz13) merged commit c602f82 into PowerShell:master May 28, 2020
@iSazonov
Ilya (iSazonov) deleted the remove-extra-line-gci branch May 29, 2020 03:10
@musm

Mustafa Mohamad (musm) commented May 29, 2020

Copy link
Copy Markdown

It would be good to audit the rest of the system. IMO gci still outputs several unnecessary new lines, which makes it too verbose. Clearly this PR is the correct fix, but future exploring this has utility.

@iSazonov

Ilya (iSazonov) commented May 29, 2020

Copy link
Copy Markdown
Collaborator Author

Mustafa Mohamad (@musm) If you don't see already opened issues for broken formatting please open new ones.

@musm

Mustafa Mohamad (musm) commented May 29, 2020

Copy link
Copy Markdown

WIll do, Thanks for this fix!

@musm Mustafa Mohamad (musm) mentioned this pull request Jun 19, 2020
Closed
@ghost

Deleted user (ghost) commented Jun 25, 2020

Copy link
Copy Markdown

🎉v7.1.0-preview.4 has been released which incorporates this pull request.:tada:

Handy links:

@ghost ghost mentioned this pull request Jun 25, 2020
Closed
Thatgfsj (Thatgfsj) pushed a commit to Thatgfsj/PowerShell that referenced this pull request Aug 6, 2026
@iSazonov
Remove extra line before formatting group (PowerShell#12163)
fa3f545
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TravisEz13 Travis Plunk (TravisEz13) TravisEz13 approved these changes
@SteveL-MSFT Steve Lee (SteveL-MSFT) SteveL-MSFT approved these changes
+2 more reviewers
@PoshChan PoshChan-Bot (PoshChan) PoshChan left review comments
@JamesWTruher James Truher (JamesWTruher) JamesWTruher approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@TravisEz13 Travis Plunk (TravisEz13)

Labels

CL-Engine Indicates that a PR should be marked as an engine change in the Change Log

Projects

None yet

Milestone

7.1.0-preview.4

Development

Successfully merging this pull request may close these issues.

Unwanted empty lines in the output of gci

6 participants

@iSazonov @TravisEz13 @musm @JamesWTruher @SteveL-MSFT @PoshChan