Don't flatten blocks that have labels#5677
Conversation
Previously we alwasy assumed the match arm pattern would have `shape.width` - 5 characters of space to work with. Now if we're formatting a block expression with a label we'll take the label into account.
| // 7 = ` => : {` | ||
| let label_len = label.ident.as_str().len(); |
There was a problem hiding this comment.
Does the identifier string include the ' prefix? If not we should account for that token in the width ourselves
There was a problem hiding this comment.
I double checked when I was writing the code and yes the label contains the '.
There was a problem hiding this comment.
Maybe we should future proof the code by adding a check for the leading ' in case that changes in the future
|
This is a problem that needs fixing, but also not the most urgent in the world give the ability to utilize the skip attribute on the match expression. I'd like to cross reference this against the style guide (if you'd be up for that it would be great), to both ensure that we're fully in adherence, and also see if there's any augmenting text needed to the guide |
|
@ytmimi I'm perusing through some of the open PRs to find ones that seem like good (see: easy 😆) candidates for review and merge and this seems like one. Do you have any thoughts on my prior comment about checking the style guide? If unspecified in the guide then we can probably move ahead, and perhaps it's worth a PR to update the guide? |
|
I tried checking the guide and I wasn't able to find anything about block labels. Does that mean we're good to move ahead with this? |
Thanks for checking. I suspect that's because the presence of this syntax was driven by the fairly recent RFC 2046, so yes, we should be good here |
calebcartwright
added
release-notes
2 participants
Fixes #5676
This PR prevents rustfmt from flattening a match arm's body block if that block was labeled.
I wonder if it would be worth making this change a little smarter to allow flattening in cases where the label isn't used inside the block?
r? @calebcartwright