-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Special ExprTuple
formatting option for for
-loops
#5175
Conversation
PR Check ResultsEcosystem✅ ecosystem check detected no changes. BenchmarkLinux
Windows
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed on discord. The best way is probably to bypass Expr
and call the Tuple formatting directly from the for
formatting (and pass a custom option).
ad18250
to
61ef362
Compare
ExprTuple
formatting option for for
-loops
i completely rewrote this |
if it makes testing easier, and this is the only blocker, should we consider merging the |
That sounds reasonable to me. But we can do either |
## Motivation While black keeps parentheses nearly everywhere, with the notable exception of in the body of for loops: ```python for (a, b) in x: pass ``` becomes ```python for a, b in x: pass ``` This currently blocks #5163, which this PR should unblock. ## Solution This changes the `ExprTuple` formatting option to include one additional option that removes the parentheses when not using magic trailing comma and not breaking. It is supposed to be used through ``` #[derive(Debug)] struct ExprTupleWithoutParentheses<'a>(&'a Expr); impl Format<PyFormatContext<'_>> for ExprTupleWithoutParentheses<'_> { fn fmt(&self, f: &mut Formatter<PyFormatContext<'_>>) -> FormatResult<()> { match self.0 { Expr::Tuple(expr_tuple) => expr_tuple .format() .with_options(TupleParentheses::StripInsideForLoop) .fmt(f), other => other.format().with_options(Parenthesize::IfBreaks).fmt(f), } } } ``` ## Testing The for loop formatting isn't merged due to missing this (and i didn't want to create more git weirdness across two people), but I've confirmed that when applying this to while loops instead of for loops, then ```rust write!( f, [ text("while"), space(), ExprTupleWithoutParentheses(test.as_ref()), text(":"), trailing_comments(trailing_condition_comments), block_indent(&body.format()) ] )?; ``` makes ```python while (a, b): pass while ( ajssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssa, b, ): pass while (a,b,): pass ``` formatted as ```python while a, b: pass while ( ajssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssa, b, ): pass while ( a, b, ): pass ```
61ef362
to
8c036eb
Compare
Either works fine, but i think it's easier to test if merge or rebase main into #5163 |
Motivation
While black keeps parentheses nearly everywhere, the notable exception is in the body of for loops:
becomes
This currently blocks #5163, which this PR should unblock.
Solution
This changes the
ExprTuple
formatting option to include one additional option that removes the parentheses when not using magic trailing comma and not breaking. It is supposed to be used throughTesting
The for loop formatting isn't merged due to missing this (and i didn't want to create more git weirdness across two people), but I've confirmed that when applying this to while loops instead of for loops, then
makes
formatted as