Skip to content

Remove extra line before formatting group#12163

Merged
TravisEz13 merged 4 commits intoPowerShell:masterfrom
iSazonov:remove-extra-line-gci
May 28, 2020
Merged

Remove extra line before formatting group#12163
TravisEz13 merged 4 commits intoPowerShell:masterfrom
iSazonov:remove-extra-line-gci

Conversation

@iSazonov
Copy link
Copy Markdown
Collaborator

@iSazonov iSazonov commented Mar 20, 2020 •
edited
Loading

PR Summary

Fix #11846.

Add newline only before each formatting group except first.

PR Context

PR Checklist

@ghost ghost assigned TravisEz13 Mar 20, 2020
@iSazonov iSazonov changed the title WIP: Remove extra line before formatting group Remove extra line before formatting group Mar 20, 2020
@iSazonov 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 iSazonov added this to the 7.1.0-preview.1 milestone Mar 20, 2020
@iSazonov iSazonov force-pushed the remove-extra-line-gci branch from cb52a75 to 76b436b Compare March 29, 2020 06:31
@TravisEz13 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
JamesWTruher reviewed Apr 14, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@JamesWTruher JamesWTruher left a comment

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.

@ghost ghost added the Review - Needed The PR is being reviewed label May 27, 2020
@ghost
Copy link
Copy Markdown

ghost commented May 27, 2020

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
Copy link
Copy Markdown
Collaborator Author

iSazonov commented May 27, 2020

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

@TravisEz13
Copy link
Copy Markdown
Member

TravisEz13 commented May 27, 2020

@SteveL-MSFT @JamesWTruher Can you update your review?

@TravisEz13 TravisEz13 removed the Review - Needed The PR is being reviewed label May 28, 2020
JamesWTruher
JamesWTruher approved these changes May 28, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@JamesWTruher JamesWTruher left a comment

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
TravisEz13 approved these changes May 28, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@TravisEz13 TravisEz13 left a comment

Choose a reason for hiding this comment

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

approve as maintainer

@TravisEz13 TravisEz13 merged commit c602f82 into PowerShell:master May 28, 2020
@iSazonov iSazonov deleted the remove-extra-line-gci branch May 29, 2020 03:10
@musm
Copy link
Copy Markdown

musm commented May 29, 2020

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
Copy link
Copy Markdown
Collaborator Author

iSazonov commented May 29, 2020

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

@musm
Copy link
Copy Markdown

musm commented May 29, 2020

WIll do, Thanks for this fix!

@musm musm mentioned this pull request Jun 19, 2020
Closed
@ghost
Copy link
Copy Markdown

ghost commented Jun 25, 2020

🎉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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@TravisEz13 TravisEz13 TravisEz13 approved these changes

@SteveL-MSFT SteveL-MSFT SteveL-MSFT approved these changes

+2 more reviewers

@PoshChan PoshChan PoshChan left review comments

@JamesWTruher JamesWTruher JamesWTruher approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@TravisEz13 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