Visitar URL original
Fix `# fmt: skip` combined with `# fmt: off` / `# yapf: disable` in the same comment by fizzbeom · Pull Request #5665 · psf/black · GitHub
Skip to content

Fix # fmt: skip combined with # fmt: off / # yapf: disable in the same comment - #5665

Open
fizzbeom wants to merge 5 commits into
psf:mainfrom
fizzbeom:fix/fmt-skip-with-fmt-off-in-same-comment
Open

fizzbeom wants to merge 5 commits into
psf:mainfrom
fizzbeom:fix/fmt-skip-with-fmt-off-in-same-comment

Conversation

@fizzbeom

@fizzbeom fizzbeom commented Nov 6, 2026

Copy link
Copy Markdown

Description

A comment that contains both an fmt: off directive and # fmt: skip, such as
# yapf: disable # fmt: skip (common when migrating from yapf), is handled
inconsistently. generate_ignored_nodes() already treats it as # fmt: skip, but
the rest of the conversion still treats it as # fmt: off:

  1. As a trailing comment, the line is formatted anyway, while # noqa # fmt: skip
    is left alone:

    x = [ 1,2 ]  # yapf: disable  # fmt: skip

    becomes x = [1, 2] # yapf: disable # fmt: skip.

  2. As an own-line comment inside brackets, Black crashes with "produced different
    code on the second pass", because the comment is emitted twice and grows on
    every run:

    x = [
        # fmt: off  # fmt: skip
        1,
    ]

This change lets # fmt: skip win consistently, so such a comment behaves the same
as a plain # fmt: skip. I found it by inserting fmt comments into the existing
test cases and running Black with its equivalence and stability checks.

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?

This PR was generated with AI (Claude Code).

fizzbeom and others added 3 commits November 6, 2026 08:59
A comment like `# yapf: disable  # fmt: skip` contains both an fmt: off
directive and fmt: skip. generate_ignored_nodes() already treats such a
comment as fmt: skip, but the rest of the conversion still treated it as
fmt: off. As a trailing comment it was rejected (a trailing fmt: off is
invalid), so the line was formatted anyway; as an own-line comment inside
brackets the comment was emitted twice and grew on every run, failing
the stability check.

Let fmt: skip win consistently.

@chrikrah chrikrah left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fizzbeom nothing here blocks a merge from my side at 446b2ea. Reverting comments.py with your case file kept fails the new case:

# black from the exported 446b2ea tree, Python 3.12
$ python -m pytest tests -q -p no:cacheprovider --ignore=tests/test_blackd.py
620 passed, 3 skipped, 31 subtests passed
# src/black/comments.py reverted to b4f1fca, test data kept
$ python -m pytest tests/test_format.py -q -p no:cacheprovider -k fmt
FAILED tests/test_format.py::test_simple_format[fmtskip_with_fmt_off_in_same_comment]
1 failed, 66 passed, 248 deselected

# not run: tests/test_blackd.py (needs the d extra)

If a later push touches comments.py, I can rerun the revert check against it.

Add unformatted lines after the trailing and own-line cases so the test
shows the following code is still formatted.
…mt-off-in-same-comment

# Conflicts:
#	CHANGES.md
@github-actions

Copy link
Copy Markdown
Contributor

diff-shades results comparing this PR (c336126) to main (0b5c7a4):

--preview style: no changes

--stable style: no changes


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

This branch has not been deployed

No deployments
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.

3 participants