Preserve normalized comments after last list item #5091
Conversation
There was a problem hiding this comment.
Fix LGTM, thank you! Couple minor changes/extensions requested on the tests.
One left inline, but GitHub is being weird and won't let me add other inline comments, so wanted to ask that the empty enum and struct (E2 and S2) be decorated in some way to denote the incorrect indentation of the second line in each is due to the bug described in #4854 as opposed to expected/desired behavior.
Adding a comment to that effect will be helpful for someone in the future once #4854 is fixed so they know they can safely update those lines in these tests
tests/target/issue-4909.rs
Outdated
| @@ -0,0 +1,70 @@ | |||
| // rustfmt-normalize_comments: true | |||
|
|
|||
There was a problem hiding this comment.
There was a comment reported on the linked issue that the same symptoms were observed with wrap_comments as well. Would you mind also checking if the original behavior can be reproduced with wrap_comments, and if so, whether that's also resolved with your fix (and include a test with wrap_comments too 🙏)
There was a problem hiding this comment.
Yes, the issue was present with wrap_comments too. I added more tests now.
There was a problem hiding this comment.
Fantastic! One last thing 😄 would you mind restructuring the tests to have separate pairs with wrap_comments and normalize_comments set in isolation of each other?
There was a problem hiding this comment.
Ohh I didn't realize it was also happening with wrap_comments in isolation. I will add another test.
mujpao
force-pushed
the
fix-denormalizes-some-comments
branch
from
4081380 to
e3ab83e
Compare
mujpao
force-pushed
the
fix-denormalizes-some-comments
branch
from
e3ab83e to
47a6dca
Compare
|
That'll work thank you! I suspect there's probably at least one duplicate report #4909 somewhere in the issue backlog. If you find yourself really bored and willing to search for any other issues that are now fixed by your changes that'd be great (no worries if not though) |
|
Awesome, thanks so much for checking! Would you be willing/interested in opening another PR that adds respective test cases for those issues? |
|
Sure! |
Fixes #4909
This issue was that a multi-line comment starting on a new line after the last item of a list is considered a post-comment.
This adds a check for whether a post-comment in a list starts with a newline and doesn't force the comment to become a block comment in that case.