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

Not moving the first line comment of blocks #4745

Open
wants to merge 2 commits into
base: rustfmt-2.0.0-rc.2
Choose a base branch
from

Conversation

whizsid
Copy link
Contributor

@whizsid whizsid commented Mar 8, 2021

Fixes #3255 , Fixes #3994 , Closing #4715

@calebcartwright
Copy link
Member

Thank you! My review queue is pretty backed up right now, but I'm really looking forward to this one. The current behavior that breaks the comment association in these cases is a common source of frustration for users, so there will be a lot of folks that will really appreciate this!

@calebcartwright calebcartwright changed the base branch from master to rustfmt-2.0.0-rc.2 April 30, 2021 00:28
@ytmimi ytmimi added pr-targeting-2.0 This PR is targeting the 2.0 branch p-low labels Jul 27, 2022
Comment on lines +22 to +25
if true {
/* Sample
comment */
1
Copy link

Choose a reason for hiding this comment

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

personally, I think it makes more sense to not move the comment one line down. For example, the following use cases

if true { /* this is a supppppppppppppppppppppppppppper 
   long comment to describe the if condition */
   1
}

@jobarr-amzn
Copy link

I just ran into #3255. Is there any way to move this PR forward? It looks like it's labeled p-low because it's targetting 2.0.

A comment in #3887 says:

To be perfectly transparent though, rustfmt 2.0 may not happen ever, but even if it does it certainly won't happen any time soon. If and when there will be a 2.0, rest assured that will be communicated loudly and broadly. Until then I'd suggest largely forgetting about it and working with the 1.x versions of rustfmt available today.

Does that mean this PR is simply not happening?

@calebcartwright
Copy link
Member

Does that mean this PR is simply not happening?

Yes and no. It does mean this specific PR isn't going to get merged given the target branch, but the proposed changes could be merged if someone wants to pick them up and target the correct/master/1.x branch

@RalfJung
Copy link
Member

RalfJung commented Aug 4, 2024

I attempted porting this to master: #6265.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p-low pr-not-reviewed pr-targeting-2.0 This PR is targeting the 2.0 branch
Projects
None yet
6 participants