Support `fmt: skip` on compound statements by MichaReiser · Pull Request #6593 · astral-sh/ruff

MichaReiser · 2023-08-15T12:10:30Z

Summary

This PR adds support for trailing fmt: skip comments suppressing the formatting of a compound statement's clause header (if, elif, try...)

Test Plan

I added new test cases

MichaReiser · 2023-08-15T12:10:43Z

Current dependencies on/for this PR:

main
- PR Use single lookup for leading, dangling, and trailing comments #6589
  - PR Support fmt: skip for simple-statements and decorators #6561
    - PR Support fmt: skip on compound statements #6593 👈

This comment was auto-generated by Graphite.

crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__fmtskip8.py.snap

crates/ruff_python_formatter/src/verbatim.rs

MichaReiser · 2023-08-15T12:31:25Z

crates/ruff_python_formatter/src/statement/stmt_class_def.rs

+        if SuppressionKind::has_skip_comment(trailing_definition_comments, f.context().source()) {
+            SuppressedClauseHeader::Class(item).fmt(f)?;
+        } else {
+            write!(f, [text("class"), space(), name.format()])?;


The pattern is always the same. Test if there's a trailing skip comment and, if so, call SuppressedClauseHeader. The enclosing node remains responsible for formatting the body and the trailing comments.

We could wrap this in a ClauseHeader struct but it would result in the same level of nesting because it would need to accept a Format object to write the content. It may be worth adding considering that @tjkuson needs it in #6592 to collapse ... bodies.

github-actions · 2023-08-15T12:42:13Z

PR Check Results

Benchmark

Linux

group                                      main                                    pr
-----                                      ----                                    --
formatter/large/dataset.py                 1.03      3.9±0.28ms    10.5 MB/sec     1.00      3.8±0.35ms    10.8 MB/sec
formatter/numpy/ctypeslib.py               1.01   776.0±37.22µs    21.5 MB/sec     1.00   769.0±70.03µs    21.7 MB/sec
formatter/numpy/globals.py                 1.00     77.8±7.67µs    37.9 MB/sec     1.06     82.2±5.34µs    35.9 MB/sec
formatter/pydantic/types.py                1.01  1481.6±220.30µs    17.2 MB/sec    1.00  1469.3±93.90µs    17.4 MB/sec
linter/all-rules/large/dataset.py          1.00     11.9±0.44ms     3.4 MB/sec     1.06     12.6±0.71ms     3.2 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.2±0.10ms     5.3 MB/sec     1.03      3.2±0.12ms     5.1 MB/sec
linter/all-rules/numpy/globals.py          1.00   454.6±18.04µs     6.5 MB/sec     1.06   482.9±41.58µs     6.1 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.2±0.22ms     4.1 MB/sec     1.06      6.6±0.33ms     3.8 MB/sec
linter/default-rules/large/dataset.py      1.00      6.5±0.32ms     6.2 MB/sec     1.01      6.6±0.31ms     6.2 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1344.7±55.15µs    12.4 MB/sec     1.06  1424.2±76.70µs    11.7 MB/sec
linter/default-rules/numpy/globals.py      1.00    183.9±7.88µs    16.0 MB/sec     1.01   184.8±10.45µs    16.0 MB/sec
linter/default-rules/pydantic/types.py     1.00      2.9±0.14ms     8.7 MB/sec     1.01      3.0±0.15ms     8.6 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      3.7±0.02ms    11.0 MB/sec    1.01      3.7±0.03ms    10.9 MB/sec
formatter/numpy/ctypeslib.py               1.00    739.7±7.58µs    22.5 MB/sec    1.01    747.6±9.00µs    22.3 MB/sec
formatter/numpy/globals.py                 1.00     77.2±1.14µs    38.2 MB/sec    1.01     78.2±2.17µs    37.7 MB/sec
formatter/pydantic/types.py                1.00  1514.5±14.79µs    16.8 MB/sec    1.02  1539.5±81.10µs    16.6 MB/sec
linter/all-rules/large/dataset.py          1.01     13.1±0.06ms     3.1 MB/sec    1.00     12.9±0.10ms     3.1 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.6±0.04ms     4.6 MB/sec    1.00      3.6±0.02ms     4.6 MB/sec
linter/all-rules/numpy/globals.py          1.00    380.0±6.09µs     7.8 MB/sec    1.00    378.8±5.13µs     7.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.8±0.06ms     3.7 MB/sec    1.00      6.8±0.11ms     3.7 MB/sec
linter/default-rules/large/dataset.py      1.02      7.2±0.05ms     5.6 MB/sec    1.00      7.1±0.04ms     5.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1488.4±10.67µs    11.2 MB/sec    1.00   1488.6±9.56µs    11.2 MB/sec
linter/default-rules/numpy/globals.py      1.00    154.4±1.57µs    19.1 MB/sec    1.00    154.8±3.34µs    19.1 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.2±0.02ms     8.0 MB/sec    1.00      3.2±0.02ms     8.0 MB/sec

MichaReiser · 2023-08-15T16:44:14Z

@charliermarsh The PR has two commits:

Repeats the code to check whether there's a suppression comment. Plain and simple but with some repetitive code.
Introduces a new clause_header formatter that checks the suppression and formatting the :, the trailing colon comment, and any potential leading comments.

I'm undecided whether 2 is over-engineer. Wdyt?

MichaReiser · 2023-08-15T16:45:34Z

crates/ruff_python_formatter/src/other/except_handler_except_handler.rs

+                clause_header(
+                    ClauseHeader::ExceptHandler(item),
+                    dangling_comments,
+                    &format_with(|f| {


The formatting of the case header has been moved into the format_with. The clause_header implementation now takes care of writing the : and the trailing colon comments.

MichaReiser · 2023-08-15T16:46:03Z

crates/ruff_python_formatter/src/statement/clause.rs

The ClauseHeader abstractin could potentially also be useful in placement.rs

charliermarsh · 2023-08-15T17:02:36Z

Haha yeah, this is a close one, but I like what you have in the second commit, it doesn't feel over-engineered to me and is more declarative + feels very consistent with the other design patterns in the formatter.

crates/ruff_python_formatter/src/statement/clause.rs

MichaReiser mentioned this pull request Aug 15, 2023

Use single lookup for leading, dangling, and trailing comments #6589

Merged

MichaReiser mentioned this pull request Aug 15, 2023

Support fmt: skip for simple-statements and decorators #6561

Merged

MichaReiser commented Aug 15, 2023

View reviewed changes

crates/ruff_python_formatter/tests/snapshots/black_compatibility@simple_cases__fmtskip8.py.snap Show resolved Hide resolved

MichaReiser commented Aug 15, 2023

View reviewed changes

crates/ruff_python_formatter/src/verbatim.rs Outdated Show resolved Hide resolved

MichaReiser commented Aug 15, 2023

View reviewed changes

MichaReiser force-pushed the fmt-skip-statement-and-decorators branch from ef86a00 to 8215d9d Compare August 15, 2023 14:20

MichaReiser force-pushed the fmt-skip-clause-header branch 2 times, most recently from e008ba2 to b9667c7 Compare August 15, 2023 16:42

MichaReiser commented Aug 15, 2023

View reviewed changes

MichaReiser force-pushed the fmt-skip-statement-and-decorators branch from 8215d9d to 247979d Compare August 15, 2023 17:05

MichaReiser force-pushed the fmt-skip-clause-header branch 2 times, most recently from 89f9f04 to ca611a8 Compare August 15, 2023 17:11

MichaReiser added the formatter Related to the formatter label Aug 16, 2023

MichaReiser force-pushed the fmt-skip-statement-and-decorators branch from 247979d to 142d47b Compare August 16, 2023 06:07

MichaReiser force-pushed the fmt-skip-clause-header branch 2 times, most recently from 9fc3fd4 to 6fc6c13 Compare August 16, 2023 06:10

MichaReiser marked this pull request as ready for review August 16, 2023 06:10

MichaReiser requested a review from charliermarsh August 16, 2023 06:10

MichaReiser mentioned this pull request Aug 15, 2023

Formatter: Suppression comments #5336

Closed

6 tasks

MichaReiser force-pushed the fmt-skip-clause-header branch 2 times, most recently from 47988c9 to f20577f Compare August 16, 2023 06:36

charliermarsh approved these changes Aug 16, 2023

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

MichaReiser force-pushed the fmt-skip-statement-and-decorators branch from 142d47b to e2de436 Compare August 17, 2023 05:49

Base automatically changed from fmt-skip-statement-and-decorators to main August 17, 2023 05:58

Support fmt: skip on clause headers

ffa81b1

Introduce clause_header builder

c8b4a9e

MichaReiser force-pushed the fmt-skip-clause-header branch from f20577f to c8b4a9e Compare August 17, 2023 05:59

MichaReiser enabled auto-merge (squash) August 17, 2023 05:59

MichaReiser merged commit fa7442d into main Aug 17, 2023

MichaReiser deleted the fmt-skip-clause-header branch August 17, 2023 06:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Comments

Support `fmt: skip` on compound statements#6593

Support `fmt: skip` on compound statements#6593
MichaReiser merged 2 commits intomainfrom
fmt-skip-clause-header

MichaReiser commented Aug 15, 2023

Uh oh!

MichaReiser commented Aug 15, 2023

Uh oh!

Uh oh!

Uh oh!

MichaReiser Aug 15, 2023 •

edited

Loading

Uh oh!

github-actions bot commented Aug 15, 2023 •

edited

Loading

Uh oh!

MichaReiser commented Aug 15, 2023

Uh oh!

MichaReiser Aug 15, 2023

Uh oh!

MichaReiser Aug 15, 2023

Uh oh!

charliermarsh commented Aug 15, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Comments

Conversation

MichaReiser commented Aug 15, 2023

Summary

Test Plan

Uh oh!

MichaReiser commented Aug 15, 2023

Uh oh!

Uh oh!

Uh oh!

MichaReiser Aug 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Aug 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Check Results

Benchmark

Linux

Windows

Uh oh!

MichaReiser commented Aug 15, 2023

Uh oh!

MichaReiser Aug 15, 2023

Choose a reason for hiding this comment

Uh oh!

MichaReiser Aug 15, 2023

Choose a reason for hiding this comment

Uh oh!

charliermarsh commented Aug 15, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

MichaReiser Aug 15, 2023 •

edited

Loading

github-actions bot commented Aug 15, 2023 •

edited

Loading