-
Notifications
You must be signed in to change notification settings - Fork 902
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
Fix pattern struct fields #5120
base: master
Are you sure you want to change the base?
Conversation
@calebcartwright can I bother you for your initial thoughts on this? My assumption is that this change is "obviously right" and it would be good to land it before very long. |
Unfortunately with something as subjective as code style there"s rarely anything that truly, objectively falls into the category of "obviously right". Open to considering this, but to be fully transparent it"s not something I"m likely to get to any time soon. |
I think I can make at least one objective claim, that the current behavior goes against this statement in the style guide:
|
Yeah I was a little worried about that, and it"s actually why this may end up being markedly more involved than just the code diff. AFAIK there"s not been a case yet where rustfmt defaults contradict the style guide which would put us in the novel, and unfortunate, position of having to either break compatibility with the style guide or break the formatting stability guarantee. I realize the proposed changes here are version gated, but it"s something that I"d rather tackle holistically and not have to deal with potentially flip-flopping formatting on those utilizing v2 |
That is a pickle. I"d be sad to see the current behavior accepted permanently. This is the one case where I am tempted to reach for |
@ytmimi Can this be accepted for version Two? |
@SUPERCILEX apologies for the delay. I think we"re going to hold off on this one for right now. As mentioned in #5120 (comment) we still need to determine which approach we"ll take to resolve this. |
Just to make sure I"m understanding the issue correctly, is the following question what"s being debated? When rustfmt violates the style guide, should the resolution break formatting stability or trigger a style guide rewrite to match the current formatting? I see that question and this version gated PR as somewhat orthogonal. As far as I understand, the point of having multiple formatting versions is to allow the evolution of the style guide. Thus, whatever the answer to the previous question is, it is only relevant to the version one style guide and therefore would not cause flip-flopping for version 2 users (for whom formatting stability would be broken since version 2 isn"t stable yet). |
@calebcartwright Does anything change here since the style guide editions RFC was merged? Could this possibly be a 2024 edition change? |
Fixes #5117