-
Notifications
You must be signed in to change notification settings - Fork 924
handle long param attributes #4047
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
handle long param attributes #4047
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR! LGTM, +1 if the max-width issue can be fixed as well 👍
@@ -0,0 +1,16 @@ | |||
fn a1( | |||
#[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa] a: u8, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, if the total width exceeds the max width, then the attribute and the parameter should be put on different lines:
fn a1(
#[aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa]
a: u8,
) {
}
@calebcartwright It would be great if you can fix this in this PR, though I am happy to merge this PR as is as entirely removing the attribute is much worse!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, if the total width exceeds the max width, then the attribute and the parameter should be put on different lines
Agreed. AFAICT, the attribute and param will be split as desired in most cases. Based on some testing with this, I believe the following circumstances have to all be true in order for them to not be split:
- only one attribute on the param
- neither the attribute nor the param individually exceed the max width
- param is named and not an empty infer/closure param
- the combination of the formatted attributed and the param pattern is under the max width, but the addition of the colon and param type causes the total line length to exceed max width
That makes me think it's a bit of an edge case, but happy to try to find a way to handle it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@calebcartwright Thank you for investigating the issue further! I agree that this is an edge case and not problematic enough to block this PR. As such, I will merge this PR as is. If you find it interesting and fun, you are more than welcome to send the follow-up PR to fix this corner case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've opened #4059 in case anyone wants to work on it and to make sure we don't lose track of it. I'll try to circle back to it eventually if no one else does, but need to finish some other issues (for rustfmt 2.x, etc.) first 👍
backported in #4980 |
Closes #4032