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

Long lines that concatenate list comprehensions #4353

Open
tomkcook opened this issue May 8, 2024 · 3 comments
Open

Long lines that concatenate list comprehensions #4353

tomkcook opened this issue May 8, 2024 · 3 comments
Labels
F: linebreak How should we split up lines? T: style What do we want Blackened code to look like?

Comments

@tomkcook
Copy link

tomkcook commented May 8, 2024

Describe the style change

Concatenated list comprehensions should be put one-per-line rather than splitting within a comprehension.

Examples in the current Black style

            matching_routes = [r for r in routes if r.get("dst") and ip in ipaddress.ip_network(r.get("dst"))] + [
                r for r in routes if r.get("dst") == "" and r.get("family") == family
            ]

Desired style

            matching_routes = (
                [r for r in routes if r.get("dst") and ip in ipaddress.ip_network(r.get("dst"))] +
                [r for r in routes if r.get("dst") == "" and r.get("family") == family]
            )

Additional context

Currently, Black will reformat the desired style above into the current style shown above, which is a pretty clear loss for readability. Note that the above example was done with a custom line length limit of 120 but the same issue will come up with other line lengths.

@tomkcook tomkcook added the T: style What do we want Blackened code to look like? label May 8, 2024
@tomkcook
Copy link
Author

tomkcook commented May 8, 2024

A complete example:

def foo():
    matching_routes = [
        r for r in routes if r.get("dst") and ip in ipaddress.ip_network(r.get("dst"))
    ] + [r for r in routes if r.get("dst") == "" and r.get("family") == family]

would be better as:

def foo():
    matching_routes = (
        [r for r in routes if r.get("dst") and ip in ipaddress.ip_network(r.get("dst"))] +
        [r for r in routes if r.get("dst") == "" and r.get("family") == family]
    )

@JelleZijlstra JelleZijlstra added the F: linebreak How should we split up lines? label May 8, 2024
@AMK9978
Copy link

AMK9978 commented May 16, 2024

Hey Guys!
@JelleZijlstra Is this accepted? I agree with @tomkcook that his proposal is more beautiful. I'd like to work on this!

@JelleZijlstra
Copy link
Collaborator

I agree it looks better in these examples and I'd encourage you to make a PR trying to implement it, but style changes in Black often have effects in unexpected areas, and I'll have to see the changes from the PR before I can commit to making a style change.

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

No branches or pull requests

3 participants