From 199d40fa5589bb0e74c4086e30825015277439c1 Mon Sep 17 00:00:00 2001 From: Marcus Klaas <mail@marcusklaas.nl> Date: Tue, 6 Oct 2015 22:13:14 +0200 Subject: [PATCH] Increase default function call width limit --- Cargo.lock | 16 +++++++-------- src/comment.rs | 3 +-- src/config.rs | 2 +- src/expr.rs | 54 +++++++++++++------------------------------------- src/filemap.rs | 3 +-- src/imports.rs | 8 ++++---- src/issues.rs | 7 ++----- src/items.rs | 4 +--- src/lib.rs | 1 - src/lists.rs | 17 +++++++--------- src/macros.rs | 7 +------ src/types.rs | 15 +++++++------- src/visitor.rs | 8 ++------ 13 files changed, 49 insertions(+), 96 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b0f1f478df3..3648e5e5f67 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2,11 +2,11 @@ name = "rustfmt" version = "0.0.1" dependencies = [ - "diff 0.1.5 (git+https://github.com/utkarshkukreti/diff.rs.git)", + "diff 0.1.7 (git+https://github.com/utkarshkukreti/diff.rs.git)", "regex 0.1.41 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.16 (registry+https://github.com/rust-lang/crates.io-index)", "strings 0.0.1 (git+https://github.com/nrc/strings.rs.git)", - "term 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)", + "term 0.2.12 (registry+https://github.com/rust-lang/crates.io-index)", "toml 0.1.22 (registry+https://github.com/rust-lang/crates.io-index)", "unicode-segmentation 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -21,15 +21,15 @@ dependencies = [ [[package]] name = "diff" -version = "0.1.5" -source = "git+https://github.com/utkarshkukreti/diff.rs.git#1921576a73e1b50a0ecb26c8ce62eefb26d273b4" +version = "0.1.7" +source = "git+https://github.com/utkarshkukreti/diff.rs.git#6edb9454bf4127087aced0fe07ab3ea6894083cb" [[package]] name = "kernel32-sys" version = "0.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ - "winapi 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)", "winapi-build 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)", ] @@ -84,11 +84,11 @@ dependencies = [ [[package]] name = "term" -version = "0.2.11" +version = "0.2.12" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "kernel32-sys 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", - "winapi 0.2.3 (registry+https://github.com/rust-lang/crates.io-index)", + "winapi 0.2.4 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] @@ -106,7 +106,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "winapi" -version = "0.2.3" +version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] diff --git a/src/comment.rs b/src/comment.rs index b08e46681a7..86d1a9aeb14 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -348,8 +348,7 @@ mod test { assert_eq!(&uncommented("abc/*...*/"), "abc"); assert_eq!(&uncommented("// .... /* \n../* /* *** / */ */a/* // */c\n"), "..ac\n"); - assert_eq!(&uncommented("abc \" /* */\" qsdf"), - "abc \" /* */\" qsdf"); + assert_eq!(&uncommented("abc \" /* */\" qsdf"), "abc \" /* */\" qsdf"); } #[test] diff --git a/src/config.rs b/src/config.rs index 1688b4b363e..b237ce4ddfc 100644 --- a/src/config.rs +++ b/src/config.rs @@ -260,7 +260,7 @@ create_config! { max_width: usize, 100, "Maximum width of each line"; ideal_width: usize, 80, "Ideal width of each line"; tab_spaces: usize, 4, "Number of spaces per tab"; - fn_call_width: usize, 55, + fn_call_width: usize, 60, "Maximum width of the args of a function call before faling back to vertical formatting"; struct_lit_width: usize, 16, "Maximum width in the body of a struct lit before faling back to vertical formatting"; diff --git a/src/expr.rs b/src/expr.rs index 53e24b70984..ac966dc5118 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -14,7 +14,7 @@ use std::borrow::Borrow; use Indent; use rewrite::{Rewrite, RewriteContext}; use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic, - DefinitiveListTactic, definitive_tactic}; + DefinitiveListTactic, definitive_tactic, ListItem, format_fn_args}; use string::{StringFormat, rewrite_string}; use utils::{span_after, extra_offset, last_line_width, wrap_str, binary_search}; use visitor::FmtVisitor; @@ -148,13 +148,7 @@ impl Rewrite for ast::Expr { Some(format!("break{}", id_str)) } ast::Expr_::ExprClosure(capture, ref fn_decl, ref body) => { - rewrite_closure(capture, - fn_decl, - body, - self.span, - context, - width, - offset) + rewrite_closure(capture, fn_decl, body, self.span, context, width, offset) } ast::Expr_::ExprField(..) | ast::Expr_::ExprTupField(..) | @@ -172,10 +166,7 @@ impl Rewrite for ast::Expr { }) } ast::Expr_::ExprRet(None) => { - wrap_str("return".to_owned(), - context.config.max_width, - width, - offset) + wrap_str("return".to_owned(), context.config.max_width, width, offset) } ast::Expr_::ExprRet(Some(ref expr)) => { rewrite_unary_prefix(context, "return ", expr, width, offset) @@ -227,7 +218,8 @@ pub fn rewrite_array<'a, I>(expr_iter: I, .map(|li| li.item.as_ref().map(|s| s.len() > 10)) .fold(Some(false), |acc, x| acc.and_then(|y| x.map(|x| (x || y))))); - let tactic = if has_long_item || items.iter().any(|li| li.is_multiline()) { + + let tactic = if has_long_item || items.iter().any(ListItem::is_multiline) { definitive_tactic(&items, ListTactic::HorizontalVertical, max_item_width) } else { DefinitiveListTactic::Mixed @@ -282,9 +274,7 @@ fn rewrite_closure(capture: ast::CaptureClause, span_after(span, "|", context.codemap), body.span.lo); let item_vec = arg_items.collect::<Vec<_>>(); - let tactic = definitive_tactic(&item_vec, - ListTactic::HorizontalVertical, - horizontal_budget); + let tactic = definitive_tactic(&item_vec, ListTactic::HorizontalVertical, horizontal_budget); let budget = match tactic { DefinitiveListTactic::Horizontal => horizontal_budget, _ => budget, @@ -589,11 +579,7 @@ fn rewrite_if_else(context: &RewriteContext, // Try to format if-else on single line. if allow_single_line && context.config.single_line_if_else { - let trial = single_line_if_else(context, - &pat_expr_string, - if_block, - else_block_opt, - width); + let trial = single_line_if_else(context, &pat_expr_string, if_block, else_block_opt, width); if trial.is_some() { return trial; @@ -780,8 +766,7 @@ fn rewrite_match(context: &RewriteContext, } } // BytePos(1) = closing match brace. - let last_span = mk_sp(arm_end_pos(&arms[arms.len() - 1]), - span.hi - BytePos(1)); + let last_span = mk_sp(arm_end_pos(&arms[arms.len() - 1]), span.hi - BytePos(1)); let last_comment = context.snippet(last_span); let comment = try_opt!(rewrite_match_arm_comment(context, &last_comment, @@ -894,8 +879,7 @@ impl Rewrite for ast::Arm { // 4 = ` => `.len() let same_line_body = if context.config.max_width > line_start + comma.len() + 4 { let budget = context.config.max_width - line_start - comma.len() - 4; - let offset = Indent::new(offset.block_indent, - line_start + 4 - offset.block_indent); + let offset = Indent::new(offset.block_indent, line_start + 4 - offset.block_indent); let rewrite = nop_block_collapse(body.rewrite(context, budget, offset), budget); match rewrite { @@ -1081,13 +1065,7 @@ pub fn rewrite_call<R>(context: &RewriteContext, where R: Rewrite { let closure = |callee_max_width| { - rewrite_call_inner(context, - callee, - callee_max_width, - args, - span, - width, - offset) + rewrite_call_inner(context, callee, callee_max_width, args, span, width, offset) }; // 2 is for parens @@ -1145,7 +1123,7 @@ fn rewrite_call_inner<R>(context: &RewriteContext, span.lo, span.hi); - let list_str = match ::lists::format_fn_args(items, remaining_width, offset, context.config) { + let list_str = match format_fn_args(items, remaining_width, offset, context.config) { Some(str) => str, None => return Err(Ordering::Less), }; @@ -1174,9 +1152,7 @@ fn rewrite_struct_lit<'a>(context: &RewriteContext, width: usize, offset: Indent) -> Option<String> { - debug!("rewrite_struct_lit: width {}, offset {:?}", - width, - offset); + debug!("rewrite_struct_lit: width {}, offset {:?}", width, offset); assert!(!fields.is_empty() || base.is_some()); enum StructLitField<'a> { @@ -1327,9 +1303,7 @@ fn rewrite_tuple_lit(context: &RewriteContext, width: usize, offset: Indent) -> Option<String> { - debug!("rewrite_tuple_lit: width: {}, offset: {:?}", - width, - offset); + debug!("rewrite_tuple_lit: width: {}, offset: {:?}", width, offset); let indent = offset + 1; // In case of length 1, need a trailing comma if items.len() == 1 { @@ -1350,7 +1324,7 @@ fn rewrite_tuple_lit(context: &RewriteContext, span.lo + BytePos(1), // Remove parens span.hi - BytePos(1)); let budget = try_opt!(width.checked_sub(2)); - let list_str = try_opt!(::lists::format_fn_args(items, budget, indent, context.config)); + let list_str = try_opt!(format_fn_args(items, budget, indent, context.config)); Some(format!("({})", list_str)) } diff --git a/src/filemap.rs b/src/filemap.rs index f3f952bd00c..d94c1501734 100644 --- a/src/filemap.rs +++ b/src/filemap.rs @@ -115,8 +115,7 @@ fn write_file(text: &StringBuffer, try!(write_system_newlines(&mut v, text, config)); let fmt_text = String::from_utf8(v).unwrap(); let diff = make_diff(&ori_text, &fmt_text, 3); - print_diff(diff, - |line_num| format!("\nDiff at line {}:", line_num)); + print_diff(diff, |line_num| format!("\nDiff at line {}:", line_num)); } WriteMode::Return => { // io::Write is not implemented for String, working around with diff --git a/src/imports.rs b/src/imports.rs index 2c518378b36..aab5dd76799 100644 --- a/src/imports.rs +++ b/src/imports.rs @@ -9,7 +9,7 @@ // except according to those terms. use Indent; -use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic}; +use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, definitive_tactic}; use utils::span_after; use rewrite::{Rewrite, RewriteContext}; @@ -153,9 +153,9 @@ pub fn rewrite_use_list(width: usize, items[1..].sort_by(|a, b| a.item.cmp(&b.item)); } - let tactic = ::lists::definitive_tactic(&items[first_index..], - ::lists::ListTactic::Mixed, - remaining_width); + let tactic = definitive_tactic(&items[first_index..], + ::lists::ListTactic::Mixed, + remaining_width); let fmt = ListFormatting { tactic: tactic, separator: ",", diff --git a/src/issues.rs b/src/issues.rs index 6f64436bb52..04a61fff358 100644 --- a/src/issues.rs +++ b/src/issues.rs @@ -236,8 +236,7 @@ fn find_unnumbered_issue() { fn check_pass(text: &str) { let mut seeker = BadIssueSeeker::new(ReportTactic::Unnumbered, ReportTactic::Unnumbered); - assert_eq!(None, - text.chars().position(|c| seeker.inspect(c).is_some())); + assert_eq!(None, text.chars().position(|c| seeker.inspect(c).is_some())); } check_fail("TODO\n", 4); @@ -272,9 +271,7 @@ fn find_issue() { ReportTactic::Never, ReportTactic::Always)); - assert!(!is_bad_issue("bad FIXME\n", - ReportTactic::Always, - ReportTactic::Never)); + assert!(!is_bad_issue("bad FIXME\n", ReportTactic::Always, ReportTactic::Never)); } #[test] diff --git a/src/items.rs b/src/items.rs index e2f59f0fb6a..9cc228ae2c8 100644 --- a/src/items.rs +++ b/src/items.rs @@ -312,9 +312,7 @@ impl<'a> FmtVisitor<'a> { let context = self.get_context(); let ret_str = fd.output - .rewrite(&context, - self.config.max_width - indent.width(), - indent) + .rewrite(&context, self.config.max_width - indent.width(), indent) .unwrap(); // Args. diff --git a/src/lib.rs b/src/lib.rs index 723c603ea3f..1dfb05c88b6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,7 +19,6 @@ // keeping some scratch mem for this and running our own StrPool? // TODO for lint violations of names, emit a refactor script - #[macro_use] extern crate log; diff --git a/src/lists.rs b/src/lists.rs index 514acd13f53..e7565851871 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -19,12 +19,15 @@ use comment::{FindUncommented, rewrite_comment, find_comment_end}; use config::Config; #[derive(Eq, PartialEq, Debug, Copy, Clone)] +/// Formatting tactic for lists. This will be cast down to a +/// DefinitiveListTactic depending on the number and length of the items and +/// their comments. pub enum ListTactic { // One item per row. Vertical, // All items on one row. Horizontal, - // Try Horizontal layout, if that fails then vertical + // Try Horizontal layout, if that fails then vertical. HorizontalVertical, // HorizontalVertical with a soft limit of n characters. LimitedHorizontalVertical(usize), @@ -72,11 +75,7 @@ pub fn format_item_list<I>(items: I, -> Option<String> where I: Iterator<Item = ListItem> { - list_helper(items, - width, - offset, - config, - ListTactic::HorizontalVertical) + list_helper(items, width, offset, config, ListTactic::HorizontalVertical) } fn list_helper<I>(items: I, @@ -111,7 +110,7 @@ impl AsRef<ListItem> for ListItem { pub struct ListItem { // None for comments mean that they are not present. pub pre_comment: Option<String>, - // Item should include attributes and doc comments. None indicates failed + // Item should include attributes and doc comments. None indicates a failed // rewrite. pub item: Option<String>, pub post_comment: Option<String>, @@ -121,7 +120,6 @@ pub struct ListItem { impl ListItem { pub fn is_multiline(&self) -> bool { - // FIXME: fail earlier! self.item.as_ref().map(|s| s.contains('\n')).unwrap_or(false) || self.pre_comment.is_some() || self.post_comment.as_ref().map(|s| s.contains('\n')).unwrap_or(false) @@ -142,6 +140,7 @@ impl ListItem { } #[derive(Eq, PartialEq, Debug, Copy, Clone)] +/// The definitive formatting tactic for lists. pub enum DefinitiveListTactic { Vertical, Horizontal, @@ -488,8 +487,6 @@ fn calculate_width<'li, I, T>(items: I) -> (usize, usize) } fn total_item_width(item: &ListItem) -> usize { - // FIXME: If the item has a `None` item, it may be better to fail earlier - // rather than later. comment_len(item.pre_comment.as_ref().map(|x| &(*x)[..])) + comment_len(item.post_comment.as_ref().map(|x| &(*x)[..])) + item.item.as_ref().map(|str| str.len()).unwrap_or(0) diff --git a/src/macros.rs b/src/macros.rs index 51fdd48a2ba..163a9f12f24 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -89,12 +89,7 @@ pub fn rewrite_macro(mac: &ast::Mac, match style { MacroStyle::Parens => { // Format macro invocation as function call. - rewrite_call(context, - ¯o_name, - &expr_vec, - mac.span, - width, - offset) + rewrite_call(context, ¯o_name, &expr_vec, mac.span, width, offset) } MacroStyle::Brackets => { // Format macro invocation as array literal. diff --git a/src/types.rs b/src/types.rs index 6f6afbfc42e..2cbbed92fa5 100644 --- a/src/types.rs +++ b/src/types.rs @@ -13,7 +13,7 @@ use syntax::print::pprust; use syntax::codemap::{self, Span, BytePos, CodeMap}; use Indent; -use lists::itemize_list; +use lists::{format_item_list, itemize_list, format_fn_args}; use rewrite::{Rewrite, RewriteContext}; use utils::{extra_offset, span_after, format_mutability, wrap_str}; @@ -226,10 +226,10 @@ fn rewrite_segment(segment: &ast::PathSegment, }, list_lo, span_hi); - let list_str = try_opt!(::lists::format_item_list(items, - list_width, - offset + extra_offset, - context.config)); + let list_str = try_opt!(format_item_list(items, + list_width, + offset + extra_offset, + context.config)); // Update position of last bracket. *span_lo = next_span_lo; @@ -258,7 +258,7 @@ fn rewrite_segment(segment: &ast::PathSegment, |ty| ty.rewrite(context, budget, offset), list_lo, span_hi); - let list_str = try_opt!(::lists::format_fn_args(items, budget, offset, context.config)); + let list_str = try_opt!(format_fn_args(items, budget, offset, context.config)); format!("({}){}", list_str, output) } @@ -363,8 +363,7 @@ impl Rewrite for ast::TyParamBound { } ast::TyParamBound::TraitTyParamBound(ref tref, ast::TraitBoundModifier::Maybe) => { let budget = try_opt!(width.checked_sub(1)); - Some(format!("?{}", - try_opt!(tref.rewrite(context, budget, offset + 1)))) + Some(format!("?{}", try_opt!(tref.rewrite(context, budget, offset + 1)))) } ast::TyParamBound::RegionTyParamBound(ref l) => { Some(pprust::lifetime_to_string(l)) diff --git a/src/visitor.rs b/src/visitor.rs index 883d46a22d4..b02e8a72b8f 100644 --- a/src/visitor.rs +++ b/src/visitor.rs @@ -319,9 +319,7 @@ impl<'a> FmtVisitor<'a> { } fn format_mod(&mut self, m: &ast::Mod, s: Span, ident: ast::Ident) { - debug!("FmtVisitor::format_mod: ident: {:?}, span: {:?}", - ident, - s); + debug!("FmtVisitor::format_mod: ident: {:?}, span: {:?}", ident, s); // Decide whether this is an inline mod or an external mod. let local_file_name = self.codemap.span_to_filename(s); @@ -359,9 +357,7 @@ impl<'a> FmtVisitor<'a> { overflow_indent: Indent::empty(), }; // 1 = ";" - match vp.rewrite(&context, - self.config.max_width - offset.width() - 1, - offset) { + match vp.rewrite(&context, self.config.max_width - offset.width() - 1, offset) { Some(ref s) if s.is_empty() => { // Format up to last newline let prev_span = codemap::mk_sp(self.last_pos, span.lo);