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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

(馃巵) String processing: re-enable wrap concatenated strings in parens in all cases #3955

Open
KotlinIsland opened this issue Oct 19, 2023 · 0 comments
Labels
T: bug Something isn't working

Comments

@KotlinIsland
Copy link
Contributor

KotlinIsland commented Oct 19, 2023

input:

f(
    "lorem ipsum dolor sit amet consectetur adipiscing elit sed do eiusmod tempor" 
    "incididunt ut labore et dolore magna aliqua Ut enim ad minim",
    "",
)

Expected output:

f(
    (
        "lorem ipsum dolor sit amet consectetur adipiscing elit sed do eiusmod tempor" 
        "incididunt ut labore et dolore magna aliqua Ut enim ad minim"
    ),
    "",
)

Actual:

No change

Playground

playground

Context:

This was implemented and reverted due to disruptive changes:

f(
    x,
-   "lorem ipsum dolor sit amet consectetur adipiscing elit sed do eiusmod tempor" 
-   "incididunt ut labore et dolore magna aliqua Ut enim ad minim",
+   (
+       "lorem ipsum dolor sit amet consectetur adipiscing elit sed do eiusmod tempor" 
+       "incididunt ut labore et dolore magna aliqua Ut enim ad minim"
+   ),
)

I think that it should be re-enabled, because it does improve style, readability, and catch bugs. Some additional cases should be considered:

  • Only wrap the string if it shares a boundary with an expression that starts with a string literal:
Details

f(
    x,
    "a"  # don't wrap
    "b",
)

f(
    "a"  # don't wrap
    "b",
    x,
)

f(
    1,
    "a"  # don't wrap
    "b",
)

f(
    "a" + "b",
    "a"  # wrap, the preceding parameter begins with a string literal
    "b",
)

  • Don't wrap in cases of following keyword arguments:
Details

f(
    "a"  # don't wrap
    "b",
    a="a",
)

f(
    "a",
    a="a"  # don't wrap
    "b",
)

f(
    a="a",
    "a"  # wrap
    "b",
)

f(
    a="a"  # wrap
    "b",
    "a",
)

I feel like this is a healthy balance of disruption and readability.

Related:

Black version 23.10.0

@KotlinIsland KotlinIsland added the T: bug Something isn't working label Oct 19, 2023
@KotlinIsland KotlinIsland changed the title (馃悶) String processing: wrap concatenated strings in parens in all cases (馃悶) String processing: re-enable wrap concatenated strings in parens in all cases Oct 19, 2023
@KotlinIsland KotlinIsland changed the title (馃悶) String processing: re-enable wrap concatenated strings in parens in all cases (馃巵) String processing: re-enable wrap concatenated strings in parens in all cases Oct 19, 2023
@psf psf deleted a comment Oct 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant