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

Use blank_lines_* to control newlines between module level attrs #5438

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ytmimi
Copy link
Contributor

@ytmimi ytmimi commented Jul 11, 2022

Closes #4082

Now semantic meaning created by grouping attributes into "paragraphs" is maintained. Setting blank_lines_* will not add blank lines between module level attributes if they didn't already exist in the source code, but when they do exist the blank_lines_* config options will dictate how many blank lines are emitted into the output.

For example,

No blank lines are added or removed here because none exist

#![feature(core_intrinsics)]
#![feature(lang_items)]
#![feature(libc)]
#![feature(nll)]

blank lines between groups are maintained here because they are present in the original source

#![feature(core_intrinsics)]
#![feature(lang_items)]

#![feature(libc)]
#![feature(nll)]

To prevent breaking formatting changes newlines are only affected when version=Two is set.

It might be best to review this PR 1 commit at a time.

To better reuse the logic in `FmtVisitor::push_vertical_spaces` it was
extracted into its own function.

`FmtVisitor::push_vertical_spaces` now calls the stand alone function in
it's implementation.
A new parameter was added to recover_missing_comment_in_span to control
whether leading newline characters should always be removed from its
output.
Closes 4082

Now semantic meaning created by grouping attributes into "paragraphs"
is maintained.

To prevent breaking formatting changes newlines are only affected when
`version=Two` is set.
@ytmimi ytmimi changed the title Use blank_lines_* to control newlines between module level attrs Use blank_lines_* to control newlines between module level attrs Jul 11, 2022
@ytmimi
Copy link
Contributor Author

ytmimi commented Jul 11, 2022

I'm also happy to make the change referenced here #4295 (comment), and rename push_vertical_spaces to push_newlines.

@calebcartwright
Copy link
Member

Won't get around to looking at the diff in the immediate future, but did want to note I was suggesting we consider a new set of options in the same vein as the current blank lines options. I worry that if we try to use a single option for everything that we'll end up with another brace_style or use_small_heuristics (back when it was the single public option) that won't provide sufficient granularity for what users need.

@ytmimi
Copy link
Contributor Author

ytmimi commented Jul 19, 2022

Won't get around to looking at the diff in the immediate future

That's totally fine! No rush. I know we've got other priorities right now.

I was suggesting we consider a new set of options in the same vein as the current blank lines options. I worry that if we try to use a single option for everything that we'll end up with another brace_style or use_small_heuristics (back when it was the single public option) that won't provide sufficient granularity for what users need.

Ahh gotcha. I think I might have misunderstood things when I read through #4082. Totally happy to put this on hold or even propose a separate PR where I implement this using a new config option just for module level attributes.

@ytmimi ytmimi added the p-low label Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustfmt removes empty newlines between blocks of module-level attributes
2 participants