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

Is blank_lines_lower_bound correct? #2954

Closed
gnzlbg opened this issue Aug 24, 2018 · 3 comments · Fixed by #4295
Closed

Is blank_lines_lower_bound correct? #2954

gnzlbg opened this issue Aug 24, 2018 · 3 comments · Fixed by #4295
Assignees
Labels
1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release bug Panic, non-idempotency, invalid code, etc. only-with-option requires a non-default option value to reproduce
Milestone

Comments

@gnzlbg
Copy link
Contributor

gnzlbg commented Aug 24, 2018

The blank_lines_lower_bound documentation says:

Minimum number of blank lines which must be put between items. If two items have fewer blank lines between them, additional blank lines are inserted.

Then the example shows that by setting it to 1:

fn foo() {
    println!("a");
}
fn bar() {
    println!("b");
    println!("c");
}

is formatted into

fn foo() {

    println!("a");
}
fn bar() {

    println!("b");

    println!("c");
}

which is how rustfmt behaves. However, I am not sure this is correct because aren't those println! are statements? Not all statements are items, e.g. let-decl are not items IIUC (and for let this option produces the same behavior). I think it would be much more useful to output:

fn foo() {
    println!("a");
}

fn bar() {
    println!("b");
    println!("c");
}

That is, it would be much more useful for this to control the spacing between items that are not statements. I don't know how useful it would be to also be able to control the spacing between statements (independently of whether they are items or not), but if someone needs that it could be added as a different option.

@gnzlbg
Copy link
Contributor Author

gnzlbg commented Aug 24, 2018

The option appears to be completely broken, as in, it just inserts N blank lines between each line that contains source code, although not always:

#[cfg(test)]
mod tests { }

becomes

#[cfg(test)]

mod tests {}

and

for i in 0..10 {
    let x = i;
    let y = i;
}

becomes

for i in 0..10 {
    let x = i;

    let y = i;
}

@topecongiro topecongiro added bug Panic, non-idempotency, invalid code, etc. only-with-option requires a non-default option value to reproduce labels Aug 26, 2018
@nrc
Copy link
Member

nrc commented Aug 26, 2018

Expected behaviour is that it should newlines between module-scoped items (even if they appear inside functions), but not between statements/expressions.

@faern
Copy link

faern commented Jan 30, 2019

I agree with the issue poster. The current functionality does not seem very useful. What I would like it to do is to separate type declarations, impl blocks, functions etc from each other. But not inserting a newline between two use or const or static. Basically keeping "blocks" of the same thing together. Personally I think it would be most helpful if it had this effect:

use std::io;
use std::net;
/// A module doing foo
mod foomodule;
#[cfg(windows)]
pub const CONST_NUMBER: u8 = 3;
pub static STATIC_NUMBER: u8 = 5;
fn function() {}
struct MyStruct;
impl MyStruct {}

Into:

use std::io;
use std::net;

/// A module doing foo
mod foomodule;

#[cfg(windows)]
pub const CONST_NUMBER: u8 = 3;
pub static STATIC_NUMBER: u8 = 5;

fn function() {}

struct MyStruct;

impl MyStruct {}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1x-backport:pending Fixed/resolved in source but not yet backported to a 1x branch and release bug Panic, non-idempotency, invalid code, etc. only-with-option requires a non-default option value to reproduce
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants