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

New config option for more control over block wrapping match arm bodies #4896

Open
calebcartwright opened this issue Jul 11, 2021 · 2 comments · May be fixed by #4924
Open

New config option for more control over block wrapping match arm bodies #4896

calebcartwright opened this issue Jul 11, 2021 · 2 comments · May be fixed by #4924
Labels
a-matches match arms, patterns, blocks, etc good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted

Comments

@calebcartwright
Copy link
Member

Context

The Rust Style Guide codifies the rules for formatting match expressions which is accordingly the default formatting rustfmt applies, including whether match arm bodies should or should not be block wrapped.

There's been a number of feature requests over the years to provide users with more control and flexibility around block wrapping arm bodies and a couple configuration options do exist today, including match_arm_blocks and force_multiline_blocks, that provide a degree of control.

While those two new options have proven useful, they don't provide a sufficient amount of control that some users desire. The objective for this issue is to provide enhanced rustfmt support for block wrapping match arm bodies that can be controlled by users via a new config option

Requirements

Ultimately, the new config option should support the following use cases (each one of which would be mapped to a different value of the config option)

  • Current/default behavior (maps exactly to the Style Guide Prescription)
  • Same as the default behavior, except when the opening line of the body can't fit on the same line as the => due to width constraints
    • this would be the equivalent to match_arm_blocks being overridden with a value of false (there's examples of this in the docs for that option)
  • Always block wrap all match arm bodies
  • Preserve block wrapping if the developer originally had the body wrapped
  • Same as default behavior but wrap the body if the entire formatted body cannot fit on the same line as the => operator

This new config option should of course be an enum type with corresponding variants for the different styles, and it would naturally obviate the need for the boolean match_arm_blocks. We'll need to do a soft deprecate of match_arm_blocks which means that when rustfmt encounters a case where the user has set match_arm_blocks then rustfmt should print a warning about deprecation, and internally map the specified match_arm_blocks value to the corresponding variant of the new option (there's several other cases where we've done this which can serve as references)

Implementation hints

It would probably be easiest to break this work up into chunks, and first start with the creation of the new option with the variants that map to the existing true/false values for match_arm_blocks. The next easiest piece (which could potentially be incorporated with the first) would probably be the "always wrap" variant, since the other two would require some additional work to inspect elements to conditionally determine whether to wrap and may have a few other considerations.

Feel free to propose a name for the new option and its corresponding variants as part of the PR, as we don't have any preferred names at this time. Don't worry too much about the name up front as we can always (and likely will) bikeshed on it later.

A decent starting location in the codebase for arm handling would probably be:

rustfmt/src/matches.rs

Lines 217 to 223 in 2837ca5

fn rewrite_match_arm(
context: &RewriteContext<'_>,
arm: &ast::Arm,
shape: Shape,
is_last: bool,
has_leading_pipe: bool,
) -> Option<String> {

And the configuration option info can be found in:
https://github.com/rust-lang/rustfmt/tree/master/src/config
https://github.com/rust-lang/rustfmt/blob/master/Configurations.md
7186375#diff-3fadefeddae400d637b630074fb57b82a1e37a0c44f2ba7cfa95c0a1c8a3dc75 (example soft deprecation/mapping an old op to a new one)

There should be plenty of existing snippets under tests/{source,target]/.. for matches particularly with the existing configuration options that provide similar formatting behavior, but please also feel free to reference the contributing docs on adding tests and/or asks any questions here or in Discord

@calebcartwright calebcartwright added good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted a-matches match arms, patterns, blocks, etc labels Jul 11, 2021
@ashvin021
Copy link
Contributor

Hi! I could give this a shot.

@calebcartwright
Copy link
Member Author

That's great @ashvin021! Let us know if you have any questions

ashvin021 added a commit to ashvin021/rustfmt that referenced this issue Jul 21, 2021
…ping (rust-lang#4896)

Map `match_arm_blocks` option as `match_arm_wrapping` variants.
ashvin021 added a commit to ashvin021/rustfmt that referenced this issue Jul 21, 2021
ashvin021 added a commit to ashvin021/rustfmt that referenced this issue Jul 22, 2021
ashvin021 added a commit to ashvin021/rustfmt that referenced this issue Jul 27, 2021
…ping (rust-lang#4896)

Map `match_arm_blocks` option as `match_arm_wrapping` variants.
ashvin021 added a commit to ashvin021/rustfmt that referenced this issue Jul 27, 2021
ashvin021 added a commit to ashvin021/rustfmt that referenced this issue Jul 27, 2021
@ashvin021 ashvin021 linked a pull request Jul 27, 2021 that will close this issue
ashvin021 added a commit to ashvin021/rustfmt that referenced this issue Jul 27, 2021
…ping (rust-lang#4896)

Map `match_arm_blocks` option as `match_arm_wrapping` variants.
ashvin021 added a commit to ashvin021/rustfmt that referenced this issue Jul 27, 2021
ashvin021 added a commit to ashvin021/rustfmt that referenced this issue Jul 27, 2021
ashvin021 added a commit to ashvin021/rustfmt that referenced this issue Jul 28, 2021
…ping (rust-lang#4896)

Map `match_arm_blocks` option as `match_arm_wrapping` variants.
ashvin021 added a commit to ashvin021/rustfmt that referenced this issue Jul 28, 2021
ashvin021 added a commit to ashvin021/rustfmt that referenced this issue Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-matches match arms, patterns, blocks, etc good first issue Issues up for grabs, also good candidates for new rustfmt contributors help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants