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

Black shouldn't merge multi-line string when last character on the line is '\n' #1540

Open
Conchylicultor opened this issue Jul 9, 2020 · 9 comments
Labels
F: strings Related to our handling of strings T: style What do we want Blackened code to look like?

Comments

@Conchylicultor
Copy link

Conchylicultor commented Jul 9, 2020

Describe the style change

It may be related to #1467, but it seems #1467 is fixed when trying on https://black.now.sh/ while this one still fail.

Currently, string split in multiple lines are merged together.

Input:

ValueError(
    'Invalid input:\n'
    f' * x={x}\n'
    f' * y={y}\n'
    f' * z={z}'
)

When the last character of the line is a \n, the code formatting match how the string will be displayed:

Invalid input:
 * x=12
 * y=41
 * z=0

Examples in the current Black style

Black merge all lines together, while keeping segments separated, which hurts readability:

ValueError("Invalid input:\n" f" * x={x}\n" f" * y={y}\n" f" * z={z}")

Desired style

Black shouldn't merge the lines together and keep the original formatting:

ValueError(
    'Invalid input:\n'
    f' * x={x}\n'
    f' * y={y}\n'
    f' * z={z}'
)
@Conchylicultor Conchylicultor added the T: style What do we want Blackened code to look like? label Jul 9, 2020
@ichard26
Copy link
Collaborator

ichard26 commented Jul 20, 2020

Looks like a duplicate of #1467 (not sure, this should stay open for now). IMHO, I wouldn't exactly trust the online Black demo at https://black.now.sh, from my experience it seems unreliable for certain reproducing conditions.

@bbugyi200
Copy link
Contributor

bbugyi200 commented Jul 22, 2020

@Conchylicultor With the introduction of #1132, the segments are now merged together.

@Conchylicultor
Copy link
Author

@Conchylicultor With the introduction of #1132, the segments are now merged together.

But I don't want black to merge the string. Black should respect the \n, similarly to #1467.

msg = (
    'Instructions:\n'
    ' 1) ...\n'
    ' 2) ...\n'
    ' 3) ...'
)

Is more readable than:

msg =  'Instructions:\n 1) ...\n 2) ...\n 3) ...'

As code formatting match the way the string is displayed.

@bbugyi200
Copy link
Contributor

bbugyi200 commented Jul 22, 2020

@Conchylicultor The case described by this issue is a bit different than #1467, since #1467 relates directly to #1132, whereas this issue does not.

I'm not saying that black shouldn't act the way you describe in this isue and the way described in #1467. I'm just pointing out that the code changes necessary in the two cases are very different.

@Conchylicultor
Copy link
Author

@bbugyi200, thank you for the clarification. I understand now that #1132 wasn't trying to solve this issue.

@ichard26 ichard26 added the F: strings Related to our handling of strings label May 30, 2021
@zsol
Copy link
Collaborator

zsol commented Sep 29, 2021

As a workaround you can put explicit + operators to prevent black from merging the string.

@mvong-dlb
Copy link

mvong-dlb commented Dec 22, 2021

using explicit "+" still results in the strings merging into 1 (using version=black, 21.12b0)

@bbugyi200
Copy link
Contributor

bbugyi200 commented Dec 22, 2021

My personal opinion is that this issue should be closed and #1467 should remain open.

Moreover, I think that this issue's example should continue to be formatted as it is currently (when the --experimental-string-processing flag is set, that is), since short strings should always be merged into one string when possible IMO. When string splitting / wrapping is necessary (i.e. for long strings), however, I agree that black should try to respect newlines in the string.

@Andrej730
Copy link

This issue is kind of popularizes use of "+" for string concatenation for me. 🥲

a = (
    "&SortByValue=priority&SortByOrder=asc"
    + "&IndexToStartPaging={3}"
    + "&NumberOfElementsToShow={2}"
    + "&CacheDurationMinutes=0"
    + "&AllowAlternativeResults=false"
)

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

No branches or pull requests

6 participants