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

[unstable option] blank_lines_lower_bound #3382

Open
scampi opened this issue Feb 13, 2019 · 12 comments
Open

[unstable option] blank_lines_lower_bound #3382

scampi opened this issue Feb 13, 2019 · 12 comments
Labels
unstable option tracking issue of an unstable option

Comments

@scampi
Copy link
Contributor

scampi commented Feb 13, 2019

Tracking issue for blank_lines_lower_bound

@scampi scampi added the unstable option tracking issue of an unstable option label Feb 13, 2019
@Boscop
Copy link

Boscop commented Sep 27, 2019

It would be more useful if statements inside functions are not covered by this, only declarations!

Or is there another way to specify that I want to have at least 1 empty line between declarations (also between function-local declarations and whatever comes before/after)? (But not between statements in a function body.)

Example
Original Code (rustfmt will not change it with the default value of 0):

const A: u32 = 1;
enum B {
	C
}
impl B {
	// ...
}

Set to 1:

const A: u32 = 1;

enum B {
	C
}

impl B {
	// ...
}

@lunacookies
Copy link

What are the issues blocking the stabilisation of this option?

@calebcartwright
Copy link
Member

@arzg - The process and requirements for stabilizing a configuration option are described here. Specifically, there are several issues that exist with blank_lines_lower_bound in released versions of rustfmt (1.x versions), for example #2954.

Some fixes/improvements to blank_lines_lower_bound have been made within the rustfmt source, but they have not yet been released. As such there are at least two stabilization requirements not yet for this configuration option:

  • 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.

@Systemcluster
Copy link

With blank_lines_lower_bound set to 0, rustfmt (1.4.27-nightly (580d826e 2020-11-16)) will still insert blank lines between fns, for example

impl A {
    fn a() { /* ... */ }
    fn b() { /* ... */ }
}

will get formatted to

impl A {
    fn a() { /* ... */ }

    fn b() { /* ... */ }
}

Is this correct? I.e., should this case be covered by this configuration option?

@nrc
Copy link
Member

nrc commented Mar 8, 2021

It would be nice to have slightly more finegrained control with this option, in particular, I would like to allow items which fit on a single line to have a 0 value, but larger items to have a 1 value.

@bluss
Copy link
Member

bluss commented Apr 22, 2021

This option doesn't seem to have a useful behaviour at the moment, is it something that will be tweaked?

My expectation would be that items (functions, struct definitions, similar) are separated by the lines (the documentation says items).

What actually happens is that attributes are spaced with one line before their functions, statements by one line etc, doc comments by one line before their functions etc. (Using blank_lines_lower_bound = 1 rustfmt 1.4.37-nightly (0bd2b19 2021-04-03)

@@ -1460,6 +1490,7 @@ impl<A> Clone for OwnedArcRepr<A> {
 /// [`RawArrayView`](type.RawArrayView.html) /
 /// [`RawArrayViewMut`](type.RawArrayViewMut.html) for the array type!*
 #[derive(Copy, Clone)]
+
 // This is just a marker type, to carry the mutability and element type.
 pub struct RawViewRepr<A> {
     ptr: PhantomData<A>,
@@ -1467,7 +1498,9 @@ pub struct RawViewRepr<A> {
 
 impl<A> RawViewRepr<A> {
     #[inline(always)]
+
     fn new() -> Self {
+
         RawViewRepr { ptr: PhantomData }
     }
 }
@@ -1620,6 +1686,8 @@ mod impl_raw_views;
 mod impl_cow;
 
 /// Returns `true` if the pointer is aligned.
+
 pub(crate) fn is_aligned<T>(ptr: *const T) -> bool {
+
     (ptr as usize) % ::std::mem::align_of::<T>() == 0
 }

@calebcartwright
Copy link
Member

This option doesn't seem to have a useful behaviour at the moment, is it something that will be tweaked?

@bluss - your snippets are related to a bug (#2954) that's been fixed in source but not included yet in a release, it will (at a minimum) be corrected to only ensure spacing between items

@Keavon
Copy link

Keavon commented Aug 10, 2021

Hello, would it be possible to please get a update on the status of this being stabilized? Thank you very much.

@dcow
Copy link

dcow commented May 19, 2022

When

This option doesn't seem to have a useful behaviour at the moment, is it something that will be tweaked?

@bluss - your snippets are related to a bug (#2954) that's been fixed in source but not included yet in a release, it will (at a minimum) be corrected to only ensure spacing between items

I am still experiencing this bug.

@ytmimi
Copy link
Contributor

ytmimi commented May 19, 2022

@dcow I'd suggest that you open up a new issue to fully describe the problem that you're experiencing

@sezna
Copy link
Contributor

sezna commented May 7, 2024

I just finished reading the saga over here about newlines between definitions etc., and then found the implementation and ultimately this tracking issue. It seems like originally the consensus was to allow for configurability in whitespace between both statements and items. I understand this to mean configuration for statements, and configuration for items. But the implementation currently provides one option for both statements and items. As @bluss pointed out above, this is not particularly useful behavior.

I would say it is pretty common that teams want blank newlines between function definitions, but uncommon that teams want blank newlines between every single statement in a function. I don't see any discussion between the issue I linked above and the implementation, though, so I'm not sure how this new merged-configuration implementation was decided upon. Is there any other discussion I am missing, perhaps?

Also, thanks for all the work up until this point. Obviously the tool is great and I'm happy that the discussion has made it this far. I am just seeking clarity on what happened, and if it is still planned to add configurability for spaces between definitions.

@dxps
Copy link

dxps commented May 8, 2024

I would say it is pretty common that teams want blank newlines between function definitions

Yes, that would be preferrable, as some functions signature becomes readable when a new blank line within the function body exists.

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

No branches or pull requests