Respect `fmt: skip` for compound statements on single line #20633

dylwil3 · 2025-09-29T17:20:12Z

Essentially the approach is to implement Format for a new struct FormatClause which is just a clause header and its body. We then have the information we need to see whether there is a skip suppression comment on the last child in the body and it all fits on one line.

The ruff test fixture (credit: mostly created by Claude) contains several lines that seem to crash black, see:

Crash with preview style fix_fmt_skip_in_one_liners for certain compound statements psf/black#4782
Crash with preview style fix_fmt_skip_in_one_liners for function with annotated parameter and # fmt: skip on one line psf/black#4781

and another line containing a deviation that I think is unintentional for black, see:

Preview style fix_fmt_skip_in_one_liners does not work for semi-colon separated statements in body of compound statement psf/black#4783

However, it's not clear whether we want the same behavior here? In this PR we have:

if True: print("this"); print("that") # fmt: skip

unchanged, but we do add a newline between the statements here (unlike Black):

print("this"); print("that") # fmt: skip

(see #11430 and #17331 .)

The other deviation from black is that we format the placement of the skip comment itself.

For review, it is simplest to go commit by commit.

github-actions · 2025-09-29T17:28:25Z

`ruff-ecosystem` results

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

crates/ruff_python_formatter/src/statement/clause.rs

MichaReiser · 2025-10-07T07:59:19Z

unchanged, but we do add a newline between the statements here (unlike Black):
print("this"); print("that") # fmt: skip

This seems unrelated to this PR as it is a pre-existing issue, right?

MichaReiser

This is great :) I suggest adding a few more tests with different comment placements. Just to make sure we handle them correctly.

crates/ruff_python_formatter/src/preview.rs

crates/ruff_python_formatter/src/statement/clause.rs

MichaReiser · 2025-10-07T08:14:08Z

crates/ruff_python_formatter/src/statement/stmt_class_def.rs

                        Ok(())
                    }),
+                    body,
+                    SuiteKind::Class,


Can we derive the SuiteKind from the ClauseHeader to reduce the number of arguments?

I don't think so because I think we have to be told whether we're the last suite in the statement (but I think that's the only missing info)

crates/ruff_python_formatter/src/statement/clause.rs

crates/ruff_python_formatter/tests/snapshots/format@fmt_skip__compound_one_liners.py.snap

dylwil3 · 2025-10-24T17:40:04Z

The CI failure here https://github.com/astral-sh/ruff/actions/runs/18664070057/job/53211233008?pr=20633 was caused by this PR uncovering a pre-existing bug. See #21065 and #21067 . Will rebase once that PR is merged and then continue working on this!

dylwil3 · 2025-10-27T20:56:09Z

Cut down a comment-related hydra head and more grew in its place. This snippet causes a panic because the indented comment is not marked as formatted. Annoyingly it is attached to bar() (because there is no orelse node).

for x in it: foo()
    # comment
else: bar() # fmt: skip

Converting to draft while I sort it out 😄

MichaReiser

Nice

crates/ruff_python_formatter/src/statement/clause.rs

) This PR adjusts the logic for skipping formatting so that a `fmt: skip` can affect multiple statements if they lie on the same line. Specifically, a `fmt: skip` comment will now suppress all the statements in the suite in which it appears whose range intersects the line containing the skip directive. For example: ```python x=[ '1' ];x=2 # fmt: skip ``` remains unchanged after formatting. (Note that compound statements are somewhat special and were handled in a previous PR - see #20633). Closes #17331 and #11430. Simplest to review commit by commit - the key diffs of interest are the commit introducing the core logic, and the diff between the snapshots introduced in the last commit (compared to the second commit). # Implementation On `main` we format a suite of statements by iterating through them. If we meet a statement with a leading or trailing (own-line)`fmt: off` comment, then we suppress formatting until we meet a `fmt: on` comment. Otherwise we format the statement using its own formatting rule. How are `fmt: skip` comments handled then? They are handled internally to the formatting of each statement. Specifically, calling `.fmt` on a statement node will first check to see if there is a trailing, end-of-line `fmt: skip` (or `fmt: off`/`yapf: off`), and if so then write the node with suppressed formatting. In this PR we move the responsibility for handling `fmt: skip` into the formatting logic of the suite itself. This is done as follows: - Before beginning to format the suite, we do a pass through the statements and collect the data of ranges with skipped formatting. More specifically, we create a map with key given by the _first_ skipped statement in a block and value a pair consisting of the _last_ skipped statement and the _range_ to write verbatim. - We iterate as before, but if we meet a statement that is a key in the map constructed above, we pause to write the associated range verbatim. We then advance the iterator to the last statement in the block and proceed as before. ## Addendum on range formatting We also had to make some changes to range formatting in order to support this new behavior. For example, we want to make sure that ```python <RANGE_START>x=1<RANGE_END>;x=2 # fmt: skip ``` formats verbatim, rather than becoming ```python x = 1;x=2 # fmt: skip ``` Recall that range formatting proceeds in two steps: 1. Find the smallest enclosing node containing the range AND that has enough info to format the range (so it may be larger than you think, e.g. a docstring has enclosing node given by the suite, not the string itself.) 2. Carve out the formatted range from the result of formatting that enclosing node. We had to modify (1), since the suite knows how to format skipped nodes, but nodes may not "know" they are skipped. To do this we altered the `visit_body` bethod of the `FindEnclosingNode` visitor: now we iterate through the statements and check for skipped ranges intersecting the format range. If we find them, we return without descending. The result is to consider the statement containing the suite as the enclosing node in this case.

dylwil3 added bug Something isn't working formatter Related to the formatter preview Related to preview mode features labels Sep 29, 2025

dylwil3 force-pushed the fmt-skip-one-liner branch 3 times, most recently from c695437 to aa2fd23 Compare October 6, 2025 20:34

dylwil3 commented Oct 6, 2025

View reviewed changes

crates/ruff_python_formatter/src/statement/clause.rs Show resolved Hide resolved

dylwil3 marked this pull request as ready for review October 6, 2025 20:44

dylwil3 requested a review from MichaReiser as a code owner October 6, 2025 20:44

dylwil3 mentioned this pull request Oct 6, 2025

Ruff 2026 Style Guide #20482

Open

31 tasks

MichaReiser reviewed Oct 7, 2025

View reviewed changes

ntBre mentioned this pull request Oct 13, 2025

Update Black tests #20794

Merged

dylwil3 force-pushed the fmt-skip-one-liner branch from aa2fd23 to 00d2823 Compare October 20, 2025 19:59

dylwil3 force-pushed the fmt-skip-one-liner branch from 813abca to 6e1bb22 Compare October 27, 2025 15:22

dylwil3 marked this pull request as draft October 27, 2025 16:19

dylwil3 mentioned this pull request Nov 10, 2025

Adjust own-line comment placement between branches #21185

Merged

dylwil3 removed the preview Related to preview mode features label Nov 10, 2025

dylwil3 force-pushed the fmt-skip-one-liner branch 3 times, most recently from b5cde38 to 417dd01 Compare November 17, 2025 15:04

dylwil3 added 6 commits November 17, 2025 10:43

add fixture for compound one liners

bff6208

add convenience methods for ClauseHeader

5525845

add Clause and implement formatting logic

39e591f

use clause instead of clause_header and clause_body

9043904

make ClauseBody private since it is unused outside module

e9b535a

add snapshots

79c3d6b

dylwil3 added 4 commits November 17, 2025 10:43

add new test and snapshot

13a5ae1

add fixture with comment in clause header

aac5fe3

handle comments in clause header

f7c958e

snapshots

fd13d35

dylwil3 force-pushed the fmt-skip-one-liner branch from 417dd01 to fd13d35 Compare November 17, 2025 16:43

dylwil3 marked this pull request as ready for review November 17, 2025 16:48

dylwil3 requested a review from MichaReiser November 17, 2025 16:48

MichaReiser approved these changes Nov 18, 2025

View reviewed changes

crates/ruff_python_formatter/src/statement/clause.rs Outdated Show resolved Hide resolved

crates/ruff_python_formatter/src/statement/clause.rs Outdated Show resolved Hide resolved

crates/ruff_python_formatter/src/statement/clause.rs Outdated Show resolved Hide resolved

dylwil3 added 3 commits November 18, 2025 07:56

remove unnecessary comment

9639447

inline formatclause data structures

0453382

remove expect by passing last_child_in_clause along

36c8d17

dylwil3 merged commit 62343a1 into astral-sh:main Nov 18, 2025
36 checks passed

dylwil3 deleted the fmt-skip-one-liner branch November 18, 2025 18:02

BrewTestBot mentioned this pull request Nov 21, 2025

ruff 0.14.6 Homebrew/homebrew-core#255415

Merged

dylwil3 mentioned this pull request Dec 22, 2025

Respect fmt: skip for multiple statements on same logical line #22119

Merged

Respect fmt: skip for compound statements on single line #20633

Respect fmt: skip for compound statements on single line #20633

Uh oh!

Conversation

dylwil3 commented Sep 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Sep 29, 2025 • edited by astral-sh-bot bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ruff-ecosystem results

Formatter (stable)

Formatter (preview)

Uh oh!

Uh oh!

MichaReiser commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MichaReiser Oct 7, 2025

Choose a reason for hiding this comment

Uh oh!

dylwil3 Oct 20, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dylwil3 commented Oct 24, 2025

Uh oh!

dylwil3 commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MichaReiser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Respect `fmt: skip` for compound statements on single line #20633

Respect `fmt: skip` for compound statements on single line #20633

dylwil3 commented Sep 29, 2025 •

edited

Loading

github-actions bot commented Sep 29, 2025 •

edited by astral-sh-bot bot

Loading

`ruff-ecosystem` results

MichaReiser commented Oct 7, 2025 •

edited

Loading

dylwil3 commented Oct 27, 2025 •

edited

Loading