black: Unnecessary line breaks in method call on multiline string

This is possibly not a bug, but the results are unexpected and don’t really seem to improve readability. However feel free to close if this is actually correct.

Operating system: Fedora 28 Python version: 3.6.5 Black version: 18.5b0 Does also happen on master: yes

Formatting this file uses some extra unexpected new lines:

MULTILINE = """
foo
""".replace("\n", "")
@@ -1,4 +1,6 @@
 MULTILINE = """
 foo
-""".replace("\n", "")
+""".replace(
+    "\n", ""
+)

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Reactions: 13
  • Comments: 20 (6 by maintainers)

Most upvoted comments

A related case is:

textwrap.dedent("""\
    Hello, I am
    a multiline string used with
    a common idiom
""")

We need to fix that, too.

Quick update: @olivia-hong who works with me on the Backend Language Tooling team at Lyft is going to be picking this work up again!

Black considers multiline strings as always not fitting a single line (per definition), so it tries to break the line if possible. I agree that in this case the result is sub-optimal. I’ll see what I can do to improve this.

That related case is how I got here. For clarity here, the above:

textwrap.dedent("""\
    Hello, I am
    a multiline string used with
    a common idiom
""")

becomes:

textwrap.dedent(
    """\
    Hello, I am
    a multiline string used with
    a common idiom
"""
)

Amusingly you can of course instead change it to:

s = """\
    Hello, I am
    a multiline string used with
    a common idiom
"""
s = textwrap.dedent(s)

in one fewer lines and which black will accept (and won’t churn when this is resolved) - or do the dedent call at the point of use.

I kept running into this so I decided to spend some time figuring how to fix it. Happy to say I have a branch that should do just that! It’s got a new 316 line test file with a bunch of cases (including the ones reported in this and linked issues). However, the code is still messy as I haven’t yet taken the time to clean it up: right now it’s ~100 new lines dumped into the is_line_short_enough function. I’ll post a draft PR in the next day or two after some basic cleanup, just wanted to comment to let others know if they had been thinking of spending time on this.

FWIW, the branch is more of a POC right now and requires more work before it could be mergeable. I have some hacked-together code for bracket tracking, trailing comma handling, etc., which will require some refactoring so I can reuse the existing functions for that (will need help from the black folks on the best approach). There’s also more work to do like finishing up draft changes to the black style guide docs & CHANGES.rst looking through the black-primer output to verify the changes and work with the relevant upstreams.

What’s the status of this one? I’ve seen this one has been opened around 2018 and evolved quite a bit over the years… if I’m not mistaken it seems black is still screwing up this common & readable pattern:

if __name__ == "__main__":
    transform(textwrap.dedent("""
        # from random import random

        def random():
            return 10

        a = random()*10
    """))

into something like this:

if __name__ == "__main__":
    transform(
        textwrap.dedent(
            """
        # from random import random

        def random():
            return 10

        a = random()*10
    """
        )
    )

One of the main reasons to use textwrap.dedent pattern with multiline strings is because “indentation awareness”… But in this case, that sense of indentation will be lost thanks to black… said otherwise, in this particular case black has destroyed the code into something much worse.

Black version: black, version 19.10b0 Python: 3.6.8 (tags/v3.6.8:3c6b436a57, Dec 23 2018, 23:31:17) [MSC v.1916 32 bit (Intel)] on win32

Posting an update here for visibility as well: I’ve updated the PR Aneesh originally put up and it’s now fully ready for review https://github.com/psf/black/pull/1879#issuecomment-1292668826! Comments/feedback are much appreciated.

Since the issue is a few years old at this point, tagging some of the original reviewers @JelleZijlstra @ichard26 as an FYI. Apologies for any noise.

Some context from me:

  • I’m planning to roll out black across all repos at my company (I’m on the languages team at Lyft). Similar to you, this is the only piece of black formatting that I’ve found annoying enough to consider a blocker since we have a number of repos that use multi-line strings heavily.
  • I haven’t forgotten about the PR, but unfortunately it has been put on the backburner due to other internal priorities.

I would like to get back to the PR later this year but can’t give any guarantees as to when that’ll be. In the meantime, I welcome anyone to take over the work and make their own PR!

Opened https://github.com/psf/black/pull/1879 with my prototype fix! Would love feedback from folks on the new tests I added and whether the formatting I chose makes sense (AFAIK I included all the examples from this and linked issues).