Merge pull request #1821 from topecongiro/avoid-unnecessary-line-breaks

Avoid unnecessary line breaks
This commit is contained in:
Nick Cameron 2017-07-27 11:07:17 +12:00 committed by GitHub
commit bca0dd9a76
11 changed files with 109 additions and 140 deletions

View File

@ -314,8 +314,8 @@ fn left_trim_comment_line<'a>(line: &'a str, style: &CommentStyle) -> &'a str {
&line[opener.trim_right().len()..]
}
} 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..]

View File

@ -334,8 +334,7 @@ pub fn format_expr(
shape,
),
ast::ExprKind::Catch(ref block) => {
if let rewrite @ Some(_) =
rewrite_single_line_block(context, "do catch ", block, shape)
if let rewrite @ Some(_) = rewrite_single_line_block(context, "do catch ", block, shape)
{
return rewrite;
}
@ -371,56 +370,37 @@ where
LHS: Rewrite,
RHS: Rewrite,
{
// Get "full width" rhs and see if it fits on the current line. This
// usually works fairly well since it tends to place operands of
// operations with high precendence close together.
// Note that this is non-conservative, but its just to see if it's even
// worth trying to put everything on one line.
let rhs_shape = try_opt!(shape.sub_width(suffix.len()));
let rhs_orig_result = rhs.rewrite(context, rhs_shape);
let sep = if infix.ends_with(' ') { " " } else { "" };
let infix = infix.trim_right();
let lhs_overhead = shape.used_width() + prefix.len() + infix.len();
let lhs_shape = Shape {
width: try_opt!(context.config.max_width().checked_sub(lhs_overhead)),
..shape
};
let lhs_result = try_opt!(
lhs.rewrite(context, lhs_shape)
.map(|lhs_str| format!("{}{}{}", prefix, lhs_str, infix))
);
// Try to the both lhs and rhs on the same line.
let rhs_orig_result = shape
.offset_left(last_line_width(&lhs_result) + suffix.len() + sep.len())
.and_then(|rhs_shape| rhs.rewrite(context, rhs_shape));
if let Some(ref rhs_result) = rhs_orig_result {
// This is needed in case of line break not caused by a
// shortage of space, but by end-of-line comments, for example.
if !rhs_result.contains('\n') {
let lhs_shape =
try_opt!(try_opt!(shape.offset_left(prefix.len())).sub_width(infix.len()));
let lhs_result = lhs.rewrite(context, lhs_shape);
if let Some(lhs_result) = lhs_result {
let mut result = format!("{}{}{}", prefix, lhs_result, infix);
let remaining_width = shape
.width
.checked_sub(last_line_width(&result) + suffix.len())
.unwrap_or(0);
if rhs_result.len() <= remaining_width {
result.push_str(&rhs_result);
result.push_str(suffix);
return Some(result);
}
// Try rewriting the rhs into the remaining space.
let rhs_shape = shape.offset_left(last_line_width(&result) + suffix.len());
if let Some(rhs_shape) = rhs_shape {
if let Some(rhs_result) = rhs.rewrite(context, rhs_shape) {
// FIXME this should always hold.
if rhs_result.len() <= remaining_width {
result.push_str(&rhs_result);
result.push_str(suffix);
return Some(result);
}
}
}
}
// If the rhs looks like block expression, we allow it to stay on the same line
// with the lhs even if it is multi-lined.
let allow_same_line = rhs_result
.lines()
.next()
.map(|first_line| first_line.ends_with('{'))
.unwrap_or(false);
if !rhs_result.contains('\n') || allow_same_line {
return Some(format!("{}{}{}{}", lhs_result, sep, rhs_result, suffix));
}
}
// We have to use multiple lines.
// Re-evaluate the rhs because we have more space now:
let sep = if infix.ends_with(' ') { " " } else { "" };
let infix = infix.trim_right();
let rhs_shape = match context.config.control_style() {
Style::Legacy => {
try_opt!(shape.sub_width(suffix.len() + prefix.len())).visual_indent(prefix.len())
@ -435,26 +415,6 @@ where
}
};
let rhs_result = try_opt!(rhs.rewrite(context, rhs_shape));
let lhs_overhead = shape.used_width() + prefix.len() + infix.len();
let lhs_shape = Shape {
width: try_opt!(context.config.max_width().checked_sub(lhs_overhead)),
..shape
};
let lhs_result = try_opt!(
lhs.rewrite(context, lhs_shape)
.map(|lhs_str| format!("{}{}{}", prefix, lhs_str, infix))
);
if let Some(ref rhs_str) = rhs_orig_result {
if rhs_str.lines().count() <= rhs_result.lines().count() &&
rhs_str
.lines()
.next()
.map_or(false, |first_line| first_line.ends_with('{')) &&
last_line_width(&lhs_result) + sep.len() + first_line_width(rhs_str) <= shape.width
{
return Some(format!("{}{}{}{}", lhs_result, sep, rhs_str, suffix));
}
}
Some(format!(
"{}\n{}{}{}",
lhs_result,
@ -697,8 +657,8 @@ fn rewrite_closure(
}
// Figure out if the block is necessary.
let needs_block = block.rules != ast::BlockCheckMode::Default ||
block.stmts.len() > 1 || context.inside_macro ||
let needs_block = block.rules != ast::BlockCheckMode::Default || block.stmts.len() > 1 ||
context.inside_macro ||
block_contains_comment(block, context.codemap) ||
prefix.contains('\n');
@ -826,8 +786,7 @@ fn rewrite_empty_block(
block: &ast::Block,
shape: Shape,
) -> Option<String> {
if block.stmts.is_empty() && !block_contains_comment(block, context.codemap) &&
shape.width >= 2
if block.stmts.is_empty() && !block_contains_comment(block, context.codemap) && shape.width >= 2
{
return Some("{}".to_owned());
}
@ -1197,12 +1156,23 @@ impl<'a> ControlFlow<'a> {
shape: Shape,
alt_block_sep: &str,
) -> Option<(String, usize)> {
// Do not take the rhs overhead from the upper expressions into account
// when rewriting pattern.
let new_width = context
.config
.max_width()
.checked_sub(shape.used_width())
.unwrap_or(0);
let fresh_shape = Shape {
width: new_width,
..shape
};
let constr_shape = if self.nested_if {
// We are part of an if-elseif-else chain. Our constraints are tightened.
// 7 = "} else " .len()
try_opt!(shape.offset_left(7))
try_opt!(fresh_shape.offset_left(7))
} else {
shape
fresh_shape
};
let label_string = rewrite_label(self.label);
@ -1211,15 +1181,10 @@ impl<'a> ControlFlow<'a> {
let pat_expr_string = match self.cond {
Some(cond) => {
let mut cond_shape = match context.config.control_style() {
let cond_shape = match context.config.control_style() {
Style::Legacy => try_opt!(constr_shape.shrink_left(offset)),
Style::Rfc => try_opt!(constr_shape.offset_left(offset)),
};
if context.config.control_brace_style() != ControlBraceStyle::AlwaysNextLine {
// 2 = " {".len()
cond_shape = try_opt!(cond_shape.sub_width(2));
}
try_opt!(rewrite_pat_expr(
context,
self.pat,
@ -1233,8 +1198,20 @@ impl<'a> ControlFlow<'a> {
None => String::new(),
};
let brace_overhead =
if context.config.control_brace_style() != ControlBraceStyle::AlwaysNextLine {
// 2 = ` {`
2
} else {
0
};
let one_line_budget = context
.config
.max_width()
.checked_sub(constr_shape.used_width() + offset + brace_overhead)
.unwrap_or(0);
let force_newline_brace = context.config.control_style() == Style::Rfc &&
pat_expr_string.contains('\n') &&
(pat_expr_string.contains('\n') || pat_expr_string.len() > one_line_budget) &&
!last_line_extendable(&pat_expr_string);
// Try to format if-else on single line.
@ -2304,10 +2281,10 @@ fn rewrite_last_closure(
let body_shape = try_opt!(shape.offset_left(extra_offset));
// When overflowing the closure which consists of a single control flow expression,
// force to use block if its condition uses multi line.
if rewrite_cond(context, body, body_shape)
.map(|cond| cond.contains('\n'))
.unwrap_or(false)
{
let is_multi_lined_cond = rewrite_cond(context, body, body_shape)
.map(|cond| cond.contains('\n') || cond.len() > body_shape.width)
.unwrap_or(false);
if is_multi_lined_cond {
return rewrite_closure_with_block(context, body_shape, &prefix, body);
}

View File

@ -482,18 +482,18 @@ fn rewrite_use_list(
};
let list_str = try_opt!(write_list(&items[first_index..], &fmt));
let result =
if list_str.contains('\n') && context.config.imports_indent() == IndentStyle::Block {
format!(
"{}{{\n{}{}\n{}}}",
path_str,
nested_shape.indent.to_string(context.config),
list_str,
shape.indent.to_string(context.config)
)
} else {
format!("{}{{{}}}", path_str, list_str)
};
let result = if list_str.contains('\n') && context.config.imports_indent() == IndentStyle::Block
{
format!(
"{}{{\n{}{}\n{}}}",
path_str,
nested_shape.indent.to_string(context.config),
list_str,
shape.indent.to_string(context.config)
)
} else {
format!("{}{{{}}}", path_str, list_str)
};
Some(result)
}

View File

@ -2734,9 +2734,7 @@ fn format_generics(
let shape = Shape::legacy(context.budget(used_width + offset.width()), offset);
let mut result = try_opt!(rewrite_generics(context, generics, shape, span));
let same_line_brace = if !generics.where_clause.predicates.is_empty() ||
result.contains('\n')
{
let same_line_brace = if !generics.where_clause.predicates.is_empty() || result.contains('\n') {
let budget = context
.config
.max_width()

View File

@ -291,17 +291,17 @@ fn rewrite_tuple_pat(
}
let wildcard_suffix_len = count_wildcard_suffix_len(context, &pat_vec, span, shape);
let (pat_vec, span) =
if context.config.condense_wildcard_suffixes() && wildcard_suffix_len >= 2 {
let new_item_count = 1 + pat_vec.len() - wildcard_suffix_len;
let sp = pat_vec[new_item_count - 1].span();
let snippet = context.snippet(sp);
let lo = sp.lo + BytePos(snippet.find_uncommented("_").unwrap() as u32);
pat_vec[new_item_count - 1] = TuplePatField::Dotdot(mk_sp(lo, lo + BytePos(1)));
(&pat_vec[..new_item_count], mk_sp(span.lo, lo + BytePos(1)))
} else {
(&pat_vec[..], span)
};
let (pat_vec, span) = if context.config.condense_wildcard_suffixes() && wildcard_suffix_len >= 2
{
let new_item_count = 1 + pat_vec.len() - wildcard_suffix_len;
let sp = pat_vec[new_item_count - 1].span();
let snippet = context.snippet(sp);
let lo = sp.lo + BytePos(snippet.find_uncommented("_").unwrap() as u32);
pat_vec[new_item_count - 1] = TuplePatField::Dotdot(mk_sp(lo, lo + BytePos(1)));
(&pat_vec[..new_item_count], mk_sp(span.lo, lo + BytePos(1)))
} else {
(&pat_vec[..], span)
};
// add comma if `(x,)`
let add_comma = path_str.is_none() && pat_vec.len() == 1 && dotdot_pos.is_none();

View File

@ -44,12 +44,11 @@ pub fn rewrite_path(
) -> Option<String> {
let skip_count = qself.map_or(0, |x| x.position);
let mut result =
if path.is_global() && qself.is_none() && path_context != PathContext::Import {
"::".to_owned()
} else {
String::new()
};
let mut result = if path.is_global() && qself.is_none() && path_context != PathContext::Import {
"::".to_owned()
} else {
String::new()
};
let mut span_lo = path.span.lo;
@ -633,13 +632,13 @@ impl Rewrite for ast::PolyTraitRef {
Shape::legacy(max_path_width, shape.indent + extra_offset),
));
Some(if context.config.spaces_within_angle_brackets() &&
lifetime_str.len() > 0
{
format!("for< {} > {}", lifetime_str, path_str)
} else {
format!("for<{}> {}", lifetime_str, path_str)
})
Some(
if context.config.spaces_within_angle_brackets() && lifetime_str.len() > 0 {
format!("for< {} > {}", lifetime_str, path_str)
} else {
format!("for<{}> {}", lifetime_str, path_str)
},
)
} else {
self.trait_ref.rewrite(context, shape)
}

View File

@ -328,8 +328,7 @@ impl<'a> FmtVisitor<'a> {
}
ast::ItemKind::Trait(..) => {
self.format_missing_with_indent(item.span.lo);
if let Some(trait_str) =
format_trait(&self.get_context(), item, self.block_indent)
if let Some(trait_str) = format_trait(&self.get_context(), item, self.block_indent)
{
self.buffer.push_str(&trait_str);
self.last_pos = source!(self, item.span).hi;

View File

@ -119,10 +119,10 @@ fn floaters() {
.quux();
a + match x {
true => "yay!",
false => "boo!",
}
.bar()
true => "yay!",
false => "boo!",
}
.bar()
}
fn is_replaced_content() -> bool {

View File

@ -12,8 +12,7 @@ fn main() {
foo
}
if ai_timer.elapsed_time().as_microseconds() > ai_time.as_microseconds() {
if ball.position().y + ball_radius >
right_paddle.position().y + paddle_size.y / 2.
if ball.position().y + ball_radius > right_paddle.position().y + paddle_size.y / 2.
{
foo
}

View File

@ -346,12 +346,10 @@ fn complex_if_else() {
ha();
} else if xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + xxxxxxxx {
yo();
} else if let Some(x) =
xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
} else if let Some(x) = xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
{
ha();
} else if xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +
xxxxxxxxx
} else if xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx + xxxxxxxxx
{
yo();
}

View File

@ -12,9 +12,9 @@ fn main() {
// Just comments
}
'a: while loooooooooooooooooooooooooooooooooong_variable_name + another_value >
some_other_value
{}
'a: while loooooooooooooooooooooooooooooooooong_variable_name + another_value > some_other_value
{
}
while aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb {
}
@ -22,8 +22,7 @@ fn main() {
while aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa {
}
'b: for xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx in
some_iter(arg1, arg2)
'b: for xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx in some_iter(arg1, arg2)
{
// do smth
}