From 2dd8d6d097b52a34991cc032f94be4f03583d0e1 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Wed, 5 Jul 2017 23:12:58 +0900 Subject: [PATCH 1/6] Add Clone trait bound to write_list --- src/expr.rs | 2 +- src/items.rs | 4 ++-- src/lists.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/expr.rs b/src/expr.rs index 668134176319..1326d3c41b13 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -1707,7 +1707,7 @@ fn rewrite(&self, context: &RewriteContext, shape: Shape) -> Option { ends_with_newline: false, config: context.config, }; - let pats_str = try_opt!(write_list(items, &fmt)); + let pats_str = try_opt!(write_list(&items, &fmt)); let guard_shape = if pats_str.contains('\n') { shape.with_max_width(context.config) diff --git a/src/items.rs b/src/items.rs index 4ae7bcc609ac..91bba5349249 100644 --- a/src/items.rs +++ b/src/items.rs @@ -478,7 +478,7 @@ fn format_variant_list( config: self.config, }; - let list = try_opt!(write_list(items, &fmt)); + let list = try_opt!(write_list(&items.collect::>(), &fmt)); result.push_str(&list); result.push('\n'); Some(result) @@ -2539,7 +2539,7 @@ fn rewrite_where_clause_rfc_style( ends_with_newline: true, config: context.config, }; - let preds_str = try_opt!(write_list(items, &fmt)); + let preds_str = try_opt!(write_list(&items.collect::>(), &fmt)); Some(format!( "{}where\n{}{}", diff --git a/src/lists.rs b/src/lists.rs index bd4c6891c1b5..de82f2cd6243 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -157,7 +157,7 @@ pub fn definitive_tactic(items: I, tactic: ListTactic, width: usize) -> De // TODO: add unit tests pub fn write_list(items: I, formatting: &ListFormatting) -> Option where - I: IntoIterator, + I: IntoIterator + Clone, T: AsRef, { let tactic = formatting.tactic; From 1de786a79a4c7eb6b0a19f02271cb056e22c492b Mon Sep 17 00:00:00 2001 From: topecongiro Date: Thu, 6 Jul 2017 01:02:37 +0900 Subject: [PATCH 2/6] Implement vertical alignment for comments after list structure --- src/lists.rs | 140 +++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 118 insertions(+), 22 deletions(-) diff --git a/src/lists.rs b/src/lists.rs index de82f2cd6243..b05983975174 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -17,7 +17,7 @@ use comment::{find_comment_end, rewrite_comment, FindUncommented}; use config::{Config, IndentStyle}; use rewrite::RewriteContext; -use utils::mk_sp; +use utils::{first_line_width, last_line_width, mk_sp}; #[derive(Eq, PartialEq, Debug, Copy, Clone)] /// Formatting tactic for lists. This will be cast down to a @@ -86,8 +86,12 @@ pub struct ListItem { } impl ListItem { + pub fn inner_as_ref(&self) -> &str { + self.item.as_ref().map_or("", |s| &*s) + } + pub fn is_multiline(&self) -> bool { - self.item.as_ref().map_or(false, |s| s.contains('\n')) || self.pre_comment.is_some() || + self.inner_as_ref().contains('\n') || self.pre_comment.is_some() || self.post_comment .as_ref() .map_or(false, |s| s.contains('\n')) @@ -167,7 +171,9 @@ pub fn write_list(items: I, formatting: &ListFormatting) -> Option // will be a trailing separator. let trailing_separator = needs_trailing_separator(formatting.trailing_separator, tactic); let mut result = String::new(); + let cloned_items = items.clone(); let mut iter = items.into_iter().enumerate().peekable(); + let mut item_max_width: Option = None; let mut line_len = 0; let indent_str = &formatting.shape.indent.to_string(formatting.config); @@ -238,6 +244,7 @@ pub fn write_list(items: I, formatting: &ListFormatting) -> Option } else { result.push(' '); } + item_max_width = None; } result.push_str(&inner_item[..]); @@ -261,36 +268,60 @@ pub fn write_list(items: I, formatting: &ListFormatting) -> Option } if tactic == DefinitiveListTactic::Vertical && item.post_comment.is_some() { - // 1 = space between item and comment. - let width = formatting - .shape - .width - .checked_sub(item_last_line_width + 1) - .unwrap_or(1); - let mut offset = formatting.shape.indent; - offset.alignment += item_last_line_width + 1; let comment = item.post_comment.as_ref().unwrap(); - - debug!("Width = {}, offset = {:?}", width, offset); - // Use block-style only for the last item or multiline comments. - let block_style = !formatting.ends_with_newline && last || - comment.trim().contains('\n') || - comment.trim().len() > width; - - let formatted_comment = try_opt!(rewrite_comment( - comment, - block_style, - Shape::legacy(width, offset), + let block_style = !formatting.ends_with_newline && last; + let mut formatted_comment = try_opt!(rewrite_post_comment( formatting.config, + formatting.shape, + comment, + &cloned_items, + inner_item, + i, + &mut item_max_width, + item_last_line_width, + last, + block_style, )); if !formatted_comment.starts_with('\n') { - result.push(' '); + let mut comment_alignment = + post_comment_alignment(item_max_width, inner_item.len()); + if first_line_width(&formatted_comment) + last_line_width(&result) + + comment_alignment + 1 > formatting.config.max_width() + { + item_max_width = None; + formatted_comment = try_opt!(rewrite_post_comment( + formatting.config, + formatting.shape, + comment, + &cloned_items, + inner_item, + i, + &mut item_max_width, + item_last_line_width, + last, + block_style, + )); + comment_alignment = post_comment_alignment(item_max_width, inner_item.len()); + } + for _ in 0..(comment_alignment + 1) { + result.push(' '); + } + // An additional space for the missing trailing comma + if last && item_max_width.is_some() && !separate { + result.push(' '); + } + } + if formatted_comment.contains('\n') { + item_max_width = None; } result.push_str(&formatted_comment); + } else { + item_max_width = None; } if !last && tactic == DefinitiveListTactic::Vertical && item.new_lines { + item_max_width = None; result.push('\n'); } } @@ -298,6 +329,71 @@ pub fn write_list(items: I, formatting: &ListFormatting) -> Option Some(result) } +fn rewrite_post_comment( + config: &Config, + shape: Shape, + comment: &str, + cloned_items: &I, + inner_item: &str, + i: usize, + mut item_max_width: &mut Option, + item_last_line_width: usize, + last: bool, + block_style: bool, +) -> Option +where + I: IntoIterator + Clone, + T: AsRef, +{ + if item_max_width.is_none() && !last && !inner_item.contains('\n') { + *item_max_width = Some(max_width_of_item_with_post_comment(cloned_items, i)); + } + let overhead = if let &mut Some(max_width) = item_max_width { + max_width + 2 + } else { + // 1 = space between item and comment. + item_last_line_width + 1 + }; + let width = shape.width.checked_sub(overhead).unwrap_or(1); + let offset = shape.indent + overhead; + + debug!("Width = {}, offset = {:?}", width, offset); + // Use block-style only for the last item or multiline comments. + let block_style = block_style || comment.trim().contains('\n') || comment.trim().len() > width; + + rewrite_comment(comment, block_style, Shape::legacy(width, offset), config) +} + +fn max_width_of_item_with_post_comment(items: &I, i: usize) -> usize +where + I: IntoIterator + Clone, + T: AsRef, +{ + let mut max_width = 0; + let mut first = true; + for item in items.clone().into_iter().skip(i) { + let item = item.as_ref(); + if !first && (item.is_multiline() || !item.post_comment.is_some()) { + return max_width; + } + let inner_item_width = item.inner_as_ref().len(); + if max_width < inner_item_width { + max_width = inner_item_width; + } + if item.new_lines { + return max_width; + } + first = false; + } + max_width +} + +fn post_comment_alignment(item_max_width: Option, inner_item_len: usize) -> usize { + item_max_width + .and_then(|max_line_width| max_line_width.checked_sub(inner_item_len)) + .unwrap_or(0) +} + pub struct ListItems<'a, I, F1, F2, F3> where I: Iterator, From 57466dc68799f4c28bae5e7531555334052c3d65 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Thu, 6 Jul 2017 01:03:07 +0900 Subject: [PATCH 3/6] Format source codes --- src/bin/cargo-fmt.rs | 14 +++++++------- src/expr.rs | 4 +--- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/bin/cargo-fmt.rs b/src/bin/cargo-fmt.rs index b4a1b3d51722..95ac1a25bb9b 100644 --- a/src/bin/cargo-fmt.rs +++ b/src/bin/cargo-fmt.rs @@ -142,14 +142,14 @@ fn get_fmt_args() -> Vec { #[derive(Debug)] enum TargetKind { - Lib, // dylib, staticlib, lib - Bin, // bin - Example, // example file - Test, // test file - Bench, // bench file + Lib, // dylib, staticlib, lib + Bin, // bin + Example, // example file + Test, // test file + Bench, // bench file CustomBuild, // build script - ProcMacro, // a proc macro implementation - Other, // plugin,... + ProcMacro, // a proc macro implementation + Other, // plugin,... } impl TargetKind { diff --git a/src/expr.rs b/src/expr.rs index 1326d3c41b13..971bdc21aaa9 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -1298,9 +1298,7 @@ fn rewrite_cond( label_string, self.keyword, between_kwd_cond_comment.as_ref().map_or( - if pat_expr_string.is_empty() || - pat_expr_string.starts_with('\n') - { + if pat_expr_string.is_empty() || pat_expr_string.starts_with('\n') { "" } else { " " From a4cce31ea9d7dfd3213102dee2181ea9a8959279 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Thu, 6 Jul 2017 01:03:20 +0900 Subject: [PATCH 4/6] Update tests --- tests/source/structs.rs | 23 ++++++++++++++++ ...configs-struct_field_align_threshold-20.rs | 4 +-- tests/target/enum.rs | 2 +- .../target/fn-args-with-last-line-comment.rs | 4 +-- tests/target/fn-simple.rs | 2 +- tests/target/macros.rs | 2 +- tests/target/multiple.rs | 4 +-- tests/target/structs.rs | 27 +++++++++++++++++-- 8 files changed, 57 insertions(+), 11 deletions(-) diff --git a/tests/source/structs.rs b/tests/source/structs.rs index bd507667f251..e0e27f538fc3 100644 --- a/tests/source/structs.rs +++ b/tests/source/structs.rs @@ -209,3 +209,26 @@ fn foo() { convex_shape.set_point(2, &Vector2f { x: 450.0, y: 100.0 }); convex_shape.set_point(3, &Vector2f { x: 580.0, y: 150.0 }); } + +struct Foo { + aaaaa: u32, // a + + b: u32, // b + cc: u32, // cc + + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 1 + yy: u32, // comment2 + zzz: u32, // comment3 + + aaaaaa: u32, // comment4 + bb: u32, // comment5 + // separate + dd: u32, // comment7 + c: u32, // comment6 + + aaaaaaa: u32, /* multi + * line + * comment + */ + b: u32, // hi +} diff --git a/tests/target/configs-struct_field_align_threshold-20.rs b/tests/target/configs-struct_field_align_threshold-20.rs index 509bcbf6d64e..08210b2ee3c7 100644 --- a/tests/target/configs-struct_field_align_threshold-20.rs +++ b/tests/target/configs-struct_field_align_threshold-20.rs @@ -76,7 +76,7 @@ pub struct Writebatch { struct NewInt( pub i32, SomeType, // inline comment - T, // sup + T, // sup ); struct Qux< @@ -219,7 +219,7 @@ struct Foo( where T: PartialEq; struct Foo( - TTTTTTTTTTTTTTTTT, // Foo + TTTTTTTTTTTTTTTTT, // Foo UUUUUUUUUUUUUUUUUUUUUUUU, // Bar // Baz TTTTTTTTTTTTTTTTTTT, diff --git a/tests/target/enum.rs b/tests/target/enum.rs index 14b9910687d0..9bba8840581a 100644 --- a/tests/target/enum.rs +++ b/tests/target/enum.rs @@ -88,7 +88,7 @@ pub enum GenericEnum I: Iterator, { // Pre Comment - Left { list: I, root: T }, // Post-comment + Left { list: I, root: T }, // Post-comment Right { list: I, root: T }, // Post Comment } diff --git a/tests/target/fn-args-with-last-line-comment.rs b/tests/target/fn-args-with-last-line-comment.rs index 2ffb3909b742..ef40e040ea8e 100644 --- a/tests/target/fn-args-with-last-line-comment.rs +++ b/tests/target/fn-args-with-last-line-comment.rs @@ -3,8 +3,8 @@ pub trait X { fn a(&self) -> &'static str; fn bcd( &self, - c: &str, // comment on this arg - d: u16, // comment on this arg + c: &str, // comment on this arg + d: u16, // comment on this arg e: &Vec, // comment on this arg ) -> Box; } diff --git a/tests/target/fn-simple.rs b/tests/target/fn-simple.rs index 76776672ca8c..e150973a67d0 100644 --- a/tests/target/fn-simple.rs +++ b/tests/target/fn-simple.rs @@ -2,7 +2,7 @@ fn simple( // pre-comment on a function!? - i: i32, // yes, it's possible! + i: i32, // yes, it's possible! response: NoWay, // hose ) { fn op( diff --git a/tests/target/macros.rs b/tests/target/macros.rs index 1f482075df47..d784c26ce1c8 100644 --- a/tests/target/macros.rs +++ b/tests/target/macros.rs @@ -29,7 +29,7 @@ fn main() { kaas!( // comments a, // post macro - b // another + b // another ); trailingcomma!(a, b, c,); diff --git a/tests/target/multiple.rs b/tests/target/multiple.rs index d8dea8896f2b..bf12854c9880 100644 --- a/tests/target/multiple.rs +++ b/tests/target/multiple.rs @@ -39,7 +39,7 @@ fn foo() -> Box } fn baz< - 'a: 'b, // comment on 'a + 'a: 'b, // comment on 'a T: SomsssssssssssssssssssssssssssssssssssssssssssssssssssssseType, // comment on T >( a: A, @@ -71,7 +71,7 @@ impl Bar { fn foo( &mut self, a: sdfsdfcccccccccccccccccccccccccccccccccccccccccccccccccc, // comment on a - b: sdfasdfsdfasfs, // closing comment + b: sdfasdfsdfasfs, // closing comment ) -> isize { } diff --git a/tests/target/structs.rs b/tests/target/structs.rs index dcb21bb195b5..5f471e9a3cdf 100644 --- a/tests/target/structs.rs +++ b/tests/target/structs.rs @@ -44,7 +44,7 @@ pub struct Writebatch { struct NewInt( pub i32, SomeType, // inline comment - T, // sup + T, // sup ); struct Qux< @@ -187,7 +187,7 @@ struct Foo( where T: PartialEq; struct Foo( - TTTTTTTTTTTTTTTTT, // Foo + TTTTTTTTTTTTTTTTT, // Foo UUUUUUUUUUUUUUUUUUUUUUUU, // Bar // Baz TTTTTTTTTTTTTTTTTTT, @@ -246,3 +246,26 @@ fn foo() { convex_shape.set_point(2, &Vector2f { x: 450.0, y: 100.0 }); convex_shape.set_point(3, &Vector2f { x: 580.0, y: 150.0 }); } + +struct Foo { + aaaaa: u32, // a + + b: u32, // b + cc: u32, // cc + + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 1 + yy: u32, // comment2 + zzz: u32, // comment3 + + aaaaaa: u32, // comment4 + bb: u32, // comment5 + // separate + dd: u32, // comment7 + c: u32, // comment6 + + aaaaaaa: u32, /* multi + * line + * comment + * */ + b: u32, // hi +} From f7ec959c979b83f125ba9e8c274096c8838e5fa1 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Fri, 7 Jul 2017 08:51:19 +0900 Subject: [PATCH 5/6] Use closure instead of declaring function Take comment overhead into account --- src/lists.rs | 103 ++++++++++++++++++++------------------------------- 1 file changed, 41 insertions(+), 62 deletions(-) diff --git a/src/lists.rs b/src/lists.rs index b05983975174..4f762ea1f37b 100644 --- a/src/lists.rs +++ b/src/lists.rs @@ -269,19 +269,36 @@ pub fn write_list(items: I, formatting: &ListFormatting) -> Option if tactic == DefinitiveListTactic::Vertical && item.post_comment.is_some() { let comment = item.post_comment.as_ref().unwrap(); - let block_style = !formatting.ends_with_newline && last; - let mut formatted_comment = try_opt!(rewrite_post_comment( - formatting.config, - formatting.shape, - comment, - &cloned_items, - inner_item, - i, - &mut item_max_width, - item_last_line_width, - last, - block_style, - )); + let overhead = last_line_width(&result) + first_line_width(comment.trim()); + + let rewrite_post_comment = |item_max_width: &mut Option| { + if item_max_width.is_none() && !last && !inner_item.contains('\n') { + *item_max_width = Some(max_width_of_item_with_post_comment( + &cloned_items, + i, + overhead, + formatting.config.max_width(), + )); + } + let overhead = if let &mut Some(max_width) = item_max_width { + max_width + 2 + } else { + // 1 = space between item and comment. + item_last_line_width + 1 + }; + let width = formatting.shape.width.checked_sub(overhead).unwrap_or(1); + let offset = formatting.shape.indent + overhead; + let comment_shape = Shape::legacy(width, offset); + + // Use block-style only for the last item or multiline comments. + let block_style = !formatting.ends_with_newline && last || + comment.trim().contains('\n') || + comment.trim().len() > width; + + rewrite_comment(comment, block_style, comment_shape, formatting.config) + }; + + let mut formatted_comment = try_opt!(rewrite_post_comment(&mut item_max_width)); if !formatted_comment.starts_with('\n') { let mut comment_alignment = @@ -290,18 +307,7 @@ pub fn write_list(items: I, formatting: &ListFormatting) -> Option comment_alignment + 1 > formatting.config.max_width() { item_max_width = None; - formatted_comment = try_opt!(rewrite_post_comment( - formatting.config, - formatting.shape, - comment, - &cloned_items, - inner_item, - i, - &mut item_max_width, - item_last_line_width, - last, - block_style, - )); + formatted_comment = try_opt!(rewrite_post_comment(&mut item_max_width)); comment_alignment = post_comment_alignment(item_max_width, inner_item.len()); } for _ in 0..(comment_alignment + 1) { @@ -329,42 +335,12 @@ pub fn write_list(items: I, formatting: &ListFormatting) -> Option Some(result) } -fn rewrite_post_comment( - config: &Config, - shape: Shape, - comment: &str, - cloned_items: &I, - inner_item: &str, +fn max_width_of_item_with_post_comment( + items: &I, i: usize, - mut item_max_width: &mut Option, - item_last_line_width: usize, - last: bool, - block_style: bool, -) -> Option -where - I: IntoIterator + Clone, - T: AsRef, -{ - if item_max_width.is_none() && !last && !inner_item.contains('\n') { - *item_max_width = Some(max_width_of_item_with_post_comment(cloned_items, i)); - } - let overhead = if let &mut Some(max_width) = item_max_width { - max_width + 2 - } else { - // 1 = space between item and comment. - item_last_line_width + 1 - }; - let width = shape.width.checked_sub(overhead).unwrap_or(1); - let offset = shape.indent + overhead; - - debug!("Width = {}, offset = {:?}", width, offset); - // Use block-style only for the last item or multiline comments. - let block_style = block_style || comment.trim().contains('\n') || comment.trim().len() > width; - - rewrite_comment(comment, block_style, Shape::legacy(width, offset), config) -} - -fn max_width_of_item_with_post_comment(items: &I, i: usize) -> usize + overhead: usize, + max_budget: usize, +) -> usize where I: IntoIterator + Clone, T: AsRef, @@ -373,10 +349,13 @@ fn max_width_of_item_with_post_comment(items: &I, i: usize) -> usize let mut first = true; for item in items.clone().into_iter().skip(i) { let item = item.as_ref(); - if !first && (item.is_multiline() || !item.post_comment.is_some()) { + let inner_item_width = item.inner_as_ref().len(); + if !first && + (item.is_multiline() || !item.post_comment.is_some() || + inner_item_width + overhead > max_budget) + { return max_width; } - let inner_item_width = item.inner_as_ref().len(); if max_width < inner_item_width { max_width = inner_item_width; } From 680a3a1d190448b93ebac9b2148dd374558eec01 Mon Sep 17 00:00:00 2001 From: topecongiro Date: Fri, 7 Jul 2017 09:01:14 +0900 Subject: [PATCH 6/6] Add tests to check alignment don't exceed max_width --- tests/source/structs.rs | 15 +++++++++++++++ tests/target/structs.rs | 15 +++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/tests/source/structs.rs b/tests/source/structs.rs index e0e27f538fc3..5fd660c0e861 100644 --- a/tests/source/structs.rs +++ b/tests/source/structs.rs @@ -210,6 +210,7 @@ fn foo() { convex_shape.set_point(3, &Vector2f { x: 580.0, y: 150.0 }); } +// Vertical alignment struct Foo { aaaaa: u32, // a @@ -231,4 +232,18 @@ struct Foo { * comment */ b: u32, // hi + + do_not_push_this_comment1: u32, // comment1 + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 2 + please_do_not_push_this_comment3: u32, // comment3 + + do_not_push_this_comment1: u32, // comment1 + // separate + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 2 + please_do_not_push_this_comment3: u32, // comment3 + + do_not_push_this_comment1: u32, // comment1 + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 2 + // separate + please_do_not_push_this_comment3: u32, // comment3 } diff --git a/tests/target/structs.rs b/tests/target/structs.rs index 5f471e9a3cdf..ad5188100957 100644 --- a/tests/target/structs.rs +++ b/tests/target/structs.rs @@ -247,6 +247,7 @@ fn foo() { convex_shape.set_point(3, &Vector2f { x: 580.0, y: 150.0 }); } +// Vertical alignment struct Foo { aaaaa: u32, // a @@ -268,4 +269,18 @@ struct Foo { * comment * */ b: u32, // hi + + do_not_push_this_comment1: u32, // comment1 + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 2 + please_do_not_push_this_comment3: u32, // comment3 + + do_not_push_this_comment1: u32, // comment1 + // separate + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 2 + please_do_not_push_this_comment3: u32, // comment3 + + do_not_push_this_comment1: u32, // comment1 + xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx: u32, // 2 + // separate + please_do_not_push_this_comment3: u32, // comment3 }