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

Support multiline formatting for "with" blocks #557

Closed
jeffdunn opened this issue Oct 10, 2018 · 8 comments
Closed

Support multiline formatting for "with" blocks #557

jeffdunn opened this issue Oct 10, 2018 · 8 comments
Labels
T: style What do we want Blackened code to look like?

Comments

@jeffdunn
Copy link

Operating system: CentOS
Python version: 3.6.3rc1+
Black version: 18.9b0
Does also happen on master: Yes

with blocks that contain multiple elements are collapsed into a single line which violates the line length constraint. In this instance, I think we should break them into multiple lines. See the code blocks below which illustrate the current behavior. IMO, I think the input formatting should be preserved.

Input:

import tempfile

with tempfile.TemporaryDirectory() as executor_dir, \
        tempfile.TemporaryDirectory() as cache_dir, \
        tempfile.TemporaryDirectory() as socket_dir:
    pass

Output:

import tempfile

with tempfile.TemporaryDirectory() as executor_dir, tempfile.TemporaryDirectory() as cache_dir, tempfile.TemporaryDirectory() as socket_dir:
    pass
@asottile
Copy link
Contributor

asottile commented Oct 11, 2018

maybe @ambv can throw his core-dev weight and we can get a better syntax for with statements.

it always bothered me that there wasn't (essentially) this as an option:

with (
        tempfile.TemporaryDirectory() as executor_dir,
        tempfile.TemporaryDirectory() as cache_dir,
        tempfile.TemporaryDirectory() as socket_dir,
):
    ...

it would mesh nicely with the rest of the formatting that black does. sadly needs some python-itself changes

(the astute and/or poor soul who had to port a bunch of code will remember the python2.6 days where this was fine):

with contextlib.nested(
        tempfile.TemporaryDirectory(),
        tempfile.TemporaryDirectory(),
        tempfile.TemporaryDirectory(),   # such trailing comma, very wow
) as (executor_dir, cache_dir, socket_dir):
    ...

but alas.

and that you're left with a bunch of (imo) not great options:

  1. exploding indentation:
with tempfile.TemporaryDirectory() as executor_dir:
    with tempfile.TemporaryDirectory() as cache_dir:
        with tempfile.TemporaryDirectory() as socket_dir:
            ...
  1. backslash continuations:
with \
        tempfile.TemporaryDirectory() as executor_dir, \
        tempfile.TemporaryDirectory() as cache_dir, \
        tempfile.TemporaryDirectory() as socket_dir \
:
    ...
  1. what I like to call "with-ladder" -- name is a WIP
with tempfile.TemporaryDirectory(
) as executor_dir, tempfile.TemporaryDirectory(
) as cache_dir, tempfile.TemporaryDirectory(
) as socket_dir:
    ...

they all kind-of-suck for different reasons though :/

@zsol
Copy link
Collaborator

zsol commented Oct 12, 2018

This has been discussed in the past (for example #412) and I think we all agree it would be great to have python support the with (...): syntax.

Black not splitting this particular with statement into multiple lines is because () is not considered a good place to split lines on, and that's the only place this line could be split without \.

@robertlagrant
Copy link

+1 to this. PEP8 recommends using a backslash:

Backslashes may still be appropriate at times. For example, long, multiple with-statements cannot use implicit continuation, so backslashes are acceptable:

with open('/path/to/some/file/you/want/to/read') as file_1, \
     open('/path/to/some/file/being/written', 'w') as file_2:
    file_2.write(file_1.read())

@JelleZijlstra JelleZijlstra added the T: style What do we want Blackened code to look like? label May 5, 2019
@Mikem3catsus
Copy link

Mikem3catsus commented Aug 30, 2019

I've got a case where it is doing something even worse than collapsing it to a single line, it is breaking it up across multiple lines in unsatisfying locations disregarding any backslashes in the original code. Any suggestions on a workaround? I get the feeling that some of this might be some initial bad structure coming in, but I'm perplexed on how to get it even to be consistent across my 12 file creations. BTW - it's setup for a unittest

    with env, TempFileWithContents('guop_lightC0T0.log', passing_content), TempFileWithContents(
       'guop_lightC0T1.log', passing_content
   ), TempFileWithContents('guop_lightC1T0.log', passing_content), TempFileWithContents(
       'guop_lightC1T1.log', passing_content
   ), TempFileWithContents(
       'guop_lightC2T0.log', passing_content
   ), TempFileWithContents(
       'guop_lightC2T1.log', passing_content
   ), TempFileWithContents(
       'guop_lightC3T0.log', passing_content
   ), TempFileWithContents(
       'guop_lightC3T1.log', passing_content
   ), TempFileWithContents(
       'guop_lightC4T0.log', passing_content
   ), TempFileWithContents(
       'guop_lightC4T1.log', passing_content
   ), TempFileWithContents(
       'guop_lightC5T0.log', passing_content
   ), TempFileWithContents(
       'guop_lightC5T1.log', failing_content
   ):

@zsol
Copy link
Collaborator

zsol commented Sep 4, 2019

@Mikem3catsus until Black gets smarter about these cases, you can try using contextlib.ExitStack

Something like this (note the last case passing failing_content is not handled)

with ExitStack() as stack:
    cm.enter_context(env)
    [cm.enter_context(TempFileWithContents(f'guop_lightC{i}T{j}.log', passing_content)) for i in range(5) for j in range(2)]

@aarchiba
Copy link

If you can't put parentheses around everything, isn't a comma the right place to break lines (even though backslashes are ugly)?

The places you can't put top-level parentheses around a comma-separated list are, unless I've missed one:

  • with statements: basically always a problem with no better solution than backslashes,
  • assert statements: often annoying, though the list is only two elements long, because the message can be lengthy. Breaking inside the condition or the message (often "what{}ever".format(thing), which gets broken up at the brackets) is messy. I, at least, definitely want a line break at that comma, backslash or no, before I want the elements to either side of it broken up.
  • global and nonlocal: black generates overlong lines, but the user can and probably should just break these up, so it's not crucial.

So it seems like there are only two cases, with and assert, where you have a top-level comma-separated list that can't be parenthesized, and it seems like both of them are best broken at the commas. Forgive my ignorance, but is this hard?

Wishing for python to acquire new syntax (in 3.9?) does not solve the problem for library writers for the next, well, quite a few years.

@ambv
Copy link
Collaborator

ambv commented Mar 3, 2020

I changed my mind on this. Let's special-case with statements with multiple context managers and use backslashes for those.

(Let's merge this with #664.)

@ambv ambv closed this as completed Mar 3, 2020
@terrdavis
Copy link

This is coming in 3.10!

https://bugs.python.org/issue12782

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

9 participants