Blank line between nested and function def in stub files.#3862
Blank line between nested and function def in stub files.#3862JelleZijlstra merged 2 commits intopsf:mainfrom
Conversation
The idea behind this change is that we stop looking into previous body to determine if there should be a blank before a function or class definition.
Input:
```python
import sys
if sys.version_info > (3, 7):
class Nested1:
assignment = 1
def function_definition(self): ...
def f1(self) -> str: ...
class Nested2:
def function_definition(self): ...
assignment = 1
def f2(self) -> str: ...
if sys.version_info > (3, 7):
def nested1():
assignment = 1
def function_definition(self): ...
def f1(self) -> str: ...
def nested2():
def function_definition(self): ...
assignment = 1
def f2(self) -> str: ...
```
Stable style
```python
import sys
if sys.version_info > (3, 7):
class Nested1:
assignment = 1
def function_definition(self): ...
def f1(self) -> str: ...
class Nested2:
def function_definition(self): ...
assignment = 1
def f2(self) -> str: ...
if sys.version_info > (3, 7):
def nested1():
assignment = 1
def function_definition(self): ...
def f1(self) -> str: ...
def nested2():
def function_definition(self): ...
assignment = 1
def f2(self) -> str: ...
```
In the stable formatting, we have a blank line sometimes, not depending on the previous statement on the same level, but on the last (potentially nested) statement in the previous body.
psf#2783/psf#3564 fixes this for classes in preview style:
```python
import sys
if sys.version_info > (3, 7):
class Nested1:
assignment = 1
def function_definition(self): ...
def f1(self) -> str: ...
class Nested2:
def function_definition(self): ...
assignment = 1
def f2(self) -> str: ...
if sys.version_info > (3, 7):
def nested1():
assignment = 1
def function_definition(self): ...
def f1(self) -> str: ...
def nested2():
def function_definition(self): ...
assignment = 1
def f2(self) -> str: ...
```
This PR additionally fixes this for function definitions:
```python
if sys.version_info > (3, 7):
if sys.platform == "win32":
assignment = 1
def function_definition(self): ...
def f1(self) -> str: ...
if sys.platform != "win32":
def function_definition(self): ...
assignment = 1
def f2(self) -> str: ...
if sys.version_info > (3, 8):
if sys.platform == "win32":
assignment = 1
def function_definition(self): ...
class F1: ...
if sys.platform != "win32":
def function_definition(self): ...
assignment = 1
class F2: ...
```
You can see the effect of this change on typeshed in https://github.com/konstin/typeshed/pull/1/files. As baseline, the preview mode changes without this PR are at konstin/typeshed#2.
|
Thanks for the PR and for running this on typeshed! I'd like to hear what other typeshed maintainers (@srittau, @Akuli, @AlexWaygood, @hauntsaninja) think of the changes in https://github.com/konstin/typeshed/pull/1/files. Personally I'm supportive; they do make the code more clear. |
Yeah, agreed, those pretty much all look like clear improvements to me! Thanks for working on this @konstin :D |
|
diff-shades results comparing this PR (870f12c) to main (a20338c). The full diff is available in the logs under the "Generate HTML diff report" step. |
|
LGTM |
JelleZijlstra
left a comment
There was a problem hiding this comment.
Looks good from an implementation perspective, will wait a little longer for feedback on whether the formatting change is good.
## Summary Fix all but one empty line differences with the black preview style in typeshed. The remaining differences are breaking with type comments and trailing commas in function definitions. I compared the empty line differences with the preview mode of black since stable has some oddities that would have been hard to replicate (psf/black#3861). Additionally, it assumes the style proposed in psf/black#3862. An edge case that also surfaced with typeshed are newline before trailing module comments. **main** | project | similarity index | total files | changed files | |--------------|------------------:|------------------:|------------------:| | cpython | 0.76083 | 1789 | 1632 | | django | 0.99966 | 2760 | 58 | | transformers | 0.99930 | 2587 | 447 | | twine | 1.00000 | 33 | 0 | | **typeshed** | 0.99978 | 3496 | **2173** | | warehouse | 0.99825 | 648 | 22 | | zulip | 0.99950 | 1437 | 27 | **PR** | project | similarity index | total files | changed files | |--------------|------------------:|------------------:|------------------:| | cpython | 0.76083 | 1789 | 1632 | | django | 0.99966 | 2760 | 58 | | transformers | 0.99930 | 2587 | 447 | | twine | 1.00000 | 33 | 0 | | **typeshed** | 0.99983 | 3496 | **18** | | warehouse | 0.99825 | 648 | 22 | | zulip | 0.99950 | 1437 | 27 | Closes #6723 ## Test Plan The main driver was the typeshed diff. I added new test cases for all kinds of possible empty line combinations in stub files, test cases for newlines before trailing module comments. --------- Co-authored-by: Micha Reiser <micha@reiser.io>
Description
The idea behind this change is that we stop looking into previous body to determine if there should be a blank before a function or class definition.
Input:
Stable style
In the stable formatting, we have a blank line sometimes, not depending on the previous statement on the same level, but on the last (potentially nested) statement in the previous body.
#2783/#3564 fixes this for classes in preview style:
This PR additionally fixes this for function definitions:
You can see the effect of this change on typeshed in konstin/typeshed#1. As baseline, the preview mode changes without this PR are at konstin/typeshed#2.
Closes #3861
Checklist - did you ...
CHANGES.mdif necessary?