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

allow_empty_first_line_before_new_block_or_comment can lead to inconsistent formatting #4121

Closed
MichaReiser opened this issue Dec 22, 2023 · 3 comments
Labels
C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. T: style What do we want Blackened code to look like?

Comments

@MichaReiser
Copy link

MichaReiser commented Dec 22, 2023

Describe the style change

I am working on Ruff's formatter and implementing Black's preview styles. We reviewed the allow_empty_first_line_before_new_block_or_comment preview style and decided not to implement it because it leads to inconsistent formatting after moving or deleting code or requires more manual intervention.

Examples when allow_empty_first_line_before_new_block_or_comment is enabled

I work on a refactoring and start with the following code:

def foo():
    """
    Docstring
    """

    # Here we go
    if x:
        if no_longer_needed:
            print("Some complex statements")

        # This is also now fine
        a = 123

And I delete the no_longer_needed branch:

def foo():
    """
    Docstring
    """

    # Here we go
    if x:

        # This is also now fine
        a = 123

Black removes the empty line above the comment when the allow_empty_first_line_before_new_block_or_comment style is disabled.

def foo():
    """
    Docstring
    """

    # Here we go
    if x:
        # This is also now fine
        a = 123

Black doesn't remove the empty line when enabling allow_empty_first_line_before_new_block_or_comment, which either results in an unintended empty line above the comment (inconsistency) or that I have to intervene and remove the empty line manually. This feels like something I would expect a formatter to do for me (at the cost that having empty lines before comments isn't possible)

Desired style

To keep the non-preview formatting for comments at the start of a block.

Additional context

@MichaReiser MichaReiser added the T: style What do we want Blackened code to look like? label Dec 22, 2023
@MichaReiser MichaReiser changed the title Inconsistent formatting after code-refactors with allow_empty_first_line_before_new_block_or_comment allow_empty_first_line_before_new_block_or_comment can lead to inconsistent formatting Dec 22, 2023
@hauntsaninja hauntsaninja added the C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. label Dec 28, 2023
@hauntsaninja
Copy link
Collaborator

Thanks for the feedback — and generally for all your engagement!

Note we're still in the process of changing the logic here, although I suspect you might like it even less because the changes generally reduce normalisation (except for some missed docstring cases). There's more discussion in #4043.

tldr; the consistency benefits don't feel worth the readability cost in the cases removing newlines is worse + the community has pushed back quite a bit against the removal of blank lines.

There might be a set of heuristics that works well, but complicated heuristics don't really fit well with Black's philosophy. I also want to say I remember that the early versions of Black normalised all blank lines and this was one of the first things Łukasz decided Black should compromise on.

@MichaReiser
Copy link
Author

Thanks for the kind words and I'm glad you consider the feedback useful. I've been worried that my feedback isn't useful and that I lack the history context.

I suspect you might like it even less because the changes

You'll be surprised but I find the Black empty lines rules very strict as someone coming from Prettier (JS) and Rustfmt where the tools only enforce a certain maximum of empty lines rather than a very specific number of empty lines depending on the context.

The empty lines rule are one place where I find Black's heuristics fairly complicated (and the new preview style adds another exception where empty lines are "surprisingly" preserved).

I do prefer the removal of empty lines in this specific context. Maybe again, because that's what I'm used to from Prettier and Rustfmt. Both tools remove empty lines between the beginning of a block and a comment.

@JelleZijlstra
Copy link
Collaborator

I also like removing the blank line here, but we got a lot of feedback from people who wanted to keep blank lines, and some specifically flagged blank lines before comments. I'd be open to proposals for heuristics to improve more leading blank lines, but for now let's stick with the current preview style.

@JelleZijlstra JelleZijlstra closed this as not planned Won't fix, can't repro, duplicate, stale Jan 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: preview style Issues with the preview and unstable style. Add the name of the responsible feature in the title. T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

3 participants