Track formatting all comments #6178

konstin · 2023-07-30T13:44:11Z

We currently don't format all comments as match statements are not yet implemented. We can work around this for the top level match statement by setting them manually formatted but the mocked-out top level match doesn't call into its children so they would still have unformatted comments

github-actions · 2023-07-30T14:19:18Z

PR Check Results

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      8.2±0.09ms     4.9 MB/sec    1.00      8.3±0.10ms     4.9 MB/sec
formatter/numpy/ctypeslib.py               1.01  1647.0±38.73µs    10.1 MB/sec    1.00  1633.4±11.28µs    10.2 MB/sec
formatter/numpy/globals.py                 1.02    187.6±7.25µs    15.7 MB/sec    1.00    184.5±2.58µs    16.0 MB/sec
formatter/pydantic/types.py                1.00      3.5±0.09ms     7.3 MB/sec    1.01      3.5±0.12ms     7.2 MB/sec
linter/all-rules/large/dataset.py          1.00     10.2±0.04ms     4.0 MB/sec    1.00     10.2±0.06ms     4.0 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      2.7±0.01ms     6.1 MB/sec    1.00      2.7±0.02ms     6.1 MB/sec
linter/all-rules/numpy/globals.py          1.00    383.0±0.87µs     7.7 MB/sec    1.00    382.2±0.66µs     7.7 MB/sec
linter/all-rules/pydantic/types.py         1.00      5.3±0.07ms     4.8 MB/sec    1.00      5.3±0.02ms     4.8 MB/sec
linter/default-rules/large/dataset.py      1.01      5.3±0.01ms     7.6 MB/sec    1.00      5.3±0.02ms     7.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01   1151.5±3.99µs    14.5 MB/sec    1.00   1144.3±2.90µs    14.6 MB/sec
linter/default-rules/numpy/globals.py      1.00    132.7±2.34µs    22.2 MB/sec    1.00    132.1±1.60µs    22.3 MB/sec
linter/default-rules/pydantic/types.py     1.00      2.4±0.01ms    10.6 MB/sec    1.00      2.4±0.02ms    10.7 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     11.9±0.29ms     3.4 MB/sec    1.01     12.0±0.28ms     3.4 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.3±0.07ms     7.3 MB/sec    1.03      2.3±0.13ms     7.1 MB/sec
formatter/numpy/globals.py                 1.00   255.3±11.24µs    11.6 MB/sec    1.01   257.0±12.29µs    11.5 MB/sec
formatter/pydantic/types.py                1.00      5.0±0.15ms     5.1 MB/sec    1.01      5.1±0.16ms     5.0 MB/sec
linter/all-rules/large/dataset.py          1.00     15.0±0.28ms     2.7 MB/sec    1.02     15.2±0.44ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.1±0.12ms     4.0 MB/sec    1.01      4.1±0.10ms     4.0 MB/sec
linter/all-rules/numpy/globals.py          1.00   512.6±14.51µs     5.8 MB/sec    1.01   515.7±17.90µs     5.7 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.8±0.18ms     3.3 MB/sec    1.01      7.8±0.16ms     3.3 MB/sec
linter/default-rules/large/dataset.py      1.00      8.1±0.15ms     5.0 MB/sec    1.01      8.2±0.13ms     5.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1697.9±37.91µs     9.8 MB/sec    1.01  1712.2±64.73µs     9.7 MB/sec
linter/default-rules/numpy/globals.py      1.00   195.9±11.93µs    15.1 MB/sec    1.02   199.6±10.26µs    14.8 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.6±0.07ms     7.0 MB/sec    1.00      3.6±0.09ms     7.1 MB/sec

MichaReiser · 2023-07-31T08:50:52Z

crates/ruff_python_formatter/src/lib.rs

@@ -180,6 +180,17 @@ impl Format<PyFormatContext<'_>> for NotYetImplemented {
        })?;

        f.write_element(FormatElement::Tag(Tag::EndVerbatim))?;
+
+        let comments = f.context().comments().clone();


You could implement your own PreorderVisitor that marks the comments of all children as formatted. We probably need something similar for fmt: skip anyway.

i'll wait until we have match statement formatting instead

MichaReiser · 2023-08-08T15:16:46Z

Current dependencies on/for this PR:

main
- PR Add enter and leave_node methods to Preoder visitor #6422
  - PR Add AnyNodeRef.visit_preorder #6426
    - PR Preserve dangling f-string comments #6441
      - PR Track formatting all comments #6178 👈

This comment was auto-generated by Graphite.

MichaReiser · 2023-08-08T15:23:42Z

crates/ruff_python_formatter/src/other/match_case.rs

@@ -24,7 +24,17 @@ impl FormatNodeRule<MatchCase> for FormatMatchCase {
            [
                text("case"),
                space(),
-                not_yet_implemented_custom_text("NOT_YET_IMPLEMENTED_Pattern"),
+                format_with(|f: &mut PyFormatter| {


This is intentionally ugly to make sure we remove we don't forget to remove the comment.mark_formatted when replacing this with pattern.format()

MichaReiser · 2023-08-08T15:24:17Z

Can I now approve my own PR 😅

konstin · 2023-08-08T15:30:16Z

¯\_(ツ)_/¯

konstin · 2023-08-08T20:26:17Z

so who'll merge?

MichaReiser · 2023-08-09T06:58:21Z

I can take care of merging since we have to coordinate with the downstream PRs. All I need is a thumbs up/down from you.

konstin · 2023-08-09T07:21:56Z

👍

We currently don't format all comments as match statements are not yet implemented. We can work around this for the top level match statement by setting them manually formatted but the mocked-out top level match doesn't call into its children so they would still have unformatted comments

MichaReiser · 2023-08-10T07:12:00Z

Graphite rebased this pull request as part of a merge.

MichaReiser · 2023-08-10T07:19:30Z

@MichaReiser merged this pull request with Graphite.

MichaReiser reviewed Jul 31, 2023

View reviewed changes

konstin assigned MichaReiser Aug 8, 2023

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

MichaReiser changed the base branch from main to any-node-ref-visit-preorder August 8, 2023 15:16

MichaReiser force-pushed the track_formatting_all_comments branch from d7bfa62 to a132a7c Compare August 8, 2023 15:16

This was referenced Aug 8, 2023

Add enter and leave_node methods to Preoder visitor #6422

Merged

Add AnyNodeRef.visit_preorder #6426

Merged

MichaReiser force-pushed the track_formatting_all_comments branch from a132a7c to 2f771c2 Compare August 8, 2023 15:19

MichaReiser reviewed Aug 8, 2023

View reviewed changes

MichaReiser marked this pull request as ready for review August 8, 2023 15:24

MichaReiser approved these changes Aug 8, 2023

View reviewed changes

MichaReiser mentioned this pull request Aug 9, 2023

FString formatting drops comments #6440

Closed

MichaReiser force-pushed the any-node-ref-visit-preorder branch from f6214dc to 06db26f Compare August 9, 2023 08:59

MichaReiser changed the base branch from any-node-ref-visit-preorder to fstring-comments August 9, 2023 08:59

MichaReiser force-pushed the track_formatting_all_comments branch from 2f771c2 to 20dabc8 Compare August 9, 2023 08:59

MichaReiser mentioned this pull request Aug 9, 2023

Preserve dangling f-string comments #6441

Merged

MichaReiser force-pushed the fstring-comments branch from 16e1d70 to 7d92f55 Compare August 10, 2023 06:17

MichaReiser force-pushed the track_formatting_all_comments branch from 20dabc8 to 290fff6 Compare August 10, 2023 06:17

MichaReiser force-pushed the fstring-comments branch from 7d92f55 to e69ff5d Compare August 10, 2023 07:00

Base automatically changed from fstring-comments to main August 10, 2023 07:11

MichaReiser force-pushed the track_formatting_all_comments branch from 290fff6 to dc6b84c Compare August 10, 2023 07:12

MichaReiser merged commit 39beeb6 into main Aug 10, 2023
16 checks passed

MichaReiser deleted the track_formatting_all_comments branch August 10, 2023 07:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Track formatting all comments #6178

Track formatting all comments #6178

konstin commented Jul 30, 2023

github-actions bot commented Jul 30, 2023 •

edited

Loading

MichaReiser Jul 31, 2023

konstin Aug 1, 2023

MichaReiser commented Aug 8, 2023 •

edited

Loading

MichaReiser Aug 8, 2023

MichaReiser commented Aug 8, 2023

konstin commented Aug 8, 2023 •

edited

Loading

konstin commented Aug 8, 2023

MichaReiser commented Aug 9, 2023

konstin commented Aug 9, 2023

MichaReiser commented Aug 10, 2023

MichaReiser commented Aug 10, 2023

Track formatting all comments #6178

Track formatting all comments #6178

Conversation

konstin commented Jul 30, 2023

github-actions bot commented Jul 30, 2023 • edited Loading

PR Check Results

Benchmark

Linux

Windows

MichaReiser Jul 31, 2023

Choose a reason for hiding this comment

konstin Aug 1, 2023

Choose a reason for hiding this comment

MichaReiser commented Aug 8, 2023 • edited Loading

MichaReiser Aug 8, 2023

Choose a reason for hiding this comment

MichaReiser commented Aug 8, 2023

konstin commented Aug 8, 2023 • edited Loading

konstin commented Aug 8, 2023

MichaReiser commented Aug 9, 2023

konstin commented Aug 9, 2023

MichaReiser commented Aug 10, 2023

MichaReiser commented Aug 10, 2023

github-actions bot commented Jul 30, 2023 •

edited

Loading

MichaReiser commented Aug 8, 2023 •

edited

Loading

konstin commented Aug 8, 2023 •

edited

Loading