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

Do not put multiline literal string into a single line #3620

Open
st-pasha opened this issue Mar 23, 2023 · 4 comments
Open

Do not put multiline literal string into a single line #3620

st-pasha opened this issue Mar 23, 2023 · 4 comments
Labels
T: style What do we want Blackened code to look like?

Comments

@st-pasha
Copy link

Describe the style change

If a literal string consists of multiple smaller strings concatenated via adjacency, then prefer to place each smaller string on its own line.

In Python "abc" and "a" "b" "c" are two identical strings. However, developers typically prefer the first style, as it is shorter and has less visual noise. Then, the only situation where the second style might be useful is if you want to juxtapose strings vertically:

"a"
"b"
"c"

Right now, however, Black puts those smaller strings back into a single line, negating the reason why the user wanted the string literal to be composed of several parts.

Examples in the current Black style

test("| head |\n" "|------|\n" "| row1 |\n" "| row2 |\n")

Desired style

test(
    "| head |\n"
    "|------|\n"
    "| row1 |\n"
    "| row2 |\n"
)

Additional context

@st-pasha st-pasha added the T: style What do we want Blackened code to look like? label Mar 23, 2023
@Jackenmen
Copy link
Contributor

Literal strings using implicit concatenation are only put into a single line when they will remain under the line length limit after such change.

Note that there is a small issue with the way Black joins such a string - it should actually be joined into a single literal. This is already addressed by Black's preview style and results in this formatting:

test("| head |\n|------|\n| row1 |\n| row2 |\n")

Some users have complained about Black not respecting user-made splits in the issue about experimental string processing (#2188). The problem is that generally, Black should be trying to join lines when it sees such an opportunity as it makes the style more consistent. There are some cases where user-made splits look better but in such a case, # fmt: off/on is still an option.

@st-pasha
Copy link
Author

Just encountered this problem again, with the following snippet:

log_file.write(
    f"#{'-'*79}\n"
    f"# Log started at {start_time}\n"
    f"#{'-'*79}\n"
)

Perhaps a good rule could be to always break a line at embedded newline characters, even if it would otherwise fit into a single line?

@davidgilbertson
Copy link

Note this case where it messes up the comments (this example copied from the Python docs)

Original:

import re

re.compile(
    "[A-Za-z_]"  # letter or underscore 
    "[A-Za-z0-9_]*"  # letter, digit or underscore
)

After:

import re

re.compile(
    "[A-Za-z_]" "[A-Za-z0-9_]*"  # letter or underscore  # letter, digit or underscore
)

@YodaEmbedding
Copy link

YodaEmbedding commented Mar 24, 2024

I think the simplest rule is to not ever, for any reason, do anything, to any string literal, ever, no matter what, no matter where, or what content it contains, or based on "\n", or "\r\n", ever, for any reason whatsoever.

I think the simplest rule is to break on the end of each string literal.


Inputs:

f(
    "Data: "
    f"{foo: <4} | "
    f"{bar:8d} | "
    f"{qux: >16}"
)
f(        "Data: "     f"{foo: <4} | "
  f"{bar:8d} | "                f"{qux: >16}"      )
f(    "Data: " f"{foo: <4} | " f"{bar:8d} | " f"{qux: >16}"    )

Desired output 😀:

f(
    "Data: "
    f"{foo: <4} | "
    f"{bar:8d} | "
    f"{qux: >16}"
)

Current output ☹️:

f("Data: " f"{foo: <4} | " f"{bar:8d} | " f"{qux: >16}")

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

4 participants