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: allow_empty_first_line_before_new_block_or_comment preview style #8893

Closed
Tracked by #8678
MichaReiser opened this issue Nov 29, 2023 · 5 comments
Closed
Tracked by #8678
Labels
formatter Related to the formatter preview Related to preview mode features

Comments

@MichaReiser
Copy link
Member

Implement Black's allow_empty_first_line_before_new_block_or_comment as a Ruff preview style.

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

    # Here we go
    if x:

        # This is also now fine
        a = 123

    else:
        # But not necessary
        a = 123

Notice how black now allows empty lines before the comments. Ruff seems to support this sometimes.

Goal: Implement the new formatting behind the preview flag and import the Black tests.

@MichaReiser MichaReiser added formatter Related to the formatter preview Related to preview mode features labels Nov 29, 2023
@MichaReiser MichaReiser added this to the Formatter: Stable milestone Nov 29, 2023
@charliermarsh
Copy link
Member

Heads up: it looks like this was changed to always allow a blank line at the top of the block, unless it's immediately followed by a docstring. See: psf/black#4060.

@MichaReiser
Copy link
Member Author

MichaReiser commented Dec 22, 2023

I see the motivation behind this change but it can lead to unintentional deviations that outweigh the benefit of the change.

Let's say you start with

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

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

        # This is also now fine
        a = 123

And you refactor the code to remove the no_longer_needed branch

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

    # Here we go
    if x:

        # This is also now fine
        a = 123

and you now hit save. Without this change, the comment and a = 123 gets moved directly under the if x: condition which is what I would expect and gives us consistent formatting.

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

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

This would no longer happen if we implement the change. Instead, I have to remove the line above the comment manually. While I see that being able to make an exception might help with readability in few cases, removing the new line is the desired behavior in most cases.

I opened an issue in the Black repository to share our feedback with them.

@MichaReiser MichaReiser closed this as not planned Won't fix, can't repro, duplicate, stale Dec 22, 2023
@vaughnkoch
Copy link

Hi, thanks for building Ruff!

Given that Black is allowing newlines after a block opening line in the 2024 preview style, based on psf/black#4043, does Ruff plan to reopen this issue and conform to Black's intended style?

Personally, I place a high premium on code readability, and while I don't always add blank lines after a block opening line, it can often make the code a lot clearer, as demonstrated here: psf/black#902 (comment)

I would love to see this reopened and addressed. Thanks for considering!

note: I prefer not to comment on closed issues, but this seems like the most appropriate place to comment. Please let me know if there's a better way to provide feedback :)

@MichaReiser
Copy link
Member Author

Hy @vaughnkoch

We share your concern for readability. That's the main purpose of a code formatter, isn't it.

We have some concerns with the preview style as it is defined by black and decided not to ship it as part of Ruff's upcoming new stable style (which this issue is tracking). It doesn't mean we don't care. It's just that we aren't convinced by the heuristic and want to avoid changing styles too often.

My personal take on Black's blank line rules is that they're too opinionated and I could see it as an option that we explore with simpler blank line rules in the future, but that requires taking a holistic look at all of Ruff's/Black's blank line rules and find ways to simplify them.

I recommend opening a new issue that we can use for future style discussions.

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

No branches or pull requests

3 participants