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

When splitting lines, \n is ignored #1467

Open
Jackenmen opened this issue May 31, 2020 · 4 comments
Open

When splitting lines, \n is ignored #1467

Jackenmen opened this issue May 31, 2020 · 4 comments
Labels
F: strings Related to our handling of strings T: style What do we want Blackened code to look like?

Comments

@Jackenmen
Copy link
Contributor

Jackenmen commented May 31, 2020

Describe the style change
Black should respect new lines (\n) in strings.

Input:

message = (
    "1. Go to Google Developers Console and log in with your favourite Google account.\n"
    "(https://console.developers.google.com/)\n"
    "2. You should be prompted to create a new project (name does not matter).\n"
    "3. Click on Enable APIs and Services at the top.\n"
    "4. In the list of APIs choose or search for YouTube Data API v3 and "
    "click on it. Choose Enable.\n"
    "5. Click on Credentials on the left navigation bar.\n"
    "6. Click on Create Credential at the top.\n"
    '7. At the top click the link for "API key".\n'
    "8. No application restrictions are needed. Click Create at the bottom.\n"
    "9. You now have a key to add to `[p]set api youtube api_key <your_api_key_here>`"
)

Examples in the current Black style:

message = (
    "1. Go to Google Developers Console and log in with your favourite Google"
    " account.\n(https://console.developers.google.com/)\n2. You should be prompted to"
    " create a new project (name does not matter).\n3. Click on Enable APIs and"
    " Services at the top.\n4. In the list of APIs choose or search for YouTube Data"
    " API v3 and click on it. Choose Enable.\n5. Click on Credentials on the left"
    " navigation bar.\n6. Click on Create Credential at the top.\n7. At the top click"
    ' the link for "API key".\n8. No application restrictions are needed. Click Create'
    " at the bottom.\n9. You now have a key to add to `[p]set api youtube api_key"
    " <your_api_key_here>`"
)

Desired style:
(place in which I split the 1. is random, I just mean that Black should respect \n)

message = (
    "1. Go to Google Developers Console and log in with your favourite"
    " Google account.\n"
    "(https://console.developers.google.com/)\n"
    "2. You should be prompted to create a new project (name does not matter).\n"
    "3. Click on Enable APIs and Services at the top.\n"
    "4. In the list of APIs choose or search for YouTube Data API v3 and "
    "click on it. Choose Enable.\n"
    "5. Click on Credentials on the left navigation bar.\n"
    "6. Click on Create Credential at the top.\n"
    '7. At the top click the link for "API key".\n'
    "8. No application restrictions are needed. Click Create at the bottom.\n"
    "9. You now have a key to add to `[p]set api youtube api_key <your_api_key_here>`"
)

Additional context:
None

I'm sorry for the amount of issues but I tried to run Black's master on Cog-Creators/Red-DiscordBot's code base which resulted in quite a few of errors and also some undesired (by me) changes which I wanted to at least report even if they're not gonna be changed.

@ichard26
Copy link
Collaborator

ichard26 commented Jul 20, 2020

@jack1142, on stable (19.10b0) and master Black doesn't produce the same output as your second example:

Master

(black) richard-26@ubuntu-laptop:~/programming/black$ cat test.py
message = (
    "1. Go to Google Developers Console and log in with your favourite Google"
    " account.\n"
    "(https://console.developers.google.com/)\n"
    "2. You should be prompted to create a new project (name does not matter).\n"
    "3. Click on Enable APIs and Services at the top.\n"
    "4. In the list of APIs choose or search for YouTube Data API v3 and "
    "click on it. Choose Enable.\n"
    "5. Click on Credentials on the left navigation bar.\n"
    "6. Click on Create Credential at the top.\n"
    '7. At the top click the link for "API key".\n'
    "8. No application restrictions are needed. Click Create at the bottom.\n"
    "9. You now have a key to add to `[p]set api youtube api_key <your_api_key_here>`"
)
(black) richard-26@ubuntu-laptop:~/programming/black$ black test.py --diff --color
All done! ✨ 🍰 ✨
1 file would be left unchanged
please ignore, string literal handling doesn't exist in stable ... haha ... oops

Stable (19.10b0)

(black) richard-26@ubuntu-laptop:~/programming/black$ git checkout tags/19.10b0
[...]
(black) richard-26@ubuntu-laptop:~/programming/black$ python black.py test.py --diff
All done! ✨ 🍰 ✨
1 file left unchanged.

Also, your case is already fixed on master since PR #1132 leaves manually split string literals alone as long they don't violate the line length limit. Look at the output on master above.

@Jackenmen
Copy link
Contributor Author

Jackenmen commented Jul 20, 2020

@ichard26 looks like I might have copied something wrongly when I was making this PR. I updated the reproduction (Input) in PR description.
And yes, this only happens on master, as the reformatting of actual strings is only in master right now. I'm reporting it so that it (hopefully) doesn't go into stable like this.

@bbugyi200
Copy link
Contributor

bbugyi200 commented Jul 22, 2020

@jack1142 It is worth noting here that black will only merge and re-split multiline strings if any one of the adjacent substrings violates the line length limit.

If you run black against the following code, no changes are made (in master or stable) since none of the lines violate the line length limit:

message = (
    "1. Go to Google Developers Console and log in with your favourite"
    " Google account.\n"
    "(https://console.developers.google.com/)\n"
    "2. You should be prompted to create a new project (name does not matter).\n"
    "3. Click on Enable APIs and Services at the top.\n"
    "4. In the list of APIs choose or search for YouTube Data API v3 and "
    "click on it. Choose Enable.\n"
    "5. Click on Credentials on the left navigation bar.\n"
    "6. Click on Create Credential at the top.\n"
    '7. At the top click the link for "API key".\n'
    "8. No application restrictions are needed. Click Create at the bottom.\n"
    "9. You now have a key to add to `[p]set api youtube api_key <your_api_key_here>`"
)

With the introduction of #1132, however, the following code (note that the first substring violates the line length limit) is merged and re-split by black:

### INPUT
message = (
    "1. Go to Google Developers Console and log in with your favourite Google account.\n"
    "(https://console.developers.google.com/)\n"
    "2. You should be prompted to create a new project (name does not matter).\n"
    "3. Click on Enable APIs and Services at the top.\n"
    "4. In the list of APIs choose or search for YouTube Data API v3 and "
    "click on it. Choose Enable.\n"
    "5. Click on Credentials on the left navigation bar.\n"
    "6. Click on Create Credential at the top.\n"
    '7. At the top click the link for "API key".\n'
    "8. No application restrictions are needed. Click Create at the bottom.\n"
    "9. You now have a key to add to `[p]set api youtube api_key <your_api_key_here>`"
)

### OUTPUT
message = (
    "1. Go to Google Developers Console and log in with your favourite Google"
    " account.\n(https://console.developers.google.com/)\n2. You should be"
    " prompted to create a new project (name does not matter).\n3. Click on"
    " Enable APIs and Services at the top.\n4. In the list of APIs choose or"
    " search for YouTube Data API v3 and click on it. Choose Enable.\n5. Click"
    " on Credentials on the left navigation bar.\n6. Click on Create"
    " Credential at the top.\n7. At the top click the link for \"API"
    " key\".\n8. No application restrictions are needed. Click Create at the"
    " bottom.\n9. You now have a key to add to `[p]set api youtube api_key"
    " <your_api_key_here>`"
)

Although there will always be cases where manually splitting a string is more desirable than having black split it automatically, I think that black can definitely handle this particular case better by respecting newline characters. With that said, #1132 introduces a huge amount of new code. For that reason, I am against adding any new features to string handling before the next release. Until then, the problem this issue points out can be worked around by manually splitting the above string without violating the line length limit.

@Jackenmen
Copy link
Contributor Author

Jackenmen commented Aug 22, 2020

I'm not sure if something has changed about this since I tried this last time, but I now have troubles reproducing this on commit cd3a93a when using --experimental-string-processing flag.
Edit: Looks I was wrong, it's still reproducible, I just ran into some issue with black's caching (probably because I was switching between versions hmm, looks I actually found some issue with black's caching when I'm changing used flags)

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

4 participants