Skip to content
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.

Sign up for GitHub

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

Fixed Issue 'Trailing comma is duplicated when obstructed by comments' #4520

Merged
merged 7 commits into from
Feb 27, 2021
Merged

Fixed Issue 'Trailing comma is duplicated when obstructed by comments' #4520

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4b16552
Fixed Issue 'Trailing comma is duplicated when obstructed by comments'
whizsid Nov 10, 2020
0d3b0ba
Merge branch 'master' of https://github.com/rust-lang/rustfmt into ls…
whizsid Feb 18, 2021
4cbd9f5
Put separator at the begining of the post comment
whizsid Feb 19, 2021
73f8fe3
Removed accidentally pushed tmp file
whizsid Feb 19, 2021
345298f
Add test cases for #4714
whizsid Feb 22, 2021
d88f622
Avoid ownership issue by a tricky way and added test cases for block …
whizsid Feb 26, 2021
70bd094
Fixed tests
whizsid Feb 26, 2021
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
7 changes: 6 additions & 1 deletion src/formatting/lists.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,12 @@ where
result.push_str(&formatted_comment);
}

if separate && sep_place.is_back() {
let need_extra_separator = separate
&& last
&& item.post_comment.as_ref().map_or(true, |cmnt| {
cmnt.find_uncommented(formatting.separator).is_none()
});
if separate && sep_place.is_back() && (!last || need_extra_separator) {
result.push_str(formatting.separator);
}

Expand Down
15 changes: 15 additions & 0 deletions tests/source/issue-3876.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
fn f<S>(s: S)
where
S: Send /* */,
//
{
}

fn main() {
foo(
x
// comment 1
,
// comment 2
)
}
14 changes: 14 additions & 0 deletions tests/target/issue-3876.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
fn f<S>(s: S)
where
S: Send /* */,
//
{
}

fn main() {
foo(
x // comment 1
,
// comment 2
)
}
Copy link
Member

@calebcartwright calebcartwright Nov 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This is a slight improvement in that the commas are not duplicated, though the formatting doesn't feel quite right. In particular I'm confused by the indentation/alignment in the first snippet, as well as the position of the , in the second.

I think a big question is whether there's a valid use case for comments before the comma, or whether that's just unintentional i.e.:

fn main() {
    foo(
        x, // comment 1
        // comment 2
    )
}

// or
fn main() {
    foo(
        x,
        // comment 1
        // comment 2
    )
}

although at the moment I suspect rustfmt may want to do this, though that's largely due to a bug in default visual alignment for comments (refs #4108)

fn main() {
    foo(
        x, // comment 1
           // comment 2
    )
}