rustfmt icon indicating copy to clipboard operation
rustfmt copied to clipboard

Use `blank_lines_*` to control newlines between module level attrs

Open ytmimi opened this issue 3 years ago • 3 comments

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.

ytmimi avatar Jul 11 '22 05:07 ytmimi

I'm also happy to make the change referenced here https://github.com/rust-lang/rustfmt/pull/4295#discussion_r448070933, and rename push_vertical_spaces to push_newlines.

ytmimi avatar Jul 11 '22 17:07 ytmimi

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.

calebcartwright avatar Jul 19 '22 02:07 calebcartwright

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 https://github.com/rust-lang/rustfmt/issues/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 avatar Jul 19 '22 03:07 ytmimi