Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove extra line before formatting group #12163

Merged
merged 4 commits into from
May 28, 2020

Conversation

iSazonov
Copy link
Collaborator

@iSazonov iSazonov commented Mar 20, 2020

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
@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
Copy link
Member

@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

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
Collaborator Author

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

@TravisEz13
Copy link
Member

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

@TravisEz13 TravisEz13 removed the Review - Needed The PR is being reviewed label May 28, 2020
Copy link
Member

@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

Copy link
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

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
Collaborator Author

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

@musm
Copy link

musm commented May 29, 2020

WIll do, Thanks for this fix!

@ghost
Copy link

ghost commented Jun 25, 2020

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

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-Engine Indicates that a PR should be marked as an engine change in the Change Log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unwanted empty lines in the output of gci
6 participants