black icon indicating copy to clipboard operation
black copied to clipboard

Black is inconsistent with empty lines before an inner function after code block open, with or without leading comments

Open yilei opened this issue 3 years ago • 8 comments

To Reproduce

Unformatted code:

def main():
    if a:

        # Comment
        def b():
            pass

    if b:
        def c():
            pass

Running black --preview:

def main():
    if a:
        # Comment
        def b():
            pass

    if b:

        def c():
            pass

Expected behavior

Empty line should be consistent added (or removed) between the code block open and inner function:

def main():
    if a:

        # Comment
        def b():
            pass

    if b:

        def c():
            pass

Or,

def main():
    if a:
        # Comment
        def b():
            pass

    if b:
        def c():
            pass

Additional context

I did a bisect and this was caused by https://github.com/psf/black/pull/3035.

Note that if the inner function doesn't have a leading comment, it won't remove the empty line:

def main():
    if a:

        def b():
            pass

        def c():
            pass

Thus I believe this is an undesired behavior change in #3035?

yilei avatar Sep 29 '22 22:09 yilei

#246 is possibly related since that's also caused by a comment before the function def.

yilei avatar Sep 29 '22 22:09 yilei

I agree this should be fixed.

ichard26 avatar Sep 29 '22 23:09 ichard26

Looking at EmptyLineTracker, it is indeed the same underlying issue as #246, that the tracker isn't capable of doing this when there are preceding standalone comments. I'll take a stab at a fix.

yilei avatar Sep 29 '22 23:09 yilei

Having a look at #3035, I'd expect the newline to be removed before the first function regardless of the comment. Would you prefer otherwise?

felix-hilden avatar Oct 08 '22 06:10 felix-hilden

I would actually personally prefer the newline to be removed after a block open.

I saw this is inconsistent with or without the comment, plus the current Black style says:

It will also insert proper spacing before and after function definitions. It’s one line before and after inner functions...

This is the "inner function" case.

So I'm slightly in favor of changing the preview style to always remove the newline before an inner function and immediately after a block open, with or without the leading comment (I'm strong on making the behavior consistent with or without comment). I can send a separate PR to do that once we make decision. Meanwhile I updated #3302 to keep the current behavior for this case.

yilei avatar Oct 10 '22 17:10 yilei

I'm fully in favor of doing it, but it's fine to keep the original behavior if you'd like to 👍 I think the inner function spacing rule is secondary to the "no space in a new block" rule.

felix-hilden avatar Oct 15 '22 18:10 felix-hilden

@felix-hilden Thanks. Mind approve #3302 and get it merged to fix #246? Then I'll send a follow-up PR to for this issue.

yilei avatar Oct 16 '22 04:10 yilei

Done, we'll wait for another approval since Jelle's is a bit stale. Thanks!

felix-hilden avatar Oct 16 '22 08:10 felix-hilden