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

"short" config option for let-else statements? #5684

Closed
calebcartwright opened this issue Feb 4, 2023 · 5 comments
Closed

"short" config option for let-else statements? #5684

calebcartwright opened this issue Feb 4, 2023 · 5 comments

Comments

@calebcartwright
Copy link
Member

Refs #4914

When the style team was discussing the formatting rules for let-else, an unsurprisingly key consideration was if/when/how the entire statement could be formatted on one line. As noted in the now-codified rules, there is a "if short enough" element to the required conditions for one-line formatting in the spirit of being consistent with other prescriptions in the current style guide edition.

Specifically https://github.com/rust-lang/rust/blob/master/src/doc/style-guide/src/statements.md#single-line-let-else-statements

With an intentional clause added to note the anticipated need for configurability in rustfmt:

Formatters may allow users to configure the value of the threshold used to determine whether a let-else statement is short.

Therefore the only real question IMO is: do we reuse an existing option, e.g. single_line_if_else_max_width, or do we introduce a new option, e.g. single_line_let_else_max_width), and if the latter then what should the default value be?

My inclination is to do the latter (largely based on past experiences of multi-use config options being insufficiently flexible and causing problems), likely with the same default value as single_line_if_else_max_width though I feel less strongly about this.

What are your thoughts @ytmimi?

cc @rust-lang/style for awareness

@ytmimi
Copy link
Contributor

ytmimi commented Feb 4, 2023

I think it would be best to have a dedicated config to control the let-else formatting, so I'm leaning towards adding a single_line_let_else_max_width config option.

@joshtriplett
Copy link
Member

Having a separate option makes sense, but I think that option should default to the same value as the existing top-level "short" threshold, so that changing that threshold by default will change all the other thresholds. That way, if someone wants to change all the thresholds they can do that in one place, and if they want to customize the individual thresholds they can set the individual options.

@calebcartwright
Copy link
Member Author

calebcartwright commented Jul 1, 2023

Resolved via #5790

(edit: fixed issue number typo)

@digama0
Copy link

digama0 commented Jul 1, 2023

@calebcartwright The head comment on that PR says that it does not implement a "short" config option for let-else, so it seems like there is a contradiction somewhere

@calebcartwright
Copy link
Member Author

Updated prior comment, thanks @digama0

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

4 participants