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

Black produces lines longer than 88 #64

Closed
dstufft opened this issue Mar 22, 2018 · 5 comments
Closed

Black produces lines longer than 88 #64

dstufft opened this issue Mar 22, 2018 · 5 comments

Comments

@dstufft
Copy link

dstufft commented Mar 22, 2018

Operating system: macOS
Python version: 3.6.2
Black version: 18.3a3
Does also happen on master: Yes

This may be intentional, but I'm attempting to get this working on a codebase, and I noticed that it's producing some lines that are longer than the 88 limit that black defaults to, but which could easily be broken down into compliant lines.

Things like:

                    response.cache_control.stale_while_revalidate = stale_while_revalidate
                    response.cache_control.stale_if_error = stale_if_error
                    response.cache_control.max_age = seconds

could instead be written as:

                    response.cache_control.stale_while_revalidate = \
                        stale_while_revalidate
                    response.cache_control.stale_if_error = stale_if_error
                    response.cache_control.max_age = seconds

Another example is:

    assert exc.value.faultString == "RuntimeError: This API has been removed. Please Use BigQuery instead."

Which could instead be written as:

    assert exc.value.faultString == \
        "RuntimeError: This API has been removed. Please Use BigQuery instead."

or

    assert exc.value.faultString == (
        "RuntimeError: This API has been removed. Please Use BigQuery instead.")

or something similiar

@ambv
Copy link
Collaborator

ambv commented Mar 22, 2018

Donald, thanks for testing Black!

Black will never put backslashes, and indeed is removing them when it finds them. They are just bad style.

It can't yet put or remove organizational parentheses automatically (this is tracked in #4) but if you put them, it will use them to break lines properly. Just remember to not put trailing commas that would make those parens into tuples.

Hopefully that workaround will get you going for the time being.

@dstufft
Copy link
Author

dstufft commented Mar 22, 2018

Hmm, I suppose something with () could work, though black ends up making that take up extra space, so that instead of something like

something_really_really_really_really_long_with_a_long_name = \
    'something_that_is_also_kinda_long'

You end up with something like:

something_really_really_really_really_long_with_a_long_name = (
    'something_that_is_also_kinda_long'
)

It's not clear to me that taking up an extra line and introducing more visual noise (which is also more error prone, as you mentioned accidently adding a , will cause it to be a tuple instead of line continuation) is better. That being said, if the official stand is to use ( ... ) always for this, then I'll go ahead and adjust.

@ambv
Copy link
Collaborator

ambv commented Mar 22, 2018

tl;dr - Yes, the official stand of Black is to always use parens and never use backslashes.

(I don't have a very well worded explanation for it so allow for a bit of a stream of consciousness in what follows.)

Python has significant indentation for blocks ending with a colon. It doesn't have significant indentation for bracketed expressions and backslashes. In other words:

# 1
if True:
    print('significant indent')

# 2
print(
'non-significant'
)

# 3
import\
sys

Non-significant indentation allowed in Python poses a composability problem with pieces of significant indentation. A known example is:

if (some_long_rule1
    and some_long_rule2):
    print("ugly alert: body indented like the if test!")

To combat this, Black's style always dedents closing parens to make it obvious to the reader what is going on:

if (
    some_long_rule_rule1
    and some_long_rule2
):
    print("ugly alert: body indented like the if test!")

Yes, more lines but super clear to a human eye.

The problem with backslashes is that there is no closing token here so Black wouldn't be able to show the human reader that a block ended:

if some_long_rule_rule1 \
    and some_long_rule2:
    print("ugly alert: body indented like the if test!")

Other formatters and PEP 8 work around this problem by creating extra indentation in those cases:

if some_long_rule_rule1 \
        and some_long_rule2:
    print("ugly alert: body indented like the if test!")

This looks bad. More importantly, it pushes your line four extra spaces further, making it more likely that the line won't fit, and thus creating a cascading effect of terrible formatting.

cc @willingc, we probably want this nicely spelled out in the documentation.

@willingc
Copy link
Collaborator

@dstufft After digging through some large codebases that I didn't write, I personally found that the (..) with the extra lines made it much, much easier to scan down the code. As the writer of code, I could see why less vertical space is nice, more lines on a screen. As a reader, I much prefer (..) with vertical spaces. Great energy and work on warehouse too 😄 ❤️

@ambv ambv mentioned this issue Mar 23, 2018
@carljm
Copy link
Collaborator

carljm commented Mar 23, 2018

I don't think there's anything actionable here that isn't already covered by #4.

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

4 participants