fix(fmt: skip): respect skip directive on multi-line compound statements#5114
Closed
armorbreak001 wants to merge 1 commit into
Closed
fix(fmt: skip): respect skip directive on multi-line compound statements#5114armorbreak001 wants to merge 1 commit into
armorbreak001 wants to merge 1 commit into
Conversation
When is placed on the colon line of a multi-line class (or def/if) statement, Black still reformatted it because the sibling traversal in _generate_ignored_nodes_from_fmt_skip stopped at the first leaf containing a newline in its prefix (e.g., the closing of a multi-line type parameter list). Root cause: line 646-647 unconditionally set prev_sibling to parent.prev_sibling for NEWLINE leaves with no prev_sibling, causing the function to take the per-sibling traversal path (which stops at newlines) instead of the suite-sibling collection path (which correctly collects ALL previous siblings of the suite node). Fix: skip the prev_sibling override when parent is a suite, so the elif branch that handles compound statements is reached instead.
|
Thanks for taking a look at this and for the quick fix @armorbreak001! I don't know the rules of this project, but in general I believe it would be a good idea to add a new test that reproduces the bug and is fixed by this change. |
Collaborator
|
Thanks @armorbreak001! Yes, please do add a new test case, as well as a changelog entry. |
cobaltt7
added a commit
to 17X61/black
that referenced
this pull request
May 12, 2026
cobaltt7
added a commit
that referenced
this pull request
May 12, 2026
* Preserve multiline headers with fmt skip * Add changelog entry for fmt skip fix * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Expand fmt skip multiline header tests * Add more test cases from #5114 and #5123 * Update comments.py --------- Co-authored-by: xtreme <psycheas@outlook.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: cobalt <61329810+cobaltt7@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #5112, closes #5117
fixes #5122, closes #5123
When
# fmt: skipis placed on the colon line of a multi-line class (or def/if) statement, Black still reformatted it — collapsing multi-line type parameters onto a single line.Root cause: In
_generate_ignored_nodes_from_fmt_skip(), line 647 unconditionally setsprev_sibling = parent.prev_siblingwhen the leaf (NEWLINE) has no previous sibling. This causes the function to take the per-sibling traversal path (which stops at any leaf containing a newline in its prefix) instead of the suite-sibling collection path (theelifbranch at line 738, which correctly collects ALL previous siblings of the suite node).For a multi-line class like:
The traversal starts at
:and collects],),:— then hits]whose prefix contains\n(end of line), stopping early. The entire class header (class Foo(Base[, type params) is never marked as ignored, so Black reformats it.Fix: Add
parent.type != syms.suitecondition on line 646 so that suite NEWLINEs fall through to theelifbranch that properly collects all statement header nodes.Test plan
# fmt: skipregression #5112 with the exact example from the bug report — now left unchanged# fmt: skipcases still work (assignments, one-line compounds, etc.)