rustfmt icon indicating copy to clipboard operation
rustfmt copied to clipboard

Fix/Inconsistent Struct Body Opening Brace Placement After Where Clause

Open jmj0502 opened this issue 3 years ago • 3 comments

Fixes #5507

  • A new function was added to items.rs. set_brace_pos will format generic structs with where clauses appropriately based on the version provided as a parameter.
  • Different tests case for both indent_styles were added.

jmj0502 avatar Aug 18 '22 18:08 jmj0502

@ytmimi Done! Thanks for the version gate clue btw c: Let me know if there's something else we need to add!

jmj0502 avatar Aug 22 '22 19:08 jmj0502

@ytmimi Cool! I'll start to with the changes now! The contains_comments function will be super helpful (it will allow me to simplify a lot of code) ✌️

jmj0502 avatar Aug 23 '22 01:08 jmj0502

@ytmimi The changes are done! Thanks for all the comments and guidance (they'll sure help me improve as a rustacean 🦀), I really appreciate it!

The whole experience contributing to rustfmt has been awesome and I'm really looking forward to keep on making contributions! c:

jmj0502 avatar Aug 23 '22 02:08 jmj0502

@ytmimi Hey! Hope you're doing great!

I made a little change, in order to remove some redundant code. I was checking for fields.is_empty() and body_has_comments in the match expression located on items.rs - 1302, despite the fact that such match expression is contained on an if block that checks for fields.is_empty(). Now I just check for body_has_comments.

It was just a little change, but I thought that you should know about it. Have an awesome weekend!

jmj0502 avatar Oct 08 '22 00:10 jmj0502

@ytmimi Hey! Hope you're doing well!

No worries, I know that you have other occupations, so I'm ok with PRs not being reviewed instantly 👍.

The approach you decided to take seems super cool, in fact, I like the way most of the logic could be stripped down to some simple steps based on the changes made to the function signature.

I'll implement the changes and let you know when I'm done! c:

jmj0502 avatar Oct 12 '22 13:10 jmj0502

@ytmimi Donee! thanks again for all the guidance and the help. This fix really allowed me to understand some parts of Rustfmt a little bit better (in regard to the use of some utility functions, structs, etc). I'm really glad that I'm getting to know the project better and I'm definitely looking forward to keeping on making contributions.

I really appreciate all the things you've taught me; they'll sure help me improve the quality of feature contributions. I'll be ready in case any change is needed after the review c:

jmj0502 avatar Oct 14 '22 01:10 jmj0502

@ytmimi Hey! Hope you're doing awesome! Btw, happy new year! 🥳

Sorry that I've been off for a while. I wasn't very active over the course of the last two months due to different reasons including a back injury and some other personal stuff. How you've been?

Is it possible to merge this PR? #5460 depends on this one and it's technically done as well. I'll be looking forward to integrate this changes into #5460 once they land, in order to proceed and upload the updated version of the branch.

Looking forward to hearing from you! Have a nice day!

jmj0502 avatar Jan 22 '23 17:01 jmj0502