[Python Formatter] Preserve empty lines
Summary
https://github.com/astral-sh/ruff/issues/9958 reported that code like
b = """
This looks like a docstring but is not in a notebook because notebooks can't be imported as a module.
Ruff should leave it as is
""";
a = "another normal string"
gets formatted as
b = """
This looks like a docstring but is not in a notebook because notebooks can't be imported as a module.
Ruff should leave it as is
""";
a = "another normal string"
This is unexpected and the expected behaviour of the formatter should preserve the empty lines between b and a.
What
- Modified
lines_aftersuch that it respects the semicolon. In my opinion, the extension makes a lot of sense in this function as it should not break counting of lines due to a semicolon - Test call sides
- I intentionally did not modify
lines_beforeas alone standing;'s are not allowed by the Python syntax anyway, e.g.b = """ This looks like a docstring but is not in a notebook because notebooks can't be imported as a module. Ruff should leave it as is """; ; a = "another normal string"
Test Plan
- Correct tests
- Add tests
Closes https://github.com/astral-sh/ruff/issues/9958.
ruff-ecosystem results
Linter (stable)
✅ ecosystem check detected no linter changes.
Linter (preview)
✅ ecosystem check detected no linter changes.
Formatter (stable)
✅ ecosystem check detected no format changes.
Formatter (preview)
✅ ecosystem check detected no format changes.
Some formatting has changed in the ruff-ecosystem. Seems like the backslash("\") is not recognized correctly with this change. Probably lines_after_ignoring_end_of_line_trivia needs to be adapted such that it interprets SimpleTokenKind::Continuation correctly. I will look into that.
Thank you for working on this! I agree with Micha that blank lines are hard to get right even if it seems to be working. Even if the implementation work in isolation, it breaks when mixed with other things (speaking from personal experience ;)). As suggested, testing this out thoroughly would be really helpful for us to evaluate the change.
I'm surprised this never occurs in the current ecosystem checks. Is there a project that uses this syntax that we could test on for a real world case?
Hi @dhruvmanila and @MichaReiser, thank you both for your review. I agree. I rethought my current approach of extending lines_after_ignoring_end_of_line_trivia and decided to extend lines_after instead. Note that I intentionally did not extend lines_before. Find details in the PR description. This new approach should not mess up any corner cases, as it does not touches the current combination of lines_after and lines_before. @MichaReiser, do you mind to re-review? Thank you.