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

Formatter: Empty lines in stub files #6723

Closed
konstin opened this issue Aug 21, 2023 · 0 comments · Fixed by #7206
Closed

Formatter: Empty lines in stub files #6723

konstin opened this issue Aug 21, 2023 · 0 comments · Fixed by #7206
Assignees
Labels
formatter Related to the formatter help wanted Contributions especially welcome

Comments

@konstin
Copy link
Member

konstin commented Aug 21, 2023

When formatting typeshed (cargo run --bin ruff_dev -- format-dev --write typeshed), there is still a big diff with black wrt to empty lines.

E.g. given

class Feature:
    def getMandatoryRelease(self) -> None: ...
    compiler_flag: int

def getMandatoryRelease(self) -> None: ...
compiler_flag: int

if sys.platform != "win32":
    def can_change_color() -> bool: ...
    # Changed in Python 3.8.8 and 3.9.2
    if sys.version_info >= (3, 8):
        pass

if sys.platform == "win32":
    class Dummy:
        __new__: None
        __init__: None

    # Actual typename SummaryInformation, not exposed by the implementation
    class _SummaryInformation:
        def GetProperty(self, field: int) -> int | bytes | None: ...

black will format this as

class Feature:
    def getMandatoryRelease(self) -> None: ...
    compiler_flag: int

def getMandatoryRelease(self) -> None: ...

compiler_flag: int

if sys.platform != "win32":
    def can_change_color() -> bool: ...
    # Changed in Python 3.8.8 and 3.9.2
    if sys.version_info >= (3, 8):
        pass

if sys.platform == "win32":
    class Dummy:
        __new__: None
        __init__: None

    # Actual typename SummaryInformation, not exposed by the implementation
    class _SummaryInformation:
        def GetProperty(self, field: int) -> int | bytes | None: ...

while we format this as

class Feature:
    def getMandatoryRelease(self) -> None: ...

    compiler_flag: int

def getMandatoryRelease(self) -> None: ...

compiler_flag: int

if sys.platform != "win32":

    def can_change_color() -> bool: ...

    # Changed in Python 3.8.8 and 3.9.2
    if sys.version_info >= (3, 8):
        pass

if sys.platform == "win32":

    class Dummy:
        __new__: None
        __init__: None

    # Actual typename SummaryInformation, not exposed by the implementation
    class _SummaryInformation:
        def GetProperty(self, field: int) -> int | bytes | None: ...

The task is to go through the typeshed diff and figure out each empty line rule we're still missing. If you implement this you don't need to (and probably shouldn't) fix everything at once but you can implement any part of it (as long as it doesn't decrease the similarity index).

@konstin konstin added formatter Related to the formatter help wanted Contributions especially welcome labels Aug 21, 2023
@MichaReiser MichaReiser added this to the Formatter: Beta milestone Aug 22, 2023
@konstin konstin self-assigned this Sep 6, 2023
konstin added a commit that referenced this issue Sep 11, 2023
## 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter help wanted Contributions especially welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants