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

Formatter collapses parenthesized if-expression #7066

Closed
cnpryer opened this issue Sep 2, 2023 · 5 comments
Closed

Formatter collapses parenthesized if-expression #7066

cnpryer opened this issue Sep 2, 2023 · 5 comments
Assignees
Labels
formatter Related to the formatter

Comments

@cnpryer
Copy link
Contributor

cnpryer commented Sep 2, 2023

Somewhat related: #6588

Line-length: 88

IMO Ruff's output is better. Reporting anyway.

# Input
class A:
    def b():
        before = (
            # comment
            0
            if self.thing is None
            else before - after
        )

# Black (23.7.0)
class A:
    def b():
        before = (
            # comment
            0
            if self.thing is None
            else before - after
        )

# Ruff (0.0.287)
class A:
    def b():
        before = (
            # comment
            0 if self.thing is None else before - after
        )

Black Playground

Ruff Playground

@charliermarsh charliermarsh added the formatter Related to the formatter label Sep 2, 2023
@charliermarsh charliermarsh changed the title Formatter un-expands parenthesized comprehension Formatter un-expands parenthesized if-expression Sep 2, 2023
@charliermarsh charliermarsh added the good first issue Good for newcomers label Sep 2, 2023
@cnpryer
Copy link
Contributor Author

cnpryer commented Sep 2, 2023

Oh wow I called it a comprehension 🙈 lol. Thanks!

@charliermarsh
Copy link
Member

Oh no prob :) It might also apply to comprehensions, I’m not sure, but I renamed to reflect the example.

@charliermarsh
Copy link
Member

I don't fully understand Black's rules here (some examples -- e.g., the first vs. second case is odd and likely a bug given that they drop the comment entirely). The preview style is also quite different.

It might be sufficient to expand the parent if the IfExp has leading or trailing own-line comments:

diff --git a/crates/ruff_python_formatter/src/expression/expr_if_exp.rs b/crates/ruff_python_formatter/src/expression/expr_if_exp.rs
index 1b22fb568..dec9b7d5f 100644
--- a/crates/ruff_python_formatter/src/expression/expr_if_exp.rs
+++ b/crates/ruff_python_formatter/src/expression/expr_if_exp.rs
@@ -53,9 +53,13 @@ impl FormatNodeRule<ExprIfExp> for FormatExprIfExp {
         let comments = f.context().comments().clone();

         let inner = format_with(|f: &mut PyFormatter| {
-            // We place `if test` and `else orelse` on a single line, so the `test` and `orelse` leading
-            // comments go on the line before the `if` or `else` instead of directly ahead `test` or
-            // `orelse`
+            if comments.has_leading(item) || comments.has_trailing(item) {
+                expand_parent().fmt(f)?;
+            }

@charliermarsh
Copy link
Member

It would be nice to map this behavior to code in Black to better understand the rules.

@charliermarsh
Copy link
Member

We decided to accept this as a deviation: #7082

@charliermarsh charliermarsh closed this as not planned Won't fix, can't repro, duplicate, stale Sep 4, 2023
@cnpryer cnpryer changed the title Formatter un-expands parenthesized if-expression Formatter collapses parenthesized if-expression Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants