Skip to content

Proper indentation of multiline post-comments in a list #4572

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion src/formatting/lists.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,13 @@ where

let mut formatted_comment = rewrite_post_comment(&mut item_max_width)?;

// Multiline comments are not included in a previous "indentation group".
// Each multiline comment is considered as a separate group.
if formatted_comment.contains('\n') {
item_max_width = None;
formatted_comment = rewrite_post_comment(&mut item_max_width)?;
}

if !starts_with_newline(comment) {
if formatting.align_comments {
let mut comment_alignment = post_comment_alignment(
Expand Down Expand Up @@ -537,10 +544,21 @@ where
for item in items.clone().into_iter().skip(i) {
let item = item.as_ref();
let inner_item_width = UnicodeSegmentation::graphemes(item.inner_as_ref(), true).count();
let post_comment_is_multiline = item
.post_comment
.as_ref()
.map_or(false, |s| s.trim().contains('\n'));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kind of a bummer to have to do the multi vs. single line check multiple times, but at first glance I don't see a good way of avoiding that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried to find solution, but one of the checks whether the comment is mutliline is done in a loop for identifying the group, and the other is done later when formatting each items. Adding post_comment_is_multilne field to ListItem and initialize it in Iterator for ListItem may work, but I didn't try to do such change.


// Each multiline comment is an "indentation group" on its own
if first && post_comment_is_multiline {
return inner_item_width;
}

if !first
&& (item.is_different_group()
|| item.post_comment.is_none()
|| inner_item_width + overhead > max_budget)
|| inner_item_width + overhead > max_budget
|| post_comment_is_multiline)
{
return max_width;
}
Expand Down
163 changes: 163 additions & 0 deletions tests/source/issue-4546.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
// Basic tests (using vectors) - list multiline post-comment is indented on its own
fn main() {
let v = [
"A", /* item A comment */
"BBB", /* item B comment */
"CCCCCC", /* item C comment line 1
* item C comment line 2 */
"D", /* item D comment line 1
* item D comment line 2 */
"EEEEE", /* item E comment */
"FFF", /* item F comment */
"GG", /* item G comment line 1
* item G comment line 2 */
];
}

fn main() {
let v = [
"GG", /* item G comment line 1
* item G comment line 2 */
"AAAAA", /* item A comment */
"BBB", /* item B comment */
"CCCCCC", /* item C comment line 1
* item C comment line 2 */
"D", /* item D comment line 1
* item D comment line 2 */
"E", /* item E comment */
"FFF", /* item F comment */
];
}

// Tests for Struct
pub(crate) struct ListFormatting<'a> {
tactic: DefinitiveListTactic,
separator: &'a str, /* Comment */
trailing_separator: SeparatorTactic, /* Comment */
separator_place: SeparatorPlace, // Comment 1
// Comment 2
shape: Shape, /* Non-expressions, e.g., items, will have a new line at the end of the list.
* Important for comment styles. */
ends_with_newline: bool, // Remove newlines between list elements for expressions.
preserve_newline: bool, // Nested import lists get some special handling for the "Mixed" list type
nested: bool,
// Whether comments should be visually aligned.
align_comments: bool, /* comment */
config: &'a Config,
}

fn main() {
l = ListItem {
pre_comment, /* Comment */
pre_comment_style, /* Multiline comment
* Line 2 */
/* New line comment */
item: if self.inner.peek().is_none() && self.leave_last { /* Comment */
None
} else {
(self.get_item_string)(&item)
},
post_comment, /* Comment */
new_lines, /* Comment */
}
}

// Test for Function parameters
pub(crate) fn new(shape: Shape, config: &'a Config) -> Self {
ListFormatting {
tactic: DefinitiveListTactic::Vertical, /* Comment */
separator: ",", /* comment */
trailing_separator: SeparatorTactic::Never, /* Multiline comment
* second comment line */
separator_place: SeparatorPlace::Back, // A longggggggggggggggggggggggggggggggggggggggggggggggggggg comment
shape,
/* New line comment */
ends_with_newline: true, /* Comment */
preserve_newline: false, /* Comment */
nested: false, /* Comment */
align_comments: true, /* Another Multiline comment
* second comment line */
config, /* Last comment */
}
}

// Test for `where`
impl<'a, T, I, F1, F2, F3> Iterator for ListItems<'a, I, F1, F2, F3>
where
I: Iterator<Item = T>, /* Comment */
F111111111: Fn(&T) -> BytePos, /* Comment */
F2222222: Fn(&T) -> BytePos, /* Multiline comment
* Line 2 */
F3: Fn(&T) -> Option<String>, /* Comment */
{}

// Test for some types of lists
pub(crate) fn itemize_list<'a, T, I, F1, F2, F3>(
snippet_provider: &'a SnippetProvider, /* Comment */
inner: I, /* Comment */
terminator: &'a str, /* Multiline comment
* Line 2 */
separator: &'a str, /* Comment */
get_lo: F1, /* Comment */
get_hi: F2,
get_item_string: F3, /* Comment */
prev_span_end: BytePos, /* Multiline comment
* Line 2 */
next_span_start: BytePos, /* Comment */
leave_last: bool,
) -> ListItems<'a, I, F1, F2, F3>
where
I: Iterator<Item = T>, /* Comment */
F111111111: Fn(&T) -> BytePos, /* Multiline comment
* Line 2 */
F2222222: Fn(&T) -> BytePos,
F3: Fn(&T) -> Option<String>, /* Comment */
{
ListItems {/* Comment to ListItems */
snippet_provider, /* Multiline comment
* Line 2 */
inner: inner.peekable(), /* Another multiline comment
* another Line 2 */
get_lo, /* Comment */
get_hi,
get_item_string,
prev_span_end, /* Comment */
next_span_start,/* Comment */
terminator,
separator, /* Yet another multiline comment
* yet another Line 2 */
leave_last,/* Comment */
}
}


// Tests when comment in the same line of the item will exceed line width
fn main() {
let v = [
"A-Longgggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg", /* item A comment */
"BBB", /* item B Longgggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg comment */
"CCCCCC", /* item C comment line 1
* item C comment line 2 */
"D-Longggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg", /* item D comment line 1
* item D comment line 2 */
"EEEEE", /* item E comment */
];
}

// Test for nested items
fn main() {
let v1 = [
"GG", /* item G comment line 1
* item G comment line 2 */
"AAAAA", /* item A comment */
[
"BBB", /* item B comment */
"CCCCCC", /* item C comment line 1
* item C comment line 2 */
"D", /* item D comment line 1
* item D comment line 2 */
"E", /* item E comment */
],
"FFF", /* item F comment */
];
}
165 changes: 165 additions & 0 deletions tests/target/issue-4546.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
// Basic tests (using vectors) - list multiline post-comment is indented on its own
fn main() {
let v = [
"A", /* item A comment */
"BBB", /* item B comment */
"CCCCCC", /* item C comment line 1
* item C comment line 2 */
"D", /* item D comment line 1
* item D comment line 2 */
"EEEEE", /* item E comment */
"FFF", /* item F comment */
"GG", /* item G comment line 1
* item G comment line 2 */
];
}

fn main() {
let v = [
"GG", /* item G comment line 1
* item G comment line 2 */
"AAAAA", /* item A comment */
"BBB", /* item B comment */
"CCCCCC", /* item C comment line 1
* item C comment line 2 */
"D", /* item D comment line 1
* item D comment line 2 */
"E", /* item E comment */
"FFF", /* item F comment */
];
}

// Tests for Struct
pub(crate) struct ListFormatting<'a> {
tactic: DefinitiveListTactic,
separator: &'a str, /* Comment */
trailing_separator: SeparatorTactic, /* Comment */
separator_place: SeparatorPlace, // Comment 1
// Comment 2
shape: Shape, /* Non-expressions, e.g., items, will have a new line at the end of the list.
* Important for comment styles. */
ends_with_newline: bool, // Remove newlines between list elements for expressions.
preserve_newline: bool, // Nested import lists get some special handling for the "Mixed" list type
nested: bool,
// Whether comments should be visually aligned.
align_comments: bool, /* comment */
config: &'a Config,
}

fn main() {
l = ListItem {
pre_comment, /* Comment */
pre_comment_style, /* Multiline comment
* Line 2 */
/* New line comment */
item: if self.inner.peek().is_none() && self.leave_last {
/* Comment */
None
} else {
(self.get_item_string)(&item)
},
post_comment, /* Comment */
new_lines, /* Comment */
}
}

// Test for Function parameters
pub(crate) fn new(shape: Shape, config: &'a Config) -> Self {
ListFormatting {
tactic: DefinitiveListTactic::Vertical, /* Comment */
separator: ",", /* comment */
trailing_separator: SeparatorTactic::Never, /* Multiline comment
* second comment line */
separator_place: SeparatorPlace::Back, // A longggggggggggggggggggggggggggggggggggggggggggggggggggg comment
shape,
/* New line comment */
ends_with_newline: true, /* Comment */
preserve_newline: false, /* Comment */
nested: false, /* Comment */
align_comments: true, /* Another Multiline comment
* second comment line */
config, /* Last comment */
}
}

// Test for `where`
impl<'a, T, I, F1, F2, F3> Iterator for ListItems<'a, I, F1, F2, F3>
where
I: Iterator<Item = T>, /* Comment */
F111111111: Fn(&T) -> BytePos, /* Comment */
F2222222: Fn(&T) -> BytePos, /* Multiline comment
* Line 2 */
F3: Fn(&T) -> Option<String>, /* Comment */
{
}

// Test for some types of lists
pub(crate) fn itemize_list<'a, T, I, F1, F2, F3>(
snippet_provider: &'a SnippetProvider, /* Comment */
inner: I, /* Comment */
terminator: &'a str, /* Multiline comment
* Line 2 */
separator: &'a str, /* Comment */
get_lo: F1, /* Comment */
get_hi: F2,
get_item_string: F3, /* Comment */
prev_span_end: BytePos, /* Multiline comment
* Line 2 */
next_span_start: BytePos, /* Comment */
leave_last: bool,
) -> ListItems<'a, I, F1, F2, F3>
where
I: Iterator<Item = T>, /* Comment */
F111111111: Fn(&T) -> BytePos, /* Multiline comment
* Line 2 */
F2222222: Fn(&T) -> BytePos,
F3: Fn(&T) -> Option<String>, /* Comment */
{
ListItems {
/* Comment to ListItems */
snippet_provider, /* Multiline comment
* Line 2 */
inner: inner.peekable(), /* Another multiline comment
* another Line 2 */
get_lo, /* Comment */
get_hi,
get_item_string,
prev_span_end, /* Comment */
next_span_start, /* Comment */
terminator,
separator, /* Yet another multiline comment
* yet another Line 2 */
leave_last, /* Comment */
}
}

// Tests when comment in the same line of the item will exceed line width
fn main() {
let v = [
"A-Longgggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg", /* item A comment */
"BBB", /* item B Longgggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg comment */
"CCCCCC", /* item C comment line 1
* item C comment line 2 */
"D-Longggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggggg", /* item D comment line 1
* item D comment line 2 */
"EEEEE", /* item E comment */
];
}

// Test for nested items
fn main() {
let v1 = [
"GG", /* item G comment line 1
* item G comment line 2 */
"AAAAA", /* item A comment */
[
"BBB", /* item B comment */
"CCCCCC", /* item C comment line 1
* item C comment line 2 */
"D", /* item D comment line 1
* item D comment line 2 */
"E", /* item E comment */
],
"FFF", /* item F comment */
];
}