-
Notifications
You must be signed in to change notification settings - Fork 56
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
Convention about empty lines #57
Comments
I think we should only define a convention for top level statements inside modules, i.e. one line between functions, struct definitions, I don't think we should have any other requirements, and leave how to use newlines inside blocks and between control flow statements up to the user. |
I have been somewhat reluctant to address this in rustfmt, where we treat newlines as significant whitespace and preserve it in the same way as comments. The reason for this is that People often use newline semantically - no lines between related statements, a blank line between separate groups of statements, multiple blank lines to enforce an even greater separation. Likewise, between items at module scope, if there are multiple single line, related functions there might be no newlines. Mostly functions are separated by one line, sometimes grouped using multiple lines. Furthermore, it is really easy to fix manually - you just delete them, its not like you have to re-lay out code. There seem to be some cases we can safely forbid, e.g., blank lines at the start of end of a block. For other uses I see a few options:
I prefer 1 or 3 - I think 4 is too strict, and 2 is unnecessary. |
@nrc, I am up to 3 variant. |
On February 1, 2017 10:28:04 AM GMT+01:00, Nick Cameron ***@***.***> wrote:
I have been somewhat reluctant to address this in rustfmt, where we
treat newlines as significant whitespace and preserve it in the same
way as comments. The reason for this is that People often use newline
semantically - no lines between related statements, a blank line
between separate groups of statements, multiple blank lines to enforce
an even greater separation. Likewise, between items at module scope, if
there are multiple single line, related functions there might be no
newlines. Mostly functions are separated by one line, sometimes grouped
using multiple lines.
Furthermore, it is really easy to fix manually - you just delete them,
its not like you have to re-lay out code.
There seem to be some cases we can safely forbid, e.g., blank lines at
the start of end of a block.
For other uses I see a few options:
1. continue to do preserve all newlines
2. preserve 0, 1, or 2 blank lines, e.g., a 5 line gap would be reduced
to 2.
3. preserve 0 or 1 blank lines, e.g., a 5 line gap would be reduced to
1.
4. enforce 1 line between all items and do preserve function bodies (or
reduce multiple lines to 1 inside function bodies).
I prefer 1 or 3 - I think 4 is too strict, and 2 is unnecessary.
I would favor approach 3 as well, along with guidelines about top level, start and end of block, and similar.
|
I'd strongly prefer option 2 (edit: if we transform anything at all; [1] is also fine), because I do (borrowing from Python) tend to use 2 blank lines at the top level, and especially between different sections. I find extra spaces very helpful for visually separating logically/semantically distinct sections of the code, especially the Example//! Some module docs
//!
//! More module docs. All the docs. Docs and docs and docs.
// First-party dependencies
use std::String;
use std::borrow::Borrow;
// Third-party dependencies
use foo::Foo;
/// Hey look, a `Quux`.
pub struct Quux {
// ...
}
impl Quux {
/// Give me a `Quux`!
pub fn new() -> Quux {
// ...
}
}
impl Borrow<T> for Quux {
fn borrow(&self) -> &T {
// ...
}
}
/// This is a `Bar`.
pub enum Bar {
VariantA,
VariantB(i32),
VariantC { q: u64 },
}
impl Bar {
// ...
}
/// Hey, sometimes we have standalone functions, too
pub fn baz(bar: Bar) -> Quux {
// ...
} |
I used to use 2 line spaces as significant, however, my feeling more recently that this is too subtle to be useful and that if I want to separate groups of items, then they should be in different files, or I should use a comment to separate. Having said that, I am still somewhat sympathetic to this approach. We could have option 3, with option 2 as a config option. I think this is quite suitable as a config option - it seems reasonable to disagree with the default, and it doesn't make the code unfamiliar to change the value. |
Though I come from Python, I've come to appreciate that there is no fussing about "is it one space, or is it two" in Rust. I see no readability issue, and I've even come to visually appreciate the compactness the one-space approach that promotes. Looking at @chriskrycho preference, I be sad... way too sparce, especially with those doc comments. |
Entering FCP:
|
To be clear, this also applies to statements, right? E.g. if the max is set to 1 blank line, we should change the following: fn foo() {
bar();
baz();
quux();
} Into this: fn foo() {
bar();
baz();
quux();
} |
The minimum number of (consecutive) line breaks allowed? Is this talking about a minimum between certain elements, or a rule like "a blank line must always have another blank line before or after it"? Because the former sounds like it would have to be multiple options for different contexts, and the latter would be very weird IMHO. |
We could do. What do you think? I'm a little twitchier about statements than items, just because people are more likely to put spacing in, but I think it makes sense to apply the same treatment. |
Yeah, so you'd get the following formatting with the various settings:
The default would be I don't think different contexts would require different values. We might not want to apply this to statements, only to items, or have different values for items and statements. |
With regards to settings, to match what I do today, I would force one space between items (1...1) but zero or one space between statements (0...1). Given these items (EDIT: made them multi-line. My comment below addresses single-line items): fn foo() {
// ...
}
fn bar() {
// ...
}
fn baz() {
// ...
} I would produce: fn foo() {
// ...
}
fn bar() {
// ...
}
fn baz() {
// ...
} But given these similar statements: foo(); bar();
baz(); I would produce: foo();
bar();
baz(); Basically, I find value in user-controlled statement spacing but not in user-controlled item spacing. Items are usually clearly separated by doc comments or |
Some edge cases: fn foo() {
bar();
} fn foo() {
bar();
} I would remove these end-of-block and start-of-block blank lines in all cases. |
It is fairly common for people to have large numbers of small (one line) functions without line breaks (e.g., signatures in traits). I think we should support that (probably by default). |
Fair point. A more complete rule I follow for items is this:
Note that the following are considered multi-line items: #[inline(always)]
fn foo() {} /// Documentation!
fn bar() {} But the following is allowed: fn simple() {}
fn helper() {}
fn functions() {} Here is a general mix to show what my rule entails: /// Documentation!
fn foo() {}
fn bar() {}
fn baz() {}
fn multi_line() {
// ...
}
fn quux() {}
fn xyzzy() {} Note that you do place a blank line between a multi-line item and a single-line item to avoid a crowded look. |
At this point I'd rather just allow some flexibility for the user. I think some people do like to use blank space semantically here and that seems like something we could allow without too much bother. |
To elaborate on some of my reasoning, I like my style to avoid this anti-pattern I've seen in novice code: /// Group documentation about `a`, `b`, and `c`.
fn a() {}
fn b() {}
fn c() {} Since rustdoc will only apply the documentation to /// Group documentation about `a`, `b`, and `c`.
fn a() {}
fn b() {}
fn c() {} |
Another place this comes up is with fields or variants, which I treat the same way as items: // OK
struct Foo {
/// Docs for `a`
a: A,
/// Docs for `b`
b: B,
}
// OK
struct Foo {
a: A,
b: B,
}
// I would reject this
struct Foo {
/// Docs for `a`
a: A,
/// Docs for `b`
b: B,
} |
Every time we talk about this kind of flexibility, what I mostly think is "this is yet another thing I have to manually check for in code review". There's a definite trade off between what is automatic and what is flexible, and of course I like a moderately flexible treatment for spacing in statements, but I would like as much as possible to make things like this never exist in rustfmt's output: /// Documentation.
fn foo() {
// ...
// ...
// ...
}
/// Documentation.
fn bar() {
// ...
// ...
} Because I consider this level of density harmful, and it's easier for me to tell contributors to just run rustfmt. If this isn't handled automatically I need to have my own style guide or risk arguing about style in code reviews. I'd rather never even mention style in code reviews. So, I'm occasionally accepting but quite wary of flexibility. It weakens the benefit I get from using rustfmt. |
I agree. Enforcing exactly one blank line (two newlines) between top-level items seems fine. And requiring that statements appear on separate lines and have either zero or one blank line between them (with no blank lines at the start or end of a block) also seems fine. I'd rather rustfmt not eliminate blank lines between statements, but reducing multiple blank lines to one and ensuring that multiple statements don't appear on the same line seems fine. |
I am somewhat swayed by @solson's code review argument. However, I feel personally that the example of functions close together would not warrant an r- or even a comment and so I don't feel that it is so important for rustfmt to deal with it. I worry about the details here, for example the enum example:
That seems totally reasonable to me. However if I fear there are many such edge cases and it is an area where users are going to want the flexibility. It is also somewhere where the benefit of having Rustfmt do it for you is not so large - it is much easier to hit enter than to align a bunch of expressions. And of course, if a project maintainer does want to be super-strict they can use a config file and tighten up the defaults. |
I do tend to have empty lines at the beginnings of blocks, for example when the first block element is commented. |
@phaylon Can you provide an example of that? |
Presumably rustfmt would notice the fact that there's a commented block, so it wouldn't remove whitespace in that case. |
@joshtriplett One I could find quickly:
But looking through my code, I tend to have a leading empty line whenever a block of code is split up in multiple segments. Otherwise I find the signature and the first part bleed together. I do skip that when there is a single |
Ah, I misunderstood your previous comment. I would definitely omit that blank line in my own code. |
@solson Likewise. |
We discussed this at the meeting today. The basic framework is that Rustfmt should be configurable to specify the minimum and maximum number of line breaks allowed for both statements and items. The default minimum would be 1 and maximum would be 2. I.e., you can have either 0 or 1 blank lines between everything. An extension would be to have min and max options for one-line items. This would address @solson's use case. By default these would have the same value as the multi-liners. However, by using 0 here and 1 for multi-line items (both min and max) you get totally locked down formatting. @solson, does this satisfy you? We agreed that at least initially it is best to have some flexibility, rather than trying to nail down exact rules for when to allow blank lines. In the long term we could add more rules, but I fear that the cost in implementation complexity (risk of getting it wrong in some cases) outweighs the benefit. We decided against allowing blank lines to start/end blocks. |
@nrc That sounds good to me. |
I'd like to pitch in strongly against the idea of a single empty line between top-level code. Even with documentation, two empty lines between major sections of code is a huge help when scanning quickly through a file. I would suggest the following as a default:
There's no need for a cramped style that limits spacing. This is a compiled language, and space is cheap and helpful. I come from a publishing background, and large, significant margins are crucial to the quick comprehension of text, especially in technical lists. I would put forward that the default be two blank lines between function or module definitions, and a single leading and trailing blank line in function definitions, with permitted blank lines within the function body. We want to encourage people to write large. This also helps people have an incentive to keep function bodies short and concise. Additionally, extra spacing can visually isolate unusually long one-liners, encouraging refactoring. |
I always remove these kinds of lines personally; I really like lots of
whitespace, but this feels like too much.
…On Thu, Apr 27, 2017 at 9:51 AM, OtterCoder ***@***.***> wrote:
I'd like to pitch in strongly against the idea of a single empty line
between top-level code. Even with documentation, two empty lines between
major sections of code is a huge help when scanning quickly through a file.
I would suggest the following as a default:
/// Documentation
fn foo() {
...
}
/// Documentation
fn bar() {
let baz = x;
let qux = y;
// comment
baz * qux
}
There's no need for a cramped style that limits spacing. This is a
compiled language, and space is cheap and helpful. I come from a publishing
background, and large, significant margins are crucial to the quick
comprehension of text, especially in technical lists. I would put forward
that the default be two blank lines between function or module definitions,
and a single leading and trailing blank line in function definitions, with
permitted blank lines within the function body.
We want to encourage people to write large. This also helps people have an
incentive to keep function bodies short and concise. Additionally, extra
spacing can visually isolate unusually long one-liners, encouraging
refactoring.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#57 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABsileuBrM7BuEmvup2MtIozS3XZcbVks5r0J1-gaJpZM4Lzhyj>
.
|
@OtterCode I think the consensus is against you, sorry. However, this definitely seems the sort of thing that reasonable people can disagree about and thus should be supported by configuration options, so although your proposal will not be the default, you will be able to configure Rustfmt to do what you want. |
Sorry for commenting on this old issue, but I strongly disagree with the reasoning for limiting the number of empty lines that users can put between statements. I commented here, but here is a copy for convenience:
This is basically the argument mentioned by several different people in this thread already. I do understand that some people might prefer limiting the empty lines. However, it seems wrong to me to enforce this style choice so aggressively. I think enforcing style makes sense where a divergence from that style seriously hurts readability. I don't think that's the case here. So I'd really like to get this upper limited removed or at least relaxed to some fairly high number. From reading this thread, it also doesn't seem to me at all that there is a clear consensus. Several people argued in favor of using multiple empty lines as visual separator or to at least not enforce this behavior from the official Rust style/rustfmt. |
First of all, I certainly think it's reasonable for rustfmt to allow configuring this. This issue, however, is about the default style. And in the default style, I personally believe we should continue eliminating excess blank lines, just as we eliminate excess horizontal space between tokens. The default style exists to provide consistency, formatting the same code the same way for everyone. Personally, if I feel that a single blank line doesn't separate code enough, I tend to add a comment rather than an extra blank line. |
Absolutely. I think we all agree on that. This already exists as an option, but it's nightly only. To stabilize it, we need to decide whether the default value is good -- which I don't think is the case.
I don't think this is a good comparison. Lines are limited in length already, thus a larger separation between "less related" parts of the line is not as important. Rust files on the other hand regularly have over 1000s of lines.
And that's good. I really like having a more or less community-accepted style in Rust. Of course, such a default style has to strike a fine balance between permissive and restrictive. It wouldn't make sense to put a rule in the default style that a large majority of the community disagrees with, right? That would only lead a large part of the community to abandon the default style alltogether. So how does the community think? I checked from this and the tracking issue. See the full details: (I tried really hard not to include all comments or upvotes/downvotes. If I missed something, please tell me.) In favor of only (0 or) 1 lines:
In favor of allowing more than 1 empty line:
(And in case that matters, three of those comments mentioned "strong disagreement") nrc commented in this thread a lot, but I think they stated a definitive opinion on what the default should be. They commented this, however:
(This matches my experience. I also saw many people using newlines like that.) When collecting and deduping all people that commented, upvoted or downvoted, the result is: 0 or 1 lines
Do not restrict to 1 line
In total: 9 people in favor of restricting to 0 or 1 empty lines, 13 people against this restriction. Obviously, that's a really small sample size and does probably not exactly represent the whole Rust community. However, it is enough (in my opinion) to question the current default value. Additionally, I think that a restriction in the standard style should require a larger majority than to keep it permissive. What do you think about creating a PR changing the current default, and asking the community (via the usual means) to upvote or downvote that PR to make their opinion heard? If not, how do we resolve this? |
I disagree, I think it's more valuable to have a consistent default style (even if I disagree with it in parts) than to have a loose style that allows code to be more inconsistent. I think a much stronger argument would need to be made for the style to be permissive by default than to have a specific restriction that some people dislike. For the most part we want On another note, we really can't make decisions based on upvotes and downvotes - it's essentially always a small, unrepresentative sample of the community. The default style can never be perfect for all people with strong style opinions, anyway, but I hope we can convince most people that community-wide consistency is more valuable than personally tailored preference, and at worst a short custom |
Thanks for your comments. While I still think this particular default is really not great, I see your point. I guess we can then go ahead and stabilize this rustfmt option. |
Hi everybody.
It looks like there is no any convention about empty lines.
I like to separate all function with one empty line.
I like to separate Control Flow Statements with one empty line.
I like to separate blocks with one empty line.
I like to separate unrelated pieces with one empty line.
I think we should have some convention about it.
Thanks,
Alexey
The text was updated successfully, but these errors were encountered: