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

Can omit parentheses layout for patterns #6933

Open
Tracked by #5834
MichaReiser opened this issue Aug 28, 2023 · 4 comments
Open
Tracked by #5834

Can omit parentheses layout for patterns #6933

MichaReiser opened this issue Aug 28, 2023 · 4 comments
Labels
accepted Ready for implementation formatter Related to the formatter style How should formatted code look

Comments

@MichaReiser
Copy link
Member

MichaReiser commented Aug 28, 2023

Ruff avoids adding parentheses to expressions that start or end with a parenthesized node (see can_omit_parentheses).

This is implemented by using optional_parentheses in conjunction with the in_parentheses_only_* builders. We need something similar for patterns to support

match test:
    case A | B | ["Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"]: ...

# Formatted
match test:
    case (
        A
        | B
        | ["Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"]
    ):
        ...

# Instead of
match test:
    case A | B | [
        "Aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
        "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb",
    ]:
        ...
@dhruvmanila
Copy link
Member

dhruvmanila commented Sep 25, 2023

This seems to be fixed: https://play.ruff.rs/e7f95918-d71e-479f-8fae-29cdaa476463

Closing the issue but feel free to re-open it if you think otherwise.

@MichaReiser
Copy link
Member Author

MichaReiser commented Sep 25, 2023

This isn't fixed yet. Notice how the expression gets wrapped in parentheses.

We need to avoid adding the parentheses in this case. I'm not sure why the function isn't using maybe_parenthesize(IfBreaks) directly

if has_comments {
pattern.format().with_options(Parentheses::Always).fmt(f)?;
} else {
match pattern.needs_parentheses(item.as_any_node_ref(), f.context()) {
OptionalParentheses::Multiline => {
parenthesize_if_expands(
&pattern.format().with_options(Parentheses::Never),
)
.fmt(f)?;
}
OptionalParentheses::Always => {
pattern.format().with_options(Parentheses::Always).fmt(f)?;
}
OptionalParentheses::Never => {
pattern.format().with_options(Parentheses::Never).fmt(f)?;
}
OptionalParentheses::BestFit => {
pattern.format().with_options(Parentheses::Never).fmt(f)?;
}
}
}

@charliermarsh
Copy link
Member

We want to make this change (it improves consistency), but it's not blocking for the Beta.

The challenge here is that we need to re-implement can_omit_parentheses for patterns, since that logic only applies to expressions.

@charliermarsh charliermarsh added the accepted Ready for implementation label Sep 27, 2023
@MichaReiser
Copy link
Member Author

I'm going to remove this from the stable release since it hasn't come up often.

@MichaReiser MichaReiser added the style How should formatted code look label Feb 8, 2024
@MichaReiser MichaReiser removed this from the Formatter: Stable milestone Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted Ready for implementation formatter Related to the formatter style How should formatted code look
Projects
None yet
Development

No branches or pull requests

3 participants