From ad47a7101203dbeda96b75487a104ff88b499e71 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Wed, 18 Oct 2017 21:55:45 +0900 Subject: [PATCH] Do not distinguish between a single-child chain from others `last_shape` is used when rewriting the last child on its own line. --- src/chains.rs | 57 +++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/src/chains.rs b/src/chains.rs index 43dfd83cc57..6743f605cf6 100644 --- a/src/chains.rs +++ b/src/chains.rs @@ -166,25 +166,64 @@ pub fn rewrite_chain(expr: &ast::Expr, context: &RewriteContext, shape: Shape) - let all_in_one_line = !parent_rewrite_contains_newline && rewrites.iter().all(|s| !s.contains('\n')) && almost_total < one_line_budget; - let last_shape = if rewrites.is_empty() { - // We only have a single child. - first_child_shape - } else { - match context.config.chain_indent() { - IndentStyle::Visual => other_child_shape.sub_width(shape.rhs_overhead(context.config))?, - IndentStyle::Block => other_child_shape, - } + let last_shape = match context.config.chain_indent() { + IndentStyle::Visual => other_child_shape.sub_width(shape.rhs_overhead(context.config))?, + IndentStyle::Block => other_child_shape, }; let last_shape = last_shape.sub_width(suffix_try_num)?; + + // Rewrite the last child. The last child of a chain requires special treatment. We need to + // know whether 'overflowing' the last child make a better formatting: + // + // A chain with overflowing the last child: + // ``` + // parent.child1.child2.last_child( + // a, + // b, + // c, + // ) + // ``` + // + // A chain without overflowing the last child (in vertical layout): + // ``` + // parent + // .child1 + // .child2 + // .last_child(a, b, c) + // ``` + // + // In particular, overflowing is effective when the last child is a method with a multi-lined + // block-like argument (e.g. closure): + // ``` + // parent.child1.chlid2.last_child(|a, b, c| { + // let x = foo(a, b, c); + // let y = bar(a, b, c); + // + // // ... + // + // result + // }) + // ``` + + // `rewrite_last` rewrites the last child on its own line. We use a closure here instead of + // directly calling `rewrite_chain_subexpr()` to avoid exponential blowup. let rewrite_last = || rewrite_chain_subexpr(last_subexpr, total_span, context, last_shape); let (last_subexpr_str, fits_single_line) = if all_in_one_line || extend_last_subexr { + // First we try to 'overflow' the last child and see if it looks better than using + // vertical layout. parent_shape.offset_left(almost_total).map(|shape| { if let Some(rw) = rewrite_chain_subexpr(last_subexpr, total_span, context, shape) { + // We allow overflowing here only if both of the following conditions match: + // 1. The entire chain fits in a single line expect the last child. + // 2. `last_chlid_str.lines().count() >= 5`. let line_count = rw.lines().count(); let fits_single_line = almost_total + first_line_width(&rw) <= one_line_budget; - if fits_single_line && (line_count >= 5 || extend_last_subexr) { + if fits_single_line && line_count >= 5 { (Some(rw), true) } else { + // We could not know whether overflowing is better than using vertical layout, + // just by looking at the overflowed rewrite. Now we rewrite the last child + // on its own line, and compare two rewrites to choose which is better. match rewrite_last() { Some(ref new_rw) if !fits_single_line => (Some(new_rw.clone()), false), Some(ref new_rw) if new_rw.lines().count() >= line_count => { -- 2.44.0