Skip to content

Track formatted comments#4979

Merged
MichaReiser merged 2 commits intomainfrom
skip-formatted-comments
Jun 9, 2023
Merged

Track formatted comments#4979
MichaReiser merged 2 commits intomainfrom
skip-formatted-comments

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Jun 9, 2023

Summary

This PR removes the debug-only condition from SourceComment::formatted and now uses it to track whether a comment has already been formatted and,
if so, skip it during the leading/dangling/trailing comments formatting. The motivation for this change is that it is sometimes necessary that
the parent node formats the comments and we then need a way to prevent that the FormatNodeRule formats the same comments again. One such example
is the boolean operation formatting:

if (
    self._proc is not None
    # has the child process finished?
    and self._returncode is None
    # the child process has finished, but the
    # transport hasn't been notified yet?
    and self._proc.poll() is None
):
    pass

You can see that the leading comments of the right hand side must be formatted before the operator but the default comment formatting
formats them after. Another use case is that the leading comments of a *args or **kwargs must be formatted before the * or **.

The *args and **kwargs use case used a one-off solution so far which was easy enough to implement because it only applies to Arg. Doing the same
with boolean operations isn't as straightforward because the value can be an arbitrary Expr.

Test Plan

cargo test (the *args and **kwargs formatting still work as expected without any explicit handling)

Performance

I don't expect this to change performance much because the overhead is per comment and programs often only have few comments

This was referenced Jun 9, 2023
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@MichaReiser
Copy link
Member Author

MichaReiser commented Jun 9, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@MichaReiser MichaReiser changed the title Track formatted comment Track formatted comments Jun 9, 2023
@MichaReiser MichaReiser added internal An internal refactor or improvement formatter Related to the formatter labels Jun 9, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 9, 2023 •
edited
Loading

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      7.0±0.04ms     5.8 MB/sec    1.02      7.2±0.11ms     5.7 MB/sec
formatter/numpy/ctypeslib.py               1.00   1416.9±2.37µs    11.8 MB/sec    1.01   1433.2±3.10µs    11.6 MB/sec
formatter/numpy/globals.py                 1.00    139.3±0.21µs    21.2 MB/sec    1.01    141.0±0.28µs    20.9 MB/sec
formatter/pydantic/types.py                1.00      2.9±0.02ms     8.9 MB/sec    1.01      2.9±0.01ms     8.8 MB/sec
linter/all-rules/large/dataset.py          1.00     15.3±0.15ms     2.7 MB/sec    1.00     15.3±0.19ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.6±0.04ms     4.6 MB/sec    1.00      3.7±0.10ms     4.6 MB/sec
linter/all-rules/numpy/globals.py          1.01    370.0±1.10µs     8.0 MB/sec    1.00    367.2±1.46µs     8.0 MB/sec
linter/all-rules/pydantic/types.py         1.01      6.4±0.12ms     4.0 MB/sec    1.00      6.3±0.10ms     4.0 MB/sec
linter/default-rules/large/dataset.py      1.02      7.6±0.06ms     5.4 MB/sec    1.00      7.4±0.08ms     5.5 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.02   1569.8±3.63µs    10.6 MB/sec    1.00   1539.3±4.47µs    10.8 MB/sec
linter/default-rules/numpy/globals.py      1.02    167.7±0.23µs    17.6 MB/sec    1.00    164.7±0.31µs    17.9 MB/sec
linter/default-rules/pydantic/types.py     1.01      3.4±0.01ms     7.6 MB/sec    1.00      3.3±0.03ms     7.7 MB/sec

Windows

group                                      main                                    pr
-----                                      ----                                    --
formatter/large/dataset.py                 1.18      9.5±2.47ms     4.3 MB/sec     1.00      8.0±0.14ms     5.1 MB/sec
formatter/numpy/ctypeslib.py               1.03  1682.3±187.10µs     9.9 MB/sec    1.00  1626.8±32.79µs    10.2 MB/sec
formatter/numpy/globals.py                 1.00    153.7±4.08µs    19.2 MB/sec     1.01    154.7±6.50µs    19.1 MB/sec
formatter/pydantic/types.py                1.00      3.2±0.06ms     7.9 MB/sec     1.01      3.3±0.05ms     7.8 MB/sec
linter/all-rules/large/dataset.py          1.00     18.0±0.31ms     2.3 MB/sec     1.00     18.0±0.26ms     2.3 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      4.4±0.08ms     3.8 MB/sec     1.00      4.4±0.08ms     3.8 MB/sec
linter/all-rules/numpy/globals.py          1.00   514.8±12.64µs     5.7 MB/sec     1.00   514.2±10.72µs     5.7 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.5±0.16ms     3.4 MB/sec     1.00      7.5±0.14ms     3.4 MB/sec
linter/default-rules/large/dataset.py      1.03      9.0±0.66ms     4.5 MB/sec     1.00      8.7±0.13ms     4.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.06  1924.0±162.96µs     8.7 MB/sec    1.00  1818.8±29.21µs     9.2 MB/sec
linter/default-rules/numpy/globals.py      1.06   217.6±48.57µs    13.6 MB/sec     1.00    204.8±8.10µs    14.4 MB/sec
linter/default-rules/pydantic/types.py     1.08      4.2±0.60ms     6.1 MB/sec     1.00      3.9±0.06ms     6.6 MB/sec

@MichaReiser MichaReiser requested a review from konstin June 9, 2023 07:18
konstin
konstin approved these changes Jun 9, 2023
View reviewed changes
@MichaReiser MichaReiser force-pushed the fix-bin-oip-with-leading-comments branch from 4cf24dc to 7458b5b Compare June 9, 2023 08:46
@MichaReiser MichaReiser force-pushed the skip-formatted-comments branch from 48356d4 to 192261b Compare June 9, 2023 08:46
@MichaReiser MichaReiser force-pushed the fix-bin-oip-with-leading-comments branch from 7458b5b to e341c1c Compare June 9, 2023 08:56
Base automatically changed from fix-bin-oip-with-leading-comments to main June 9, 2023 09:02
@MichaReiser MichaReiser force-pushed the skip-formatted-comments branch from ca58055 to f444677 Compare June 9, 2023 09:04
@MichaReiser MichaReiser enabled auto-merge (squash) June 9, 2023 09:04
@MichaReiser MichaReiser merged commit 68d52da into main Jun 9, 2023
@MichaReiser MichaReiser deleted the skip-formatted-comments branch June 9, 2023 09:09
konstin pushed a commit that referenced this pull request Jun 13, 2023
@MichaReiser @konstin
Track formatted comments (#4979)
716f2c8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@konstin konstin konstin approved these changes

Assignees

No one assigned

Labels

formatter Related to the formatter internal An internal refactor or improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@MichaReiser @konstin