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

Don't wrap with statement with multiple context managers in parentheses when the line only overflows because of the last one #4262

Closed
vberlier opened this issue Mar 2, 2024 · 2 comments
Labels
T: style What do we want Blackened code to look like?

Comments

@vberlier
Copy link

vberlier commented Mar 2, 2024

Describe the style change

As of version 24.1.0, Black now wraps multiple context managers in parentheses #3489

This is a great feature, but in some cases it's a bit too eager to reformat. When the line only overflows because of the last context manager in the with statement, wrapping everything in parentheses is a bit redundant and takes up a lot of vertical space. Previously, if the last context manager was already formatted to wrap over multiple lines, Black would leave it unchanged.

Examples in the current Black style

with (
    stream.intercept("newline"),
    stream.syntax(
        colon=r":",
        dash=r"\-",
        key=r"[a-zA-Z0-9._+-]+",
    ),
):

Desired style

This was the original input, left unchanged by Black prior to version 24.1.0

with stream.intercept("newline"), stream.syntax(
    colon=r":",
    dash=r"\-",
    key=r"[a-zA-Z0-9._+-]+",
):

Additional context

Diff when updating Black mcbeet/mecha@2ac5ebe

This occurs a lot when using the tokenstream library. A single with statement will often combine a few context managers to configure a TokenStream for parsing. The last context manager is usually a list of token patterns that already wraps over multiple lines.

@vberlier vberlier added the T: style What do we want Blackened code to look like? label Mar 2, 2024
@AlexWaygood
Copy link
Contributor

I prefer the new style. It's more verbose in some situations, yes, but it makes it very easy to see how many context managers there are in any given with statement.

@vberlier
Copy link
Author

I brought it up because some aspects of the 24.1.0 release were about being less wasteful with vertical space, and here the changes had the opposite effect. I'll close the issue if forcing context managers on separate lines is considered more important :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

2 participants