Skip to content

Issue6246-Let statement should rewrite rhs with comments after equal sign #6282

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

Conversation

johnhuichen
Copy link
Contributor

@johnhuichen johnhuichen commented Aug 14, 2024

Pull Request Template

Checklist

  • Confirmed that cargo test passes

Related Issues/PRs

#6246

Changes

The problem in 6246

fn main() {
    let foo = 
    // 114514
    if true {
        1919
    } else {
        810
    };
}

Rustfmt doesn't seem to account for comments between the = sign and the if expression. I solve it by checking the existence of a comment span and insert it back after rewrite.

Testing

added a test case tests/source/issue-6246.rs

@johnhuichen johnhuichen changed the title Let statement should rewrite rhs with comments after equal sign Issue6246-Let statement should rewrite rhs with comments after equal sign Aug 14, 2024
@johnhuichen johnhuichen force-pushed the let-statement-should-rewrite-rhs-with-comments-after-equal-sign branch 3 times, most recently from 38c9d8e to 41a9a72 Compare August 15, 2024 15:11
@johnhuichen johnhuichen marked this pull request as ready for review August 15, 2024 15:40
@johnhuichen johnhuichen force-pushed the let-statement-should-rewrite-rhs-with-comments-after-equal-sign branch 4 times, most recently from 3a80f0e to 2c5926b Compare August 16, 2024 01:31
@johnhuichen johnhuichen force-pushed the let-statement-should-rewrite-rhs-with-comments-after-equal-sign branch from 2c5926b to c5510ef Compare August 16, 2024 01:45
@ytmimi
Copy link
Contributor

ytmimi commented Aug 16, 2024

Haven't had a chance to review this yet (and likely won't for a while), but I wanted to point out that #5501 is already open to address the same issue. Didn't catch that until recently. This one is much simpler, but it might not be accounting for multi-line comments or block comments.

@johnhuichen
Copy link
Contributor Author

Closing my PR as https://github.com/rust-lang/rustfmt/pull/5501/files is addressing the same issue

@johnhuichen johnhuichen deleted the let-statement-should-rewrite-rhs-with-comments-after-equal-sign branch September 1, 2024 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants