Skip to content
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

[unstable option] loop_brace_style #5448

Closed
liam-clink opened this issue Jul 17, 2022 · 6 comments
Closed

[unstable option] loop_brace_style #5448

liam-clink opened this issue Jul 17, 2022 · 6 comments

Comments

@liam-clink
Copy link

Loops are not included in the brace_style or control_brace_style

@ytmimi
Copy link
Contributor

ytmimi commented Jul 17, 2022

Thanks for reaching out the control_brace_style config option is meant to control braces for control flow constructs. As such, I think it might be better to use the existing option to control the brace style for loops instead of creating a new configuration option.

@ytmimi
Copy link
Contributor

ytmimi commented Jul 17, 2022

@liam-clink, let me know what your thoughts are. It would also be great to get a little more detail on your motivation for controling the brace style of loops.

I Also want to mention that rustfmt is an implementation of the rust style guide, and the section on Control flow expressions mentions keeping the opening brace on the same line as the expression. This doesn't mean we wouldn't consider a PR to apply this option in the loop context, but we always want to keep the style guide in mind.

@liam-clink
Copy link
Author

What I was saying is that there is no option to change loop brace style, not that control flow braces should affect that. I find vertically aligned braces to be more readable in general, that's all. I know what the style guide says, and if I were contributing to an existing project I would follow the existing style. Just want this for my own projects, and if anyone wants to contribute to them, keeping formatting consistent using rustfmt

@liam-clink
Copy link
Author

I apologize, it seems that ordinary loops do already work, but I had a closure with a loop in it inside a function, which made me think loops weren't supported. The documentation doesn't mention loops, only functions structs and enums, so I believe this should be closed and an issue opened on the repo for the website, correct?

@ytmimi
Copy link
Contributor

ytmimi commented Jul 17, 2022

If control_brace_style already controls loops I think it's best if we add those examples to the configuration docs found in the Configurations.md file. Any interest in working on a PR for that?

It might also be worth opening up a separate issue to discuss the problem you encountered with the loop inside the closure. Without seeing a code snippet it's tough to say what's going on.

@calebcartwright
Copy link
Member

Thanks for reaching out, but I am going to close given the above discussion.

@liam-clink - as @ytmimi already noted control_brace_style applies to all control flow type expressions (more info can be found here) which includes loop expressions. If you're not seeing that applied on a loop expression in standard Rust code (i.e. not as part of args passed to a macro invocation) that would potentially be a bug that we'd like to know more about. It would be best to surface that as a new issue with a minimal reproducible example and relevant details (your version of rustfmt, config options, etc.)

There's broader discussion about brace style contexts in other positions in #3376

@calebcartwright calebcartwright closed this as not planned Won't fix, can't repro, duplicate, stale Jul 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants