rustfmt should avoid rightwards drifting big blocks of code

[nagisa]

Currently rustfmt formats from

            let mut arms = variants.iter().enumerate().map(|(i, &(ident, v_span, ref summary))| {
                let i_expr = cx.expr_usize(v_span, i);
                let pat = cx.pat_lit(v_span, i_expr);

                let path = cx.path(v_span, vec![substr.type_ident, ident]);
                let thing = rand_thing(cx, v_span, path, summary, |cx, sp| rand_call(cx, sp));
                cx.arm(v_span, vec!( pat ), thing)
            }).collect::<Vec<ast::Arm> >();

to

            let mut arms = variants.iter()
                                   .enumerate()
                                   .map(|(i, &(ident, v_span, ref summary))| {
                                       let i_expr = cx.expr_usize(v_span, i);
                                       let pat = cx.pat_lit(v_span, i_expr);

                                       let path = cx.path(v_span, vec![substr.type_ident, ident]);
                                       let thing = rand_thing(cx,
                                                              v_span,
                                                              path,
                                                              summary,
                                                              |cx, sp| rand_call(cx, sp));
                                       cx.arm(v_span, vec!(pat), thing)
                                   })
                                   .collect::<Vec<ast::Arm>>();

which is much worse than the original.

[nrc]

I kind of like the new version - it still fits and I find aligning the methods in the chain more readable.

However, there should be an option for block indenting of methods of a chain (rather than visual indenting) which should avoid the huge chunk of whitespace. I also thought we could format like the first example because there is enough space and the argument to map is a closure - that is new work that just landed. @marcusklaas - do you know what is going on?

[marcusklaas]

I think rustfmt would pick the former strategy if it wasn't for the collect call.

Edit: confirmed

[nrc]

Ah, that's interesting. I wonder if we could we fix that (as an option, as I said I quite like the new version).

[nagisa]

Note that the issue title mentions “big” blocks. I find current style better when the block of code is 1-to-3ish lines of code too, but it gets exponentially worse as the LOC of the block increases.

[nrc]

Hmm, good point. @arichto has also pointed out some formatting choices which make sense for small blocks but not large ones. I guess we're going to have to build in this concept some how, although the idea gives me the fear.

[nrc]

I guess it is just a cut-off at n lines, but it still seems scary some how.

[oli-obk]

how about moving to

            let mut arms = variants
                .iter()
                .enumerate()
                .map(|(i, &(ident, v_span, ref summary))| {
                    let i_expr = cx.expr_usize(v_span, i);
                    let pat = cx.pat_lit(v_span, i_expr);

                    let path = cx.path(v_span, vec![substr.type_ident, ident]);
                    let thing = rand_thing(cx,
                                           v_span,
                                           path,
                                           summary,
                                           |cx, sp| rand_call(cx, sp));
                    cx.arm(v_span, vec!(pat), thing)
                })
                .collect::<Vec<ast::Arm>>();

for large blocks?

[solson]

In the second code block in the original post, there is indentation (the closure body) past an alignment (the . for the method calls). If the method call . isn't on a multiple-of-4 column, then hitting TAB in vim (and I think other editors) will insert less than 4 spaces to reach the next multiple-of-4 column.

For example, in that code sample, hitting tab on the line after let pat = cx.pat_lit(v_span, i_expr); inserts just 1 space. This can make editing in these blocks frustrating.

Plus, the alignment forced rand_thing(cx, v_span, path, summary, |cx, sp| rand_call(cx, sp)); far enough to the right that it had to go vertical. It seems like it's easier for an automated tool to keep from doing anything drastic like that if it sticks to a style more like @nagisa's original. Maybe that should be the default, and the rightwards alignment style should be an optional flag.

[alexcrichton]

I've run across this many many times when working with rustfmt, and I agree with @nagisa's original post that the first block of code is much more readable. We've often had a problem with rightward-drift of code in Rust in the past and we have tried to tailor idioms (like RAII) around preventing that. I personally prefer to have style wherever possible that avoids rightward drift as much as possible.

[killercup]

This came up recently when I ran rustfmt on Miri: rust-lang/miri#6 (comment)