From b3488c618638c6e9288cbe9b2b34ca1109a06a1d Mon Sep 17 00:00:00 2001 From: Marcus Klaas Date: Fri, 3 Jun 2016 23:18:19 +0200 Subject: [PATCH] Fix constraints on pattern formatting of else arms --- src/comment.rs | 44 +++++++++++++++++++++++--------------------- src/expr.rs | 14 ++++++++++++-- tests/source/expr.rs | 13 +++++++++++++ tests/target/expr.rs | 15 +++++++++++++++ 4 files changed, 63 insertions(+), 23 deletions(-) diff --git a/src/comment.rs b/src/comment.rs index e417e29936e..66cc5e2fb6f 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -29,29 +29,30 @@ pub fn rewrite_comment(orig: &str, let s = orig.trim(); // Edge case: block comments. Let's not trim their lines (for now). - let (opener, closer, line_start) = if block_style { - ("/* ", " */", " * ") - } else if !config.normalize_comments { - if orig.starts_with("/**") { - ("/** ", " **/", " ** ") - } else if orig.starts_with("/*!") { - ("/*! ", " */", " * ") - } else if orig.starts_with("/*") { + let (opener, closer, line_start) = + if block_style { ("/* ", " */", " * ") - } else if orig.starts_with("///") { + } else if !config.normalize_comments { + if orig.starts_with("/**") { + ("/** ", " **/", " ** ") + } else if orig.starts_with("/*!") { + ("/*! ", " */", " * ") + } else if orig.starts_with("/*") { + ("/* ", " */", " * ") + } else if orig.starts_with("///") { + ("/// ", "", "/// ") + } else if orig.starts_with("//!") { + ("//! ", "", "//! ") + } else { + ("// ", "", "// ") + } + } else if orig.starts_with("///") || orig.starts_with("/**") { ("/// ", "", "/// ") - } else if orig.starts_with("//!") { + } else if orig.starts_with("//!") || orig.starts_with("/*!") { ("//! ", "", "//! ") } else { ("// ", "", "// ") - } - } else if orig.starts_with("///") || orig.starts_with("/**") { - ("/// ", "", "/// ") - } else if orig.starts_with("//!") || orig.starts_with("/*!") { - ("//! ", "", "//! ") - } else { - ("// ", "", "// ") - }; + }; let max_chars = width.checked_sub(closer.len() + opener.len()).unwrap_or(1); @@ -127,11 +128,12 @@ fn left_trim_comment_line(line: &str) -> &str { line.starts_with("/** ") { &line[4..] } else if line.starts_with("/* ") || line.starts_with("// ") || line.starts_with("//!") || - line.starts_with("///") || line.starts_with("** ") || line.starts_with("/*!") || - line.starts_with("/**") { + line.starts_with("///") || + line.starts_with("** ") || line.starts_with("/*!") || + line.starts_with("/**") { &line[3..] } else if line.starts_with("/*") || line.starts_with("* ") || line.starts_with("//") || - line.starts_with("**") { + line.starts_with("**") { &line[2..] } else if line.starts_with("*") { &line[1..] diff --git a/src/expr.rs b/src/expr.rs index af0492b2913..509f790842f 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -725,6 +725,14 @@ fn rewrite_if_else(context: &RewriteContext, offset: Indent, allow_single_line: bool) -> Option { + let (budget, indent) = if !allow_single_line { + // We are part of an if-elseif-else chain. Our constraints are tightened. + // 7 = "} else" .len() + (try_opt!(width.checked_sub(7)), offset + 7) + } else { + (width, offset) + }; + // 3 = "if ", 2 = " {" let pat_penalty = match context.config.else_if_brace_style { ElseIfBraceStyle::AlwaysNextLine => 3, @@ -735,8 +743,8 @@ fn rewrite_if_else(context: &RewriteContext, cond, "let ", " =", - try_opt!(width.checked_sub(pat_penalty)), - offset + 3)); + try_opt!(budget.checked_sub(pat_penalty)), + indent + 3)); // Try to format if-else on single line. if expr_type == ExprType::SubExpression && allow_single_line && @@ -778,6 +786,8 @@ fn rewrite_if_else(context: &RewriteContext, let rewrite = match else_block.node { // If the else expression is another if-else expression, prevent it // from being formatted on a single line. + // Note how we're passing the original width and offset, as the + // cost of "else" should not cascade. ast::ExprKind::IfLet(ref pat, ref cond, ref if_block, ref next_else_block) => { rewrite_if_else(context, cond, diff --git a/tests/source/expr.rs b/tests/source/expr.rs index ee24e742641..cb3c59b10ce 100644 --- a/tests/source/expr.rs +++ b/tests/source/expr.rs @@ -272,3 +272,16 @@ fn if_else() { -1.0 }; } + +fn complex_if_else() { + if let Some(x) = xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx { + } else if let Some(x) = xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx { + ha(); + } else if xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + xxxxxxxx { + yo(); + } else if let Some(x) = xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx { + ha(); + } else if xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + xxxxxxxxx { + yo(); + } +} diff --git a/tests/target/expr.rs b/tests/target/expr.rs index 87a45a98d49..12ac778be69 100644 --- a/tests/target/expr.rs +++ b/tests/target/expr.rs @@ -275,3 +275,18 @@ fn if_else() { let cx = tp1.x + any * radius * if anticlockwise { 1.0 } else { -1.0 }; } + +fn complex_if_else() { + if let Some(x) = xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx { + } else if let Some(x) = xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx { + ha(); + } else if xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + xxxxxxxx { + yo(); + } else if let Some(x) = + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx { + ha(); + } else if xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + + xxxxxxxxx { + yo(); + } +}