rust-lang / style-team

Home of the Rust style team
Apache License 2.0
453 stars 56 forks source link

Alignment versus non-aligning indentation #8

Closed joshtriplett closed 7 years ago

joshtriplett commented 8 years ago

For a variety of cases, we should decide between alignment or non-aligning indentation. When breaking a line, should the item on the next line align with the item, or just have an additional level of indentation compared to the previous line? For instance:

fn alignment(arg: T,
             arg2: T2)

fn indentation(
        arg: T,
        arg2: T2)

let aligned = (value1
             + value2);
let indented = (
        value1
        + value2);
let indented2 = (
        value1
        + value2
);

let structval_aligned = S { field: value,
                            field2: value };

let structval_indented = S {
        field: value,
        field2: value,
};

function_call_aligned(param1,
                      param2);
function_call_indented(
    param1,
    param2);

This seems mostly orthogonal to indentation size, line length, and where exactly to break various types of lines.

I would propose avoiding alignment as a general principle. While alignment can sometimes improve readability, it produces significantly more noise in version control, since changes to one line can ripple outward into spacing changes on many lines. Alignment also tends to more quickly drift to the right edge of the screen, and produces worse results when wrapping lines close to a line-length limit.

In many cases, breaking the line and indenting before the first item provides the same benefit without the corresponding problem.

strega-nil commented 8 years ago

I agree, obviously :3

nrc commented 7 years ago

The terms we use in Rustfmt (which were terms of art elsewhere) are visual indent and block indent.

For block indent, there are two variations - either using a newline before the first argument or not:

fn some_function(arg1: Type1,
    arg2: Type2);

fn some_function(
    arg1: Type1,
    arg2: Type2);

We might want to consider some cases separately - currently Rustfmt use block indenting for struct literals and field/method chains, and visual indenting elsewhere. This brings up the issue of mixing, specifically, if you have a block indented item nested inside a visually indented item, do you block indent from the visual indent, or from the previous block indent?

In general, we must consider nesting carefully here.

I think we should probably decide on some of our guiding principles before dwelling too long here - this is a big and far-reaching decision and I think which choice we take comes down to how we prioritise issues:

@joshtriplett laid out arguments in favour of block indentation. It is also easier to implement and has fewer edge cases with nesting, etc.

However, visual indentation is (IMO) vastly more aesthetic in most situations, usually easier to read, and uses less vertical space (if adopting the second variation of block indentation).

It seems to me impossible to make a purely objective trade-off between these advantages, and so we must decide on the relative principle of our guiding principles first.

solson commented 7 years ago

For myself, visual indent is more frustrating to read both in general and because it leads rustfmt into pathological edge cases more often. So, for me it's all downsides, but it seems that most of us at least agree that block indent has a simplicity advantage.

Of the guiding principles that have been discussed so far, I think these are the ones where block indent wins uncontroversially:

And these are the ones where visual indent wins uncontroversially:

And these are the subjective ones I think could be argued both ways:

Many of the principles I listed for block indent are saying nearly the same thing, so a simple bullet point count score isn't fair, but I think it's still safe to say that block indent has more objective pros.

Unless a strong argument can be made that the subjective advantages of visual indent outweigh the advantages of block indent for the majority of the community, it seems logical to favour block indent.

nrc commented 7 years ago

Of the guiding principles that have been discussed so far

I think this is maybe jumping the gun a little bit - we should decide first which principles are important to us, and then make a decision here. The principles suggested so far are just suggestions, it may be that we decided that they should not be taken into account. And, as you point out, we should probably coalesce some of these principles.

solson commented 7 years ago

@nrc Yeah, that's fair. I think putting them to the test like this might help us decide which are important, but it's good to note these are just suggestions for principles at this point.

briansmith commented 7 years ago

I would propose avoiding alignment as a general principle. While alignment can sometimes improve readability, it produces significantly more noise in version control, since changes to one line can ripple outward into spacing changes on many lines. Alignment also tends to more quickly drift to the right edge of the screen, and produces worse results when wrapping lines close to a line-length limit.

In many cases, breaking the line and indenting before the first item provides the same benefit without the corresponding problem.

The policy I've seen for clangformat in Google projects I work on seems to be "Prefer to align wrapped values unless that would cause the wrapped result to have more lines." That is, column width > minimum number of lines > aligned in terms of priorities. I think this makes the most sense.

strega-nil commented 7 years ago

Another argument in favor of block indent: visual indents is poor for accessibility, at least how it's currently used in rustc (@camlorn from irc):

camlorn eddyb: I have to specifically put the cursor there, read off the column number, then align it by counting spacces. [sic]

(note: camlorn is blind)

retep998 commented 7 years ago

For block indent, there are two variations - either using a newline before the first argument or not:

@nrc there is another variation where the ending parentheses goes on a new line, and is the style that I prefer and use.

fn some_function(
    arg1: Type1,
    arg2: Type2,
);

I remain strongly opposed to any attempts at making aligned indentation the standard style.

joshtriplett commented 7 years ago

@retep998 In languages that allow a trailing comma on the last item, that's my preferred style as well for anything that doesn't fit comfortably on one line.

retep998 commented 7 years ago

I prefer my style because it is a very consistent style. Imagine if we formatted structs like this:

struct Foo { field1: Type1,
             field2: Type2 }
struct Foo { field1: Type1,
    field2: Type2 }
struct Foo {
    field1: Type1,
    field2: Type2 }

I don't see anyone clamoring to align structs like that, so why should we align functions like that?

joshtriplett commented 7 years ago

@retep998 I think we're in complete agreement here. :smile:

ahicks92 commented 7 years ago

Not going to be very opinionated here, as I'm probably the sole blind contributor to the project and am capable of differentiating between what is good for me and what is good for the majority. That said I like the block style, though I'm not sure why the original comment is using 8 spaces.

The specific problem with aligning with the left paren as a blind person is that you can't tell if it's wrong without an explicit check. I put it right the first time. Then the function's name changes or what have you, and there's nothing reminding me to fix it. Then the PR gets a comment about it and I'm annoyed because that's annoying and also to me it's all senseless anyway.

Very few blind programmers use Braille displays for reading code. The way indentation is exposed (if you bother to have it on) is by announcing the number of spaces at every change. Python is accessible, but indentation isn't about being pretty, it's about being semantical, for lack of a better word. I use it because it's helpful in getting closing braces right and sighted people will flat out refuse to read code without it in some cases, but otherwise I wouldn't even bother with it.

I also miss things like spaces after // or ///, spaces around ->, etc. And then there's the ones I absolutely don't mean to miss like spaces before {. And the ones that make no intuitive sense to me, like (.. vs. { .. }.

I suppose it would be worse if I had always been fully blind, but I had enough that I was able to get some idea of why code layout is how it is. Otherwise, this conversation would probably be going quite, quite differently...

joshtriplett commented 7 years ago

@camlorn To me, that's a substantial argument in favor of block indentation and against visual indentation. I appreciate your feedback and perspective.

Even when it's possible to see the misaligned visual alignment at a glance, people often send patches that don't update the alignment or that copy/pastes the alignment for another function, and often that issue gets missed, resulting in code that no longer has any sensible alignment. Together with the point that it's even more painful to deal with that alignment without visual feedback, that makes me even more inclined to avoid visual alignment.

I've also proposed in #4 that we take non-visual interfaces such as speech into account as a guiding principle. That doesn't mean it'll always be the highest priority consideration, but we should always take it into account.

briansmith commented 7 years ago

@camlorn Isn't rustfmt itself an accessibility tool that obviates the need to check the formatting of code before submitting patches for review? That is, you'd just cargo fmt before git add and git commit and not need to worry at all? That is, as far as screen readers are concerned, it seems like the onus should be on the project to ensure that cargo fmt works correctly, and not necessarily on the default styling rules. Otherwise, if we optimized the default rules for screen readers then I think we'd have, for example, one line per statement, no matter how long it is.

ahicks92 commented 7 years ago

@briansmith Well, as far as I know rustfmt doesn't run on the compiler itself.

I actually don't do one-line statements when they get long. The current limit of 100 chars is about right. I'll sometimes break them at what you would consider odd places, but there is a cost to be payed to navigate from the left of a long line. I can't click in the middle like you can, so the navigation algorithm is roughly O(n) on characters. I get move up/down by line, next/previous "word" (but what that is varies depending), next/previous character, and sometimes coding specific stuff that I personally never use. To be concrete, I'd do (and did do in my recent PR) something like this:

really_really_long_function(
    really_big_expression1+really_big_expression2,
);

Or even:

really_really_long_function(
    really_big_expression1
    +really_big_expression2,
);

(by putting the + at the beginning of the line, it is easier to tell that it's continuing the previous one; if this isn't feasible or simply if I remember, I'll indent it a bit more).

But the thing here is, blindness isn't a standard modality. What I do isn't what the next blind person does, and I'd be really surprised if you could find very many "You're blind so you consider xyz to be good code formatting" statements that actually generalize. Lining up with the left paren being hard and not making much sense is one of the few that does, and even then you can probably find someone who used to program with full vision that still bothers with it, even though they're blind now.

briansmith commented 7 years ago

Well, as far as I know rustfmt doesn't run on the compiler itself.

I don't know what you mean here. Are you referring to the compiler's error messages? I guess I was thinking that anytime formatting mattered, one would run cargo fmt or equivalent, like I do when dealing with Go code.

In my message, I was specifically referring to the use case you mentioned where you run into difficulty because code reviewers demand code be formatted a certain visual way, which is hard for you to do. My thinking, based on my own experience working on Go code, is that automatic formatting tools exactly solve that problem. Maybe there are other problems that I am overlooking.

I didn't mean to imply that there's one perfect way to format code optimally for blind people. But, I'd also say that if rustfmt formatted code like the styles you suggested, then I just wouldn't use rustfmt in my projects, because they are very sub-optimal styles for me. To put my earlier statement in another way, what I meant is that certain optimizations for non-sighted people are going to have a negative effect on sighted people, and vice-versa.

retep998 commented 7 years ago

@camlorn I too prefer the style where, when splitting a binary expression across multiple lines, the operator is put at the beginning of the next line. I also indent the continuation lines as well.

As for block indenting functions, which of these two do you prefer? I've been using the former in winapi quite a bit to save vertical space (it really helps when there are literally hundreds of function declarations in a file), but I've been considering switching to the latter.

pub fn CreateFileW(
    lpFileName: LPCWSTR, dwDesiredAccess: DWORD, dwShareMode: DWORD,
    lpSecurityAttributes: LPSECURITY_ATTRIBUTES, dwCreationDisposition: DWORD,
    dwFlagsAndAttributes: DWORD, hTemplateFile: HANDLE,
) -> HANDLE;
pub fn CreateFileW(
    lpFileName: LPCWSTR,
    dwDesiredAccess: DWORD,
    dwShareMode: DWORD,
    lpSecurityAttributes: LPSECURITY_ATTRIBUTES,
    dwCreationDisposition: DWORD,
    dwFlagsAndAttributes: DWORD,
    hTemplateFile: HANDLE,
) -> HANDLE;
nrc commented 7 years ago

Several of @camlorn's points seem to come down to the intended work flow around the default styles (this is somewhat discussed in #4) - how heavily do we want to weight ease of use without Rustfmt? I think if we assume that the programmer will do no manual formatting at all (i.e., always use Rustfmt or some other tool) we will have quite different constraints to if we want a style that can be easily enforced by both a user and a tool. As I wrote on #4, I think we should try and have a style that is easy to write manually, but not at the expense of having a style that is less easily readable or less pretty.

retep998 commented 7 years ago

According to reports from other people, rustfmt can't format the internals of macro invocations, and because winapi is mostly macros, that means I effectively can't use rustfmt for winapi, even if it could achieve the style I desired. So for me, having a style that can be manually done very easily is critically important. Block indentation is incredibly easy to do manually.

joshtriplett commented 7 years ago

On September 26, 2016 5:41:03 PM PDT, Peter Atashian notifications@github.com wrote:

@camlorn I too prefer the style where, when splitting a binary expression across multiple lines, the operator is put at the beginning of the next line. I also indent the continuation lines as well.

As for block indenting functions, which of these two do you prefer? I've been using the former in winapi quite a bit to save vertical space, but I've been considering switching to the latter.

pub fn CreateFileW(
   lpFileName: LPCWSTR, dwDesiredAccess: DWORD, dwShareMode: DWORD,
lpSecurityAttributes: LPSECURITY_ATTRIBUTES, dwCreationDisposition:
DWORD,
   dwFlagsAndAttributes: DWORD, hTemplateFile: HANDLE,
) -> HANDLE;
pub fn CreateFileW(
   lpFileName: LPCWSTR,
   dwDesiredAccess: DWORD,
   dwShareMode: DWORD,
   lpSecurityAttributes: LPSECURITY_ATTRIBUTES,
   dwCreationDisposition: DWORD,
   dwFlagsAndAttributes: DWORD,
   hTemplateFile: HANDLE,
) -> HANDLE;

I vastly prefer the latter, because it doesn't result in rippling changes when adding or removing a parameter.

joshtriplett commented 7 years ago

On September 26, 2016 3:24:53 PM PDT, Brian Smith notifications@github.com wrote:

@camlorn Isn't rustfmt itself an accessibility tool that obviates the need to check the formatting of code before submitting patches for review? That is, you'd just cargo fmt before git add and git commit and not need to worry at all? That is, as far as screen readers are concerned, it seems like the onus should be on the project to ensure that cargo fmt works correctly, and not necessarily on the default styling rules. Otherwise, if we optimized the default rules for screen readers then I think we'd have, for example, one line per statement, no matter how long it is.

Only if you systematically run it on every commit, and always make a separate commit if rustfmt itself changes its style.

nrc commented 7 years ago

Only if you systematically run it on every commit

This seems like a reasonable workflow (we currently do it with make tidy), I would expect to be running Rustfmt on every commit to Rust at some point in the not too distant future and would encourage other projects to run it on their CI.

ahicks92 commented 7 years ago

@briansmith Currently, running rustfmt against the Rust compiler can't be done, or so I was told. In my own projects, I'll do whatever I like and expect people contributing to try to at least be consistent, without putting much weight on it (try doing a code review of formatting as a blind person...no). SO the only place rustfmt is helpful is projects that aren't mine and aren't Rust tools that build before Cargo. How helpful it is in practice, on those projects where It can be used, I don't know. I'm not overly familiar with it. I suppose I could do my own thing but be consistent via rustfmt, if it were sufficiently configurable.

I'm also not sure if I'm speaking about all Rust projects or specifically stuff internal to the official rust-lang organization here. I haven't actually figured out what this repository is for, and only know it exists because I was mentioned here.

@retep998 I don't have an impression. The first makes it easier to get to the body (procedure is search for fn and/or pub fn at beginnig of line, arrow down). The second makes it much less likely that I'd miss a parameter. Given that making sure I don't miss parameters is something I'm already good at, this isn't a huge deal.

The point about rippling changes is a good one, though in this case it's winapi, so they probably never change.

frewsxcv commented 7 years ago

Currently, running rustfmt against the Rust compiler can't be done, or so I was told.

rustfmt gets run on the compiler quite frequently

ahicks92 commented 7 years ago

@frewsxcv Interesting. How automatic is this? How far from make fmt are we?

nrc commented 7 years ago

Interesting. How automatic is this? How far from make fmt are we?

Not at all, and quite a way off. We basically need to format the whole repo before it is practical to have each commit formatted. It is the long term plan though. Needs Rustfmt to be a bit better and to some extent decide on these style guidelines.

ahicks92 commented 7 years ago

@nrc Shame. That would have made my life much easier.

allan-simon commented 7 years ago

I strongly agree that using

pub fn CreateFileW(
    lpFileName: LPCWSTR,
    dwDesiredAccess: DWORD,
    dwShareMode: DWORD,
    lpSecurityAttributes: LPSECURITY_ATTRIBUTES,
    dwCreationDisposition: DWORD,
    dwFlagsAndAttributes: DWORD,
    hTemplateFile: HANDLE,
) -> HANDLE;

or

my_function_call(
    param1,
    param2,
)

has some serious advantage in term of being diff friendly

-  my_function_call(
+ my_new_function_call(
+   new_parameter,

with this you can have upfront a pull request changing the function name , and a separate pull request changing the parameter name , and one removing a parameter without any causing conflict to the others. While indentation that follow the size of the function name, or indentation where you can put several items on the same line would have caused conflict. it's a strategy that really helped my team of 20 programmers to make the process of codereview more enjoyable (easier to know in one glance what the commit modified), and reduce to a minimun the chance that you got a conflict only because of how the code is organized.

last but not least, it makes a very very simple rules on how to indent, if you need multiline

and you can apply it to array initialization, function call, function signature etc. etc.

of course the indentation is a mean to readability and not an end by itself

bruno-medeiros commented 7 years ago

However, visual indentation is (IMO) vastly more aesthetic in most situations, usually easier to read, and uses less vertical space (if adopting the second variation of block indentation)

@nrc , do find code like this easier to read? -> https://github.com/jonathandturner/rls/blob/master/src/test/mod.rs#L94 😧 IMO, it doesn't look easier to read or aesthetic at all, quite the opposite. I think block indentation would be better.

retep998 commented 7 years ago

Visual indentation has a painfully common edge case that makes it look horrible. Block indentation does not have that edge case so it always looks at least acceptable.

nrc commented 7 years ago

@bruno-medeiros There appears to be indentation errors in some lines of that code, but other than that, yes, I find it easier to read.

Just to be clear, here are what I think would be 'ideal' formattings of that code in visual and block style. Note that due to long lines there are (IMO) no good formattings as written and we need to factor out some local variables, I don't think Rustfmt should do that, but I'd be OK, with it choking on the code as written. Block formatting gives more space, so I think Rustfmt could do better on this code in that case. Also note, that I use block indent for method chains in both cases, I don't think visual indent works here for that (though it might in some cases):

    let root_path = format!("{}", serde_json::to_string(&cache.abs_path(Path::new(".")))
        .expect("couldn't convert path to JSON"));
    let url = &Url::from_file_path(cache.abs_path(&source_file_path))
        .expect("couldn't convert file path to URL")
        .as_str()
        .to_owned()
    let text_document = format!("{{\"uri\":{}}}",
                                serde_json::to_string(url).expect("couldn't convert path to JSON"));
    let position = cache.mk_ls_position(src(&source_file_path, 13, "world"));
    let messages = vec![Message::new("initialize",
                                     vec![("processId", "0".to_owned()),
                                          ("rootPath", root_path)]),
                        Message::new("textDocument/definition",
                                     vec![("textDocument", text_document),
                                          ("position", position)])];
    let root_path = format!("{}", serde_json::to_string(&cache.abs_path(Path::new(".")))
        .expect("couldn't convert path to JSON"));
    let url = &Url::from_file_path(cache.abs_path(&source_file_path))
        .expect("couldn't convert file path to URL")
        .as_str()
        .to_owned()
    let text_document = format!("{{\"uri\":{}}}",
        serde_json::to_string(url).expect("couldn't convert path to JSON"));
    let position = cache.mk_ls_position(src(&source_file_path, 13, "world"));
    let messages = vec![
        Message::new("initialize", vec![
            ("processId", "0".to_owned()),
            ("rootPath", root_path)]),
        Message::new("textDocument/definition", vec![
            ("textDocument", text_document),
            ("position", position)])];
nrc commented 7 years ago

Ok, this is a really big topic and I don't think it is practical to nail it down all at once. There are a lot of different cases and a lot of complex edge cases. Although the appeal for total uniformity is attractive, I think that most users it is more desirable to be practical than completely uniform. Thus, I think it is quite reasonable to do different things for structs vs functions and function defs vs function calls, etc.

I think it would be good to keep this general discussion going, but I also think it is worth making specific decisions on a case by case basis.

In general, I prefer visual indentation to block indentation. I believe it is easier to follow with the eye and more attractive. That does not seem to be popular opinion here, but it does seem to be a relatively common opinion - the compiler and libraries generally prefer visual indentation and it is very common in other crates I look at. When we changed method chains from visual to block indentation in Rustfmt (more on this later) there was a lot of negative reaction.

So, starting with simple cases where I (in theory) prefer visual indent:

function defs:

pub fn foo(arg1: Type1,
           arg2: Type2,
           arg3: Type3) {
    ...
}

pub fn foo(arg1: Type1,
    arg2: Type2,
    arg3: Type3) {
    ...
}

pub fn foo(
    arg1: Type1,
    arg2: Type2,
    arg3: Type3
) {
    ...
}

I find the first (visual indent) the better looking - it is easy to follow the arguments because they are vertically aligned and the function body is indented separately from the args making it easy to distinguish.

The second case uses block indenting - I find the vertical mismatch between the first and second arguments irritating and the function body is not distinguished from the arguments.

The third case is an alternative block formatting, it address my issues from the second, but uses more vertical space and I dislike having ( followed by a new line or ) preceded by one (I find this fine for {}, this might be just what I'm used to or might be because of the connection to written English.

I'm skipping formatting with more than one arg per line since I think it is somewhat orthogonal and I don't want to get too much into the details here.

Method chains:

let x = foo.bar()
           .baz()
           .qux();

let x = foo.bar()
    .baz()
    .qux();

let x = foo
    .bar()
    .baz()
    .qux();

Similarly to above, I prefer visual indent because of the vertical alignment. The third option restores vertical alignment to the block indented formatting at the expense of an extra line.

(FTR Rustfmt mosly uses visual indentation by default, basically just because it is what I and Markus (the other main contributor to Rusfmt) preferred, I don't think this should count for much now).

The big problem (as I see it) with visual indent, is the rightward drift it causes. This is especially a problem with closures (or other blocks) in method chains or argument lists. We have tried in Rustfmt to make exceptions for this, but it always causes problems with following elements 'dangling'. This is why we now block indent method chains (although imperfectly).

I have come to the conclusion that you cannot special case large blocks. Either you must block indent everything or visual indent everything. With these constraints, I see three options - block indent everything, use block or indent for a whole chain, but different indents for different chains (or arg lists, etc.), use visual indent everywhere and stomach the rightward drift. The second and third options do not seem reasonable to me. Therefore I think that wherever we might expect blocks or otherwise be sensitive to rightward drift (which I believe is all expressions), then we should use block indent. I would prefer to leave the details of exactly how to more specific RFCs. I still prefer visual indent for function defs, and possible some other non-expression contexts, again I'd prefer to leave these details for other RFCs.

nrc commented 7 years ago

Another advantage of visual indent (raised by @alexcrichton on #37210) is that you can tell at a glance which level of nesting is associated with what, e.g.,

// base case
foo(bar.baz.qux).bar.baz.qux

// visual indent
// first list
foo(bar.baz
       .qux).bar.baz.qux

// second list
foo(bar.baz.qux).bar
                .baz
                .qux

// both lists
foo(bar.baz
       .qux).bar
            .baz
            .qux

// block indent
// first list
foo(bar
    .baz
    .qux).bar.baz.qux

// second list
foo(bar.baz.qux)
    .bar
    .baz
    .qux

// both lists
foo(bar
    .baz
    .qux)
    .bar
    .baz
    .qux
solson commented 7 years ago

The alignment you like in visual indentation distracts my eyes like the "rivers" that typographers do so much work to avoid.

the compiler and libraries generally prefer visual indentation

Er, the code is currently formatted this way, but "prefer" is a loaded word. I know there are some compiler devs who wish this was not the case.

nrc commented 7 years ago

The alignment you like in visual indentation distracts my eyes like the "rivers" that typographers do so much work to avoid.

This is an interesting point, I totally agree that visual indentation has this effect, but for me this draws my eye in a way that allows me to scan the code more quickly. Put another way, typographic rivers are distracting because they do not reflect the semantics of the text, whereas in code, visual indentation is helpful (IMO) because it does reflect the semantics.

briansmith commented 7 years ago

I have come to the conclusion that you cannot special case large blocks. Either you must block indent everything or visual indent everything.

clangformat formats BoringSSL's code such that it uses visual indent by default, but falls back to block indent if (AFAICT) block indent would result in fewer lines output. This is my preferred mechanism for dealing with rightward drift.

solson commented 7 years ago

@nrc Your explanation makes sense, but for some reason it doesn't seem to work for me. I guess people can go either way on that point.

What I find really compelling is what @retep998 mentioned, that visual indentation hits problematic edge cases more often.

nrc commented 7 years ago

@solson

What I find really compelling is what @retep998 mentioned, that visual indentation hits problematic edge cases more often.

Yeah, I agree with that

bruno-medeiros commented 7 years ago

The second case uses block indenting - I find the vertical mismatch between the first and second arguments irritating and the function body is not distinguished from the arguments.

Agreed, it doesn't look good. I think block indenting should be done as in the third case. Is there people advocating for block indenting as done in the second case?

retep998 commented 7 years ago

Agreed, it doesn't look good. I think block indenting should be done as in the third case. Is there people advocating for block indenting as done in the second case?

I definitely prefer the third case with the closing delimeter on a newline, because that is how regular blocks are handled.

// We do this
if foo {
    bar();
    bar();
} else {
    bar();
    bar();
}
// But not this
if foo {
    bar();
    bar(); }
else {
    bar();
    bar(); }
// Nor this
if foo {
    bar();
    bar(); } else {
    bar();
    bar(); }
// So clearly we should do this
fn foo(
    bar: i32,
    bar: i32,
) {
    meow();
    meow();
}
// But not this
fn foo(
    bar: i32,
    bar: i32)
{
    meow();
    meow();}
// Nor this
fn foo(
    bar: i32,
    bar: i32) {
    meow();
    meow();}
gsingh93 commented 7 years ago

I feel like the same points are being reiterated over and over here, and the same examples are getting posted and upvoted. At what point do we make a decision on this? Have any new points been raised recently (I don't think so)?

joshtriplett commented 7 years ago

Are we prepared to discuss and agree on a general principle at the next style team meeting? I think we could, at this point, and use that to inform the various other issues that tie into this.

nrc commented 7 years ago

@gsingh93 new in my mind at least is that we cannot mix block and visual indent like Rustfmt is trying to do, at least in the same expression.

@joshtriplett yes, lets do that.

jwilm commented 7 years ago

Is there a current summary/disposition for this discussion?

fuine commented 7 years ago

Having written a lot of python code i find it confusing to see arguments at the same indent level as the body of the function, even if they are separated by a line with brackets. Even though you can write a definition like so in python:

def foo(
    bar,
    baz
):
    meow()
    meow()

i don't think i have seen such style very often, mostly because indentation is key in python source code. This is the only reason that made me stick to the visual indentation. I'm not sure if there's any easy way to avoid this problem, but i'd like to point it out.

allan-simon commented 7 years ago

@fuine we're writing currently with that exact coding style right now, and the fact

make it actually (at least for us) a non-issue as it's quite easy to know which one is which

alexcrichton commented 7 years ago

Hm I agree with @jwilm, could there be a summary of what this issue is in final-comment-period for? Having read over the thread it sort of sounds like @nrc's previous summary comment is proposing block-indenting across-the board? If so, I would personally like to still make a case for downsides such as found earlier to see if we can solve those problems.

ahicks92 commented 7 years ago

As this is now FCP, I'm just going to drop in and reiterate that my preference is block because of screen reader reasons. I think everything related to this was spelled out above, but I don't want this to get lost in case it turns out that the only meaningful difference between these styles is whether it's friendly to blind people or not.

I will let other people debate the visual merit. One guess as to why.

philipc commented 7 years ago

To address the block indenting downsides found earlier:

I agree that it is confusing to have lines at the same indent level, but different nesting levels. I think we should require extra indents when that occurs:

// both lists
foo(bar
        .baz
        .qux)
    .bar
    .baz
    .qux

I think the problem case from #37210 is much simpler than the above though, and for it I prefer the block indenting to be the following:

build.run(build
    .tool_cmd(&compiler, "linkchecker")
    .arg(build.out.join(host).join("doc")));

This is similar to how I prefer the first argument of functions to be block indented instead of kept on the same line.

joshtriplett commented 7 years ago

I don't think we need to decide, as part of this issue, the exact format of block indentation used. This issue just needs to decide, as a general principle, whether we support visual alignment in some cases, or use block alignment universally. Other issues can determine whether, when block indenting, how many indent levels to use in what circumstances.

(As one example, I personally prefer to indent continuation lines of let statements by two indent levels, because using one level with 4-space indents causes the continuation line to line up with the variable name, which makes it harder to differentiate at a glance. But that seems entirely orthogonal to whether we agree on block or visual indentation.)