Skip to content

Preserve blank lines before fmt-off classes#5238

Merged
cobaltt7 merged 2 commits into
psf:mainfrom
sjh9714:codex/20260715-5225-fmt-off-blank-lines
Jul 21, 2026
Merged

Preserve blank lines before fmt-off classes#5238
cobaltt7 merged 2 commits into
psf:mainfrom
sjh9714:codex/20260715-5225-fmt-off-blank-lines

Conversation

@sjh9714

@sjh9714 sjh9714 commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #5225.

When a top-level class starts inside a # fmt: off block after an import, the
converted block looks like a standalone comment to the empty-line tracker. This caused
Black to keep only one blank line instead of the two required before a top-level class.

Preserve the converted block's original class token when deciding import separation.
The change is limited to a new preview feature, so stable formatting remains unchanged,
and a focused fixture covers the reported input.

Testing

  • .venv/bin/pytest tests/test_format.py -k preview_fmt_off_class_blank_lines -q
  • .venv/bin/pytest tests/test_format.py -q
  • env -u NO_COLOR .venv/bin/pytest -q
  • env -u NO_COLOR .venv/bin/pre-commit run mypy --files src/black/lines.py src/black/mode.py
  • env -u NO_COLOR .venv/bin/tox -e run_self
  • .venv/bin/tox -e generate_schema

env -u NO_COLOR .venv/bin/pre-commit run --all-files passed every hook except
repository-wide mypy, which reports an unchanged action/main.py tomllib
ignore/stub mismatch under Python 3.14. The targeted mypy hook above passes for both
changed Python modules.

Note: I used Codex while preparing this change, reviewed the final diff, and ran the
checks listed above locally.

Checklist - did you ...

  • Implement any code style changes under the --preview style, following the stability policy?
  • Add an entry in CHANGES.md if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

diff-shades results comparing this PR (c8f5587) to main (fcab648):

--preview style: no changes

--stable style: no changes


What is this? | Workflow run | diff-shades documentation

@cobaltt7

Copy link
Copy Markdown
Collaborator

What logic normally handles newlines before classes? Why isn't it triggering here? Would it make morse sense to update it?

@sjh9714

sjh9714 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Good question — I dug into where this comes from.

The logic that normally handles blank lines before classes is EmptyLineTracker._maybe_empty_lines, which routes any line whose is_decorator/is_def/is_class is true to _maybe_empty_lines_for_class_or_def (src/black/lines.py:1111).

It doesn't trigger here because normalize_fmt_off collapses the whole # fmt: off region into a single STANDALONE_COMMENT leaf (convert_one_fmt_off_pair in src/black/comments.py), keeping the original first token only in fmt_pass_converted_first_leaf. So the line's first leaf is a STANDALONE_COMMENT, not a NAME classis_class is False, the class/def dispatch is skipped, and the line falls through to the top-level import rule just below (src/black/lines.py:1128), which returns 1 blank line. That's exactly the reported behavior.

On updating the normal logic instead: I considered teaching is_class (or the dispatch) to see through fmt-pass-converted lines, but is_class/is_def are consulted in many other places (stub-class handling, previous_defs tracking, docstring logic), and routing an opaque fmt:off block into _maybe_empty_lines_for_class_or_def would let other rules act on content that # fmt: off promises to leave alone. The two import rules directly below already special-case is_fmt_pass_converted(first_leaf_matches=is_import), so I kept the fix in the same shape and place: a narrow guard for the import → fmt-converted-class boundary.

That said, if you'd rather see this handled inside _maybe_empty_lines_for_class_or_def (or generalized beyond the after-imports case), I'm happy to rework it that way.

@cobaltt7

Copy link
Copy Markdown
Collaborator

Sounds good. Thank you for investigating & fixing this!

@cobaltt7 cobaltt7 closed this Jul 21, 2026
@cobaltt7

Copy link
Copy Markdown
Collaborator

Whoops, wrong button

@cobaltt7 cobaltt7 reopened this Jul 21, 2026
@cobaltt7
cobaltt7 merged commit 7aaaeb2 into psf:main Jul 21, 2026
92 of 94 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inconsistent number of lines after import

2 participants