Skip to content

Change default value for blank_lines_upper_bound from 1 to 5 #4087

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

Closed
wants to merge 1 commit into from
Closed

Change default value for blank_lines_upper_bound from 1 to 5 #4087

wants to merge 1 commit into from

Conversation

LukasKalbertodt
Copy link
Member

I explained my reasoning in this comment, copied here for convenience:

May I ask why the default is 1? That's basically the only thing where I disagree with the standard style. I'd love to avoid having a rustfmt.toml.

My reasoning: more space between items is a useful visual separator. Rust files tend to get fairly long and often contain many different kinds of items. Compare that to some other languages, where there is usually just one class per file, for example. So in Rust files I often have "blocks" of items that somehow belong together. Maybe it's something like:

struct A;

impl A {
}

impl Display for A {
}


struct B;

impl B {
}

impl Display for B {
}

I think it should be pretty clear that some items belong closer together than others (specifically: all the A things should be somehow separated from all B things). And this is just a small example. Often I have multiple "layers" of items. So I naturally want to have larger separation between items of different "blocks". I usually use up to three blank lines between items.

So I'd like to propose increasing the default value to 3 or even 5. I think rustfmt shouldn't really be getting in the way of the programmer in this case.

But maybe people have strong opinions against large separators. I tried searching for previous discussions about this, but I found nothing. So please let me know either way!

As people agreed with my comment and no one had any counter argument, I just changed it now.

I hope this is a step in stabilizing blank_lines_upper_bound and blank_lines_lower_bound.

CC @ForLoveOfCats

Unverified

This user has not yet uploaded their public signing key.
@@ -13,7 +16,7 @@
- Fix aligning comments of different group
- Fix flattening imports with a single `self`.
- Fix removing attributes on function parameters.
- Fix removing `impl` keyword from opaque type.
Copy link
Member Author

Choose a reason for hiding this comment

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

My editor removed some trailing whitespace in this file. If I should remove those stray changes, just tell me.

@LukasKalbertodt
Copy link
Member Author

Will fix the tests soon, probably later today.

@calebcartwright
Copy link
Member

I believe the default values are based on the style guide, specifically https://github.com/rust-dev-tools/fmt-rfcs/blob/master/guide/guide.md#blank-lines.

IMHO the default values for the options in rustfmt need to align with the guide.

@ForLoveOfCats
Copy link

As I was CC-ed I figured I might as well throw my two cents in. In my opinion the defaults need not change, I just would really like to be able to configure this without using the nightly toolchain

@calebcartwright
Copy link
Member

@ForLoveOfCats - It may not be long lived, but note that you can use unstable rustfmt options on stable via cli config overrides

cargo fmt -- --config blank_lines_upper_bound=3

@LukasKalbertodt
Copy link
Member Author

I found the original thread where this topic has been discussed: rust-lang/style-team#57. I commented there, explaining my position. So yeah, I guess we first need to fix the style guide, then rustfmt. So I'll close this PR for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants