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

Use plus sign to concatenate strings #817

Closed
swenzel opened this issue Apr 28, 2019 · 4 comments
Closed

Use plus sign to concatenate strings #817

swenzel opened this issue Apr 28, 2019 · 4 comments
Labels
T: enhancement New feature or request

Comments

@swenzel
Copy link

swenzel commented Apr 28, 2019

I'd like to suggest that black enforces string concatenation using +. Why?
Consider this example:

subprocess.run([
    'curl',
    '-4',
    '-o',
    'path/to/whatever/file/i_want/this/to/end/up.txt'
    'some.long.url/that_is_about/to/be/scraped',
])

Did you see that there is a ',' missing?

subprocess.run([
    'curl',
    '-4',
    '-o',
    'path/to/whatever/file/i_want/this/to/end/up.txt'
    + 'some.long.url/that_is_about/to/be/scraped',
])

I bet now you do. This is just a simple example, but I think also in other situations it could save quite some debugging time for many developers if black did this. Feel free to disagree, though, I'm open to discussion.

Related issues are #26 and #182

@zsol zsol added the T: enhancement New feature or request label Apr 28, 2019
@jtamagnan
Copy link

I wonder if the addition of a + would change the AST and make the AST verification logic more complicated. A different option would be to wrap the string in parenthesis

subprocess.run([
    "curl",
    "-4",
    "-o",
    (
        "path/to/whatever/file/i_want/this/to/end/up.txt"
        "some.long.url/that_is_about/to/be/scraped"
    ),
])

@ambv ambv added this to To do in AST-unsafe changes May 9, 2019
@Jackenmen
Copy link
Contributor

I think doing it with parentheses is better option, I was very surprised to see that black converted this:

sth = {
    "start_msg": "Lorem ipsum dolor sit amet, consectetur adipiscing elit"
                 " Aenean sed elit venenatis, scelerisque purus ut, pretium justo."
                 " Donec suscipit hendrerit pretium."
}

into less readable:

sth = {
    "start_msg": "Lorem ipsum dolor sit amet, consectetur adipiscing elit"
    " Aenean sed elit venenatis, scelerisque purus ut, pretium justo."
    " Donec suscipit hendrerit pretium."
}

Instead of:

binary = {
    "start_msg": (
		"Lorem ipsum dolor sit amet, consectetur adipiscing elit"
	    " Aenean sed elit venenatis, scelerisque purus ut, pretium justo."
	    " Donec suscipit hendrerit pretium."
	)
}

And just adding + sign won't help with this at all, code formatted by black is supposed to always look roughly the same, so seeing one code with + string literals concatenation and other with () doesn't help with this purpose.

@Zac-HD
Copy link
Contributor

Zac-HD commented Aug 12, 2019

I strongly prefer the current behavior, which is more efficient and IMO nicer style too. I could accept adding needless parens to make this more obvious for newer pythonistas, but would be very unhappy if Black slowed down my code by adding pointless operations.

(for anyone who didn't know, string + is a runtime operation whereas implicit literal concatenation happens at compile time)

@zsol
Copy link
Collaborator

zsol commented Aug 16, 2019

Black won't ever automatically introduce + operators to concatenate strings. It will probably insert optional parentheses to make reading implicitly concatenated string literals over multiple lines easier to read, that's tracked in #620.

@zsol zsol closed this as completed Aug 16, 2019
@jtamagnan
Copy link

jtamagnan commented Aug 16, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: enhancement New feature or request
Projects
Development

No branches or pull requests

5 participants