Skip to content

Generalize bracketed end-of-line comment handling#6315

Merged
charliermarsh merged 1 commit intomainfrom
charlie/placement
Aug 3, 2023
Merged

Generalize bracketed end-of-line comment handling#6315
charliermarsh merged 1 commit intomainfrom
charlie/placement

Conversation

@charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Aug 3, 2023

Micha suggested this in #6274 (comment), and it allows us to unify the implementations for arguments and type params.

@charliermarsh charliermarsh requested a review from zanieb August 3, 2023 19:54
@charliermarsh charliermarsh mentioned this pull request Aug 3, 2023
Merged
@charliermarsh charliermarsh force-pushed the charlie/placement branch 2 times, most recently from 81049c0 to 49959c5 Compare August 3, 2023 20:01
zanieb
zanieb reviewed Aug 3, 2023
View reviewed changes
crates/ruff_python_formatter/src/comments/placement.rs Outdated
Comment on lines +49 to +50
AnyNodeRef::Arguments(_) => handle_bracketed_end_of_line_comment(comment, locator),
AnyNodeRef::TypeParams(_) => handle_bracketed_end_of_line_comment(comment, locator),
Copy link
Member

@zanieb zanieb Aug 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not

Suggested change
AnyNodeRef::Arguments(_) => handle_bracketed_end_of_line_comment(comment, locator),
AnyNodeRef::TypeParams(_) => handle_bracketed_end_of_line_comment(comment, locator),
AnyNodeRef::Arguments(_) | AnyNodeRef::TypeParams(_) => handle_bracketed_end_of_line_comment(comment, locator),

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2023 •
edited
Loading

PR Check Results

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     10.1±0.45ms     4.0 MB/sec    1.03     10.3±0.30ms     3.9 MB/sec
formatter/numpy/ctypeslib.py               1.00  1954.4±64.24µs     8.5 MB/sec    1.03      2.0±0.07ms     8.3 MB/sec
formatter/numpy/globals.py                 1.03   233.3±11.86µs    12.6 MB/sec    1.00   226.2±13.33µs    13.0 MB/sec
formatter/pydantic/types.py                1.02      4.4±0.17ms     5.8 MB/sec    1.00      4.3±0.15ms     5.9 MB/sec
linter/all-rules/large/dataset.py          1.00     14.2±0.58ms     2.9 MB/sec    1.04     14.8±0.45ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.6±0.17ms     4.7 MB/sec    1.02      3.6±0.16ms     4.6 MB/sec
linter/all-rules/numpy/globals.py          1.01   505.7±27.97µs     5.8 MB/sec    1.00   498.3±22.45µs     5.9 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.3±0.22ms     4.0 MB/sec    1.05      6.6±0.22ms     3.8 MB/sec
linter/default-rules/large/dataset.py      1.00      7.2±0.21ms     5.7 MB/sec    1.19      8.5±0.31ms     4.8 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1483.9±57.52µs    11.2 MB/sec    1.18  1747.9±45.85µs     9.5 MB/sec
linter/default-rules/numpy/globals.py      1.00    174.7±6.84µs    16.9 MB/sec    1.17   204.3±10.62µs    14.4 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.2±0.13ms     7.9 MB/sec    1.16      3.7±0.10ms     6.8 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00     10.0±0.13ms     4.1 MB/sec    1.04     10.4±0.22ms     3.9 MB/sec
formatter/numpy/ctypeslib.py               1.00  1936.9±36.52µs     8.6 MB/sec    1.03  1986.6±28.11µs     8.4 MB/sec
formatter/numpy/globals.py                 1.00   212.0±10.63µs    13.9 MB/sec    1.03    218.6±5.83µs    13.5 MB/sec
formatter/pydantic/types.py                1.00      4.2±0.15ms     6.0 MB/sec    1.02      4.3±0.07ms     5.9 MB/sec
linter/all-rules/large/dataset.py          1.03     14.0±0.27ms     2.9 MB/sec    1.00     13.6±0.31ms     3.0 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.03      3.7±0.08ms     4.5 MB/sec    1.00      3.6±0.04ms     4.7 MB/sec
linter/all-rules/numpy/globals.py          1.01    446.8±8.14µs     6.6 MB/sec    1.00    444.5±6.64µs     6.6 MB/sec
linter/all-rules/pydantic/types.py         1.01      6.4±0.10ms     4.0 MB/sec    1.00      6.4±0.13ms     4.0 MB/sec
linter/default-rules/large/dataset.py      1.00      7.4±0.11ms     5.5 MB/sec    1.01      7.4±0.11ms     5.5 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01  1508.4±21.89µs    11.0 MB/sec    1.00  1499.5±19.69µs    11.1 MB/sec
linter/default-rules/numpy/globals.py      1.00    169.9±2.89µs    17.4 MB/sec    1.00    169.7±2.64µs    17.4 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.3±0.04ms     7.8 MB/sec    1.01      3.3±0.10ms     7.7 MB/sec

@charliermarsh charliermarsh enabled auto-merge (squash) August 3, 2023 20:38
@charliermarsh charliermarsh added the formatter Related to the formatter label Aug 3, 2023
@konstin
Copy link
Member

konstin commented Aug 3, 2023

Evil edge case:

f(
    ( # a
    b
    ),
)

This works (it's an edge case anyway), but not as intended:

f(  # a
    (b),
)

@charliermarsh charliermarsh merged commit 5f225b1 into main Aug 3, 2023
@charliermarsh charliermarsh deleted the charlie/placement branch August 3, 2023 20:51
@MichaReiser
Copy link
Member

MichaReiser commented Aug 4, 2023

Evil edge case:

f(
    ( # a
    b
    ),
)

This works (it's an edge case anyway), but not as intended:

f(  # a
    (b),
)

Argh yeah, the single argument call expression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zanieb zanieb zanieb approved these changes

Assignees

No one assigned

Labels

formatter Related to the formatter

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@charliermarsh @konstin @MichaReiser @zanieb