rustfmt icon indicating copy to clipboard operation
rustfmt copied to clipboard

[unstable option] blank_lines_upper_bound

Open scampi opened this issue 6 years ago • 14 comments

Tracking issue for unstable option: blank_lines_upper_bound

scampi avatar Feb 13 '19 22:02 scampi

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!

LukasKalbertodt avatar Jan 13 '20 17:01 LukasKalbertodt

I would also like to voice my support for stabilizing this option. I personally use single lines between "paragraphs" in blocks of code, two lines between related functions and structs, and three lines between unrelated sections of source. This feature requiring nightly is rather frustrating

ForLoveOfCats avatar Feb 29 '20 19:02 ForLoveOfCats

why is it nightly? I think it makes sense to be able to set it to 2

cztomsik avatar Apr 11 '20 14:04 cztomsik

The discussion here seems like we will not change the default, unfortunately. So we could go ahead and stabilize this option. Only these conditions left:

  • The design and implementation of the option are sound and clean.
  • The option is well tested, both in unit tests and, optimally, in real usage.
  • There is no open bug about the option that prevents its use.

LukasKalbertodt avatar May 03 '20 15:05 LukasKalbertodt

The discussion here seems like we will not change the default, unfortunately. So we could go ahead and stabilize this option. Only these conditions left:

  • The design and implementation of the option are sound and clean.
  • The option is well tested, both in unit tests and, optimally, in real usage.
  • There is no open bug about the option that prevents its use.

Where does this leave us now? Doesn't look like there are bugs open about the option other than this.

The other two points seem a bit subjective; a rust n00b like me certainly can't weigh in there, but I agree with the others that having this option in stable would be very nice.

Khouderchah-Alex avatar Jan 13 '21 18:01 Khouderchah-Alex

As a reminder, issues for tracking stabilization of rustfmt configuration options are not intended to be used for debating default formatting behavior set by the Style Guide that governs rustfmt's default behavior. Additionally, the discussion around this particular aspect of the default behavior in the Style Guide is closed, so additional comments about the default, particularly here within the rustfmt repository, are not particularly productive.

As far as actually stabilizing a configuration option, the process and requirements are defined here. While I appreciate that folks are eager to have the option available on stable, that really isn't the driver/determination of stabilization. Rust takes stability very seriously, and the formatting guarantee for rustfmt imposes additional constraints on the development of rustfmt and stabilization of features.

We won't stabilize any option unless and until those criteria have all been easily cleared and we're sufficiently confident in the implementation and that it won't cause any buggy or unexpected formatting behavior anywhere, regardless of how long the option has existed in unstable or how many users are interested in stabilization. I know it can be frustrating/annoying for options to not be available on stable, but premature stabilization is absolutely unacceptable and why we always err on the side of caution and taking longer.

blank_lines_upper_bound, much like blank_lines_lower_bound, hasn't hit the threshold for stabilization yet, both because there's some known issues/gaps and because the current implementation in the released versions of rustfmt is likely not the direction we want to go (#4295, #2954, #4082, etc. note that some closed issues are still applicable/blocking)

calebcartwright avatar Feb 15 '21 22:02 calebcartwright

The option is well tested, both in unit tests and, optimally, in real usage.

I'm not sure how real usage is quantified typically, but for what it's worth, the feature is working well for us in a number of projects, and it is the main reason why these projects rely on the 2.0 RC branch.

bluenote10 avatar Nov 24 '21 09:11 bluenote10

Only these conditions left:

  • The design and implementation of the option are sound and clean.

Can't comment on that.

  • The option is well tested, both in unit tests and, optimally, in real usage.

I don't know about being well tested in unit tests, but real world usage is definitely present. Search for blank_lines_upper_bound on GH and file type TOML -> ~1600 results, which is absolutely comparable to the stable remove_nested_parens (~2500 results), even when disregarding the hurdle to use unstable features (ie. the nightly chain)

  • There is no open bug about the option that prevents its use.

The only one that's referenced is https://github.com/rust-lang/rustfmt/issues/4082, which appears to be a wontfix.

TobTobXX avatar Sep 02 '22 20:09 TobTobXX

Dear all,

I would really like to understand which option would allow me to have an explicit blank line between a fn signature and its body :question: Asking since initially I hoped this option would allow it.

I really want this (and used it in other languages) for the cases where the function signature is long or complex enough to need a blank line between it and its body to improve readability.

For example:

  • Instead of having such formatted result:
      pub async fn add(
          &self,
          slug: &String,
          title: &String,
          description: &String,
          body: &String,
          tag_list: &Vec<String>,
          author_id: i64,
      ) -> Result<DateTime<Utc>, AppError> {
          let mut txn = self.dbcp.begin().await?;
          let article_id: i64;
          let created_at: DateTime<Utc>;
    
  • I'd really prefer a clear separation (for a better readability):
      pub async fn add(
          &self,
          slug: &String,
          title: &String,
          description: &String,
          body: &String,
          tag_list: &Vec<String>,
          author_id: i64,
      ) -> Result<DateTime<Utc>, AppError> {
    
          let mut txn = self.dbcp.begin().await?;
          let article_id: i64;
          let created_at: DateTime<Utc>;
    

Testing using the nightly toolchain (by running rustup override set nightly, plus even creating a rust-toolchain file with the content of nightly within it, part of a cargo new ... project), having:

fn main() {

    println!("Hello, world!");
}

and running rustfmt --unstable-features src/main.rs it formats it as:

fn main() {
    println!("Hello, world!");
}

Thanks.

dxps avatar Dec 19 '22 07:12 dxps

@dxps I don't think that's possible.

EDIT: for reference, [clang-format calls this KeepEmptyLinesAtTheStartOfBlocks].

EDIT2: I deleted my previous comment arguing that the default should be changed, after getting used to 1, I think it does not matter.

marcospb19 avatar Dec 19 '22 16:12 marcospb19

@marcospb19 Appreciate the quick feedback! :pray: Sad, but true, some people are enforcing some things ... :disappointed: Iirc, that was the case with Dart as well.

dxps avatar Dec 19 '22 19:12 dxps

It's there any plans on passing that as stable ?

tkr-sh avatar Feb 06 '24 23:02 tkr-sh

What is so difficult in making this option stable? It has been years.

acanton77 avatar Mar 05 '24 22:03 acanton77

It's there any plans on passing that as stable ?

What is so difficult in making this option stable? It has been years.

please see #5365 and #5367

calebcartwright avatar Mar 05 '24 22:03 calebcartwright