Issue 4668

[ChinYing-Li]

Opening a new PR as I accidentally deleted the forked rustfmt repo! This PR aims at resolving #4668.

[calebcartwright]

Thanks for this! Could you please expand the set of test cases to include some other scenarios, particularly where wrap_comments still holds the default false value?

here's one in particular I'd like to include to cover non-wrapping within a chain :

fn f() {
    async {
let x  =   true  ;
        /* Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin urna felis, molestie ex. */
    }
    .boxed()
}

some other scenarios with more deeply indented/nested constructs would be good too (doesn't really matter how we get the indentations, nested items, control flow expressions, etc.)

[calebcartwright]

Could you swap these, just to maintain the association of the inline comment with the same line?

Suggested change

	// needed to set the kind of the ending character on the last line
	prev_kind = self.kind;
	prev_kind = self.kind;
	// needed to set the kind of the ending character on the last line

[ChinYing-Li]

I added a few test cases, but ran out of ideas ;)
Please let me know if you'd like to add other scenarios.

[calebcartwright]

Thank you! We'll need to backport this on to the main 1.x release branches, and it will need to be version gated (since this change will permit rustfmt to format some things it was failing to format prior) if that's something you'd be interested in!

[ChinYing-Li]

Thank you for the review, @calebcartwright ! To back-port, I will have to fix this bug on all rust-1.x branches, is my understanding correct? I'd love to work on back-porting; please let me know in case there's some pitfall I have to be aware of.

[calebcartwright]

I'd love to work on back-porting

❤️ beautiful thank you!

To back-port, I will have to fix this bug on all rust-1.x branches, is my understanding correct

Nope it's actually a bit simpler. Just open a PR against the current 1.x branch (which is https://github.com/rust-lang/rustfmt/tree/rustfmt-1.4.37 at the moment), and you'll most likely be able to just cherry-pick 65f41f3 directly.

It's a bit of a long story, but at the moment the default (master) branch in this repository contains a very different version of the rustfmt source than the version of rustfmt that actually gets released and utilized by users. That happened because there was originally a major 2.0 release planned for rustfmt with myriad breaking changes, but that's been put on hold indefinitely. Hoping to get that sorted soon, but currently we'll often have to apply changes against both the 2.0 and 1.x versions of rustfmt.

There will be some additional changes required to the code on the 1.x branch, specifically to leverage the Version gate on the arm guard. That'll be needed to maintain rustfmt's stability guarantee since this change will result in rustfmt formatting some code it wasn't formatting before.

I imagine it'll basically be taking this:

rustfmt/src/formatting/comment.rs

Line 1444 in 6b415bd

(_, FullCodeCharKind::Normal) if prev_kind == FullCodeCharKind::EndComment => {

and adding the additional check on the guard with something like:

(_, FullCodeCharKind::Normal) if prev_kind == FullCodeCharKind::EndComment && self.config_version == Version::Two => {

but you may need to get some additional context into the LineClasses struct so that is available. More info on version gating rustfmt code changes can be found here.

Feel free to open an early draft PR along the way if you have any questions!