Fix empty line handling when formatting typing stubs #1646
Conversation
| elif ( | ||
| current_line.is_def or current_line.is_decorator | ||
| ) and not self.previous_line.is_def: |
There was a problem hiding this comment.
Right now on master Black enforces this formatting:
class A: ...
@bar
class B: ...
def BMethod(self) -> Any: ...
class C: ...While the empty line between class B and C is right, I am not sure if the empty line between class A and class B is right since class A has an empty body. That empty line happens since Black can't tell the difference between the previous class not having an empty body or the current class being preceded by a decorator. Currently this commit keeps this behaviour since as far as I know classes don't have a reason to be decorated in stubs, but no idea whether that's right or not...
/cc @ambv @JelleZijlstra (I barely have any experience with typing stubs so your knowledge would be appreciated)
There was a problem hiding this comment.
I would put a blank line there. Nontrivial classes (with a body that's not ...) should have an empty line on either side.
There was a problem hiding this comment.
And there are some use cases for decorating classes in stubs, like @runtime_checkable for protocols.
There was a problem hiding this comment.
Sorry, I meant for the case of:
class A: ...
@bar
class B: ...
class C: ...The empty line between class A and B doesn't seem right as A and B have empty bodies. Master and this commit enforces this formatting, but I feel like it's wrong since if that decorator wasn't there, that empty line would disappear.
There was a problem hiding this comment.
Oh I see, I could go either way on that. I feel like if we do add blank lines, it should have blank lines both above and below it.
ichard26
force-pushed
the
fix_spacing_in_pyi_mode
branch
from
dc39a82 to
f1f8f63
Compare
Black used to erroneously remove all empty lines between non-function code and decorators when formatting typing stubs. Now a single empty line is enforced. I chose for putting empty lines around decorated classes that have empty bodies since removing empty lines around such classes would cause a formatting issue that seems to be impossible to fix. For example: ``` class A: ... @some_decorator class B: ... class C: ... class D: ... @some_other_decorator def foo(): -> None: ... ``` It is easy to enforce no empty lines between class A, B, and C. Just return 0, 0 for a line that is a decorator and precedes an stub class. Fortunately before this commit, empty lines after that class would be removed already. Now let's look at the empty line between class D and function foo. In this case, there should be an empty line there since it's class code next to function code. The problem is that when deciding to add X empty lines before a decorator, you can't tell whether it's before a class or a function. If the decorator is before a function, then an empty line is needed, while no empty lines are needed when the decorator is before a class. So even though I personally prefer no empty lines around decorated classes, I had to go the other way surrounding decorated classes with empty lines.
ichard26
force-pushed
the
fix_spacing_in_pyi_mode
branch
from
f1f8f63 to
6d3b722
Compare
|
I fixed the merge conflict, now there are some primer failures which look unrelated to this change; they're in flake8-bugbear in non-stub code. |
|
Flake8-bugbear is now Black-formatted (PyCQA/flake8-bugbear#136 (some familiar names there!)), restarting the Primer run should hopefully be green now. |
|
Now it's failing on virtualenv, likely because pypa/virtualenv@13c0498 reformatted it. |
|
PR #1695 flips the Primer switch for virtualenv. |
Black used to erroneously remove all empty lines between non-function code and decorators when formatting typing stubs. Now a single empty line is enforced. I chose for putting empty lines around decorated classes that have empty bodies since removing empty lines around such classes would cause a formatting issue that seems to be impossible to fix. For example: ``` class A: ... @some_decorator class B: ... class C: ... class D: ... @some_other_decorator def foo(): -> None: ... ``` It is easy to enforce no empty lines between class A, B, and C. Just return 0, 0 for a line that is a decorator and precedes an stub class. Fortunately before this commit, empty lines after that class would be removed already. Now let's look at the empty line between class D and function foo. In this case, there should be an empty line there since it's class code next to function code. The problem is that when deciding to add X empty lines before a decorator, you can't tell whether it's before a class or a function. If the decorator is before a function, then an empty line is needed, while no empty lines are needed when the decorator is before a class. So even though I personally prefer no empty lines around decorated classes, I had to go the other way surrounding decorated classes with empty lines. Co-authored-by: Jelle Zijlstra <[email protected]>
Fixes #1490.
Black used to erroneously remove all empty lines between functions
preceded by decorators and non-function code in pyi mode. Now a single
empty line is enforced.