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.

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

Retain trailing separator when extracting the last inline post comment #5141

Merged
merged 1 commit into from
Feb 4, 2022

Conversation

ytmimi
Copy link
Contributor

@ytmimi ytmimi commented Dec 19, 2021

Fixes #5042

Previously, trailing commas were removed from the last inline comment.
This lead to rustfmt refusing to format code snippets because
the original comment did not match the rewritten comment.

Now, when rustfmt extracts the last inline comment it leaves trailing
separators alone. Rustfmt does not need to remove these separators
because they are commented out.

Fixes 5042

Previously, trailing commas were removed from the last inline comment.
This lead to rustfmt refusing to format code snippets because
the original comment did not match the rewritten comment.

Now, when rustfmt extracts the last inline comment it leaves trailing
separators alone. Rustfmt does not need to remove these separators
because they are commented out.
@calebcartwright
Copy link
Member

Given that there's two underlying issues in the linked issue and only one is being addressed here, could we either make sure the description keyword is updated to not close the issue, or split the original issue into two to keep track of the indentation piece?

Comment on lines +621 to +629
let last_inline_comment_ends_with_separator = if is_last {
if let Some(line) = post_snippet.lines().last() {
line.ends_with(separator) && line.trim().starts_with("//")
} else {
false
}
} else {
false
};
Copy link
Member

Choose a reason for hiding this comment

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

This is perfectly good, just wanted to note that perhaps using a newer let chain or even a match expression might help avoid some of the 'else false' multiples

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I definitely agree that the let chains would help to simplify this code a lot!

Copy link
Member

@calebcartwright calebcartwright left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@calebcartwright calebcartwright merged commit 606894e into rust-lang:master Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Comment with a trailing comma in a function call prevents formatting
2 participants