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: Preserve leading empty lines of indented classes and functions #6066

Closed
Tracked by #6069 ...
konstin opened this issue Jul 25, 2023 · 7 comments · Fixed by #6206
Closed
Tracked by #6069 ...

Formatter: Preserve leading empty lines of indented classes and functions #6066

konstin opened this issue Jul 25, 2023 · 7 comments · Fixed by #6206
Assignees
Labels
formatter Related to the formatter

Comments

@konstin
Copy link
Member

konstin commented Jul 25, 2023

While the behavior for empty lines before top level classes and functions looks correct, indented classes and function can gain or lose their leading newlines

black:

def f():
    ...
    # comment

    class Meta:
        pass

    if True:

        def register_type():
            pass

ours:

def f():
    ...

    # comment

    class Meta:
        pass

    if True:
        def register_type():
            pass
@konstin konstin added the formatter Related to the formatter label Jul 25, 2023
@charliermarsh charliermarsh self-assigned this Jul 31, 2023
@charliermarsh
Copy link
Member

I'm working on this.

@charliermarsh
Copy link
Member

charliermarsh commented Jul 31, 2023

I've noticed a few other incompatibilities after looking back at my old implementation:

  1. The first entry does need a separator if it's a class or function within a nested
    body that isn't a class or function. Also, Black allows one blank line between a
    class and its docstring (the same does not apply to functions).
  2. If a function or class has a leading comment, then a blank line, we don't want to
    add one before the comment. The enforcement around
    is_last_function_or_class_definition || is_current_function_or_class_definition
    should be relative to the function or class definition, not its leading comments.
  3. We're not inserting newlines after imports.
  4. Black requires one newline after a class docstring.

(2) will cause problems with the current approach, I think. As an example, given:

x = 1
# comment

def f():
    pass

Black wants:

x = 1
# comment


def f():
    pass

Whereas we do:

x = 1


# comment

def f():
    pass

That means the code that handles newlines-after-comments needs to now be aware of the thing it's commenting, the level of nesting, etc.

@charliermarsh
Copy link
Member

Perhaps the "easiest" way to get this to work would be to make # comment a trailing comment on the previous node, not a leading comment on the function, if there's at least one blank line between the comment and the function?

@charliermarsh
Copy link
Member

(I will own all of those deviations.)

@MichaReiser
Copy link
Member

(I will own all of those deviations.)

Are you looking for input or are these comment's meant as documentation?

@charliermarsh
Copy link
Member

The deviations are just meant as documentation, not expecting any input! Unless you have a quick reaction to the idea I suggested in ‘Perhaps the "easiest" way to get this to work…’, but I haven’t spent time validating that yet.

@MichaReiser
Copy link
Member

MichaReiser commented Jul 31, 2023

Perhaps the "easiest" way to get this to work would be to make # comment a trailing comment on the previous node, not a leading comment on the function, if there's at least one blank line between the comment and the function?

I think that could work well. Not sure if implementing custom placement is necessarily easy.

@MichaReiser MichaReiser added this to the Formatter: Alpha milestone Jul 31, 2023
charliermarsh added a commit that referenced this issue Aug 1, 2023
## Summary

This PR ensures that if a function or class is the first statement in a
nested suite that _isn't_ a function or class body, we insert a leading
newline.

For example, given:

```python
def f():
    if True:

        def register_type():
            pass
```

We _want_ to preserve the newline, whereas today, we remove it.

Note that this only applies when the function or class doesn't have any
leading comments.

Closes #6066.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants