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

String literals concatenated with space between and exceed line length #1964

Closed
dargueta opened this issue Feb 4, 2021 · 2 comments
Closed
Labels
F: strings Related to our handling of strings T: bug Something isn't working

Comments

@dargueta
Copy link

dargueta commented Feb 4, 2021

The latest versions of black change long string literals with implicit concatenation onto one line, violating line length limits, and leave a space between the string literals. The code was previously formatted by black 19.10b0.

To Reproduce

Given the following code, produced by 19.10b0 ...

assert not extra_files, (
    "Found extra SQL file(s) not in execution manifest for %s: %s. Did you add a"
    " file but forget to put it in the manifest?"
    % (
        os.path.relpath(transform_directory, PROJECT_ROOT),
        ", ".join(repr(f) for f in extra_files),
    )
)

...run a later version on the above code with no arguments and you'll get this:

assert (
    not extra_files
), "Found extra SQL file(s) not in execution manifest for %s: %s. Did you add a" " file but forget to put it in the manifest?" % (
    os.path.relpath(transform_directory, PROJECT_ROOT),
    ", ".join(repr(f) for f in extra_files),
)

For this ticket I'm going to ignore the confusing assertion formatting; see #1483 for that.

There are two things wrong here that I'll concern myself with:

Expected behavior

I would expect the original code to be unmodified, or at the very least be reformatted in a way that 1) doesn't exceed the line length limit and 2) doesn't add a space between two string literals.

Environment (please complete the following information):

  • Version: 20.8b0, 20.8b1, 21.5b0, master
  • OS: Mac OS 11.2.3
  • Python 3.7.6 and 3.8.6

Does this bug also happen on master? Yes

@dargueta dargueta added the T: bug Something isn't working label Feb 4, 2021
@JelleZijlstra JelleZijlstra added the F: strings Related to our handling of strings label May 30, 2021
@Jackenmen
Copy link
Contributor

It seems that this no longer causes Black to exceed line length though it does still change this code:

assert not extra_files, (
    "Found extra SQL file(s) not in execution manifest for %s: %s. Did you add a"
    " file but forget to put it in the manifest?"
    % (
        os.path.relpath(transform_directory, PROJECT_ROOT),
        ", ".join(repr(f) for f in extra_files),
    )
)

into:

assert (
    not extra_files
), (
    "Found extra SQL file(s) not in execution manifest for %s: %s. Did you add a"
    " file but forget to put it in the manifest?"
    % (
        os.path.relpath(transform_directory, PROJECT_ROOT),
        ", ".join(repr(f) for f in extra_files),
    )
)

@dargueta
Copy link
Author

dargueta commented Jun 2, 2021

Confirmed, fixed in the latest release.

@dargueta dargueta closed this as completed Jun 2, 2021
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: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants