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

Extend to trailing/leading space on split strings [Enhancement] #12

Open
peterjc opened this issue Nov 28, 2019 · 4 comments
Open

Extend to trailing/leading space on split strings [Enhancement] #12

peterjc opened this issue Nov 28, 2019 · 4 comments

Comments

@peterjc
Copy link
Collaborator

peterjc commented Nov 28, 2019

Thanks for taking on the idea in #6, here's another idea which is related but even further from your original plugin scope. Consider a long string literal like this:

print("She's the one for me, She's my ecstasy, She's the one I need. She's one in a million, She's once in a lifetime, She made me discover one of the stars above us.")

The above would trigger E501 for a long line. Leaving aside line length settings, the question here is do you put the trailing space at the beginning or the end of the line?

You might like trailing spaces:

print("She's the one for me, She's my ecstasy, She's the one I need. She's "
      "one in a million, She's once in a lifetime, She made me discover one "
      "of the stars above us.")

or in black style,

print(
    "She's the one for me, She's my ecstasy, She's the one I need. She's "
    "one in a million, She's once in a lifetime, She made me discover one "
    "of the stars above us."
)

Alternatively, you might like leading spaces (visually obvious when they are missing):

print("She's the one for me, She's my ecstasy, She's the one I need. She's"
      " one in a million, She's once in a lifetime, She made me discover one"
      " of the stars above us.")

Or in black style,

print(
    "She's the one for me, She's my ecstasy, She's the one I need. She's"
    " one in a million, She's once in a lifetime, She made me discover one"
    " of the stars above us."
)

This plugin could define a new violation code for this, maybe:

ISC00x trailing space on string continuation (use leading space)
ISC00x leading space on string continuation (use trailing space)

If black decides on a preference, you would perhaps only need one of these codes (see discussion on psf/black#182 and linked issues).

If anyone strongly prefers the opposite, it might be useful to have both defined and one ignored by default (the solution used with contradictory checks used in other flake8 plugins).

Similarly, I often see multi-line strings with new lines in them, and here my preference is to have the \n at the end of the line:

# E501 line too long
sys.exit("UNEXPECTED ERROR: %s\nPlease report via https://github.com/example/example/issues\nThank you!")

versus:

# no errors - my preference anyway,
# in file iteration the \n is part of the preceding line
sys.exit(
    "UNEXPECTED ERROR: %s\n"
    "Please report via https://github.com/example/example/issues\n"
    "Thank you!"
)

versus:

# new ISC00x leading new-line on string continuation (use trailing new-line)
sys.exit(
    "UNEXPECTED ERROR: %s"
    "\nPlease report via https://github.com/example/example/issues"
    "\nThank you!"
)
@keisheiled
Copy link
Collaborator

@peterjc it looks like that issue was closed, but I'm not sure of the resolution

@peterjc
Copy link
Collaborator Author

peterjc commented Jul 10, 2021

From the examples on psf/black#1132 it looks like when black breaks strings, it will use a leading space (not a trailing space). But we should confirm that in testing before making that a default here too.

@peterjc
Copy link
Collaborator Author

peterjc commented Mar 21, 2022

Confirmed, black v22.1.0 with --preview to enable the string breaking gives leading spaces on breaking long lines:

print(
    "She's the one for me, She's my ecstasy, She's the one I need. She's one in a"
    " million, She's once in a lifetime, She made me discover one of the stars"
    " above us."
)

However, it does not appear to auto-break at explicit \n characters, giving this for the second example:

sys.exit(
    "UNEXPECTED ERROR: %s\nPlease report via"
    " https://github.com/example/example/issues\nThank you!"
)

@peterjc
Copy link
Collaborator Author

peterjc commented Mar 21, 2022

Sticking to leading/trailing spaces, here's what black v22.1.0 --preview did with multiple spaces:

-print("This  has  far  too  many  redundant  spaces  between   the   words,  some  people  will hate it.")
+print(
+    "This  has  far  too  many  redundant  spaces  between   the   words,  some  people"
+    "  will hate it."
+)

So leaving aside other white space for now, flake8-implicit-str-concat could add a new violation code for a multiline concatenation with trailing space(s), and thus encourage using leading space(s) as per black.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants