Skip to content

Preserve end-of-line comments on import-from statements#6216

Merged
charliermarsh merged 1 commit intomainfrom
charlie/comment-in-import
Aug 1, 2023
Merged

Preserve end-of-line comments on import-from statements#6216
charliermarsh merged 1 commit intomainfrom
charlie/comment-in-import

Conversation

@charliermarsh
Copy link
Copy Markdown
Member

@charliermarsh charliermarsh commented Aug 1, 2023

Summary

Ensures that we keep comments at the end-of-line in cases like:

from foo import (  # comment
  bar,
)

Closes #6067.

@charliermarsh charliermarsh requested a review from konstin August 1, 2023 01:08
@charliermarsh charliermarsh force-pushed the charlie/comment-in-import branch from 3b16233 to af04b20 Compare August 1, 2023 01:10
@charliermarsh
Copy link
Copy Markdown
Member Author

We can probably generalize some of this for other container types (lists, sets, etc).

@charliermarsh
Copy link
Copy Markdown
Member Author

(I'm looking into it.)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Aug 1, 2023

PR Check Results

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.01      8.3±0.07ms     4.9 MB/sec    1.00      8.2±0.13ms     5.0 MB/sec
formatter/numpy/ctypeslib.py               1.00  1644.0±36.70µs    10.1 MB/sec    1.00  1645.4±45.03µs    10.1 MB/sec
formatter/numpy/globals.py                 1.00    184.7±4.79µs    16.0 MB/sec    1.00    184.1±5.58µs    16.0 MB/sec
formatter/pydantic/types.py                1.02      3.6±0.10ms     7.1 MB/sec    1.00      3.5±0.13ms     7.3 MB/sec
linter/all-rules/large/dataset.py          1.02     11.4±0.20ms     3.6 MB/sec    1.00     11.1±0.10ms     3.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      2.8±0.04ms     5.9 MB/sec    1.00      2.8±0.03ms     5.9 MB/sec
linter/all-rules/numpy/globals.py          1.00    383.6±0.87µs     7.7 MB/sec    1.00    384.0±0.51µs     7.7 MB/sec
linter/all-rules/pydantic/types.py         1.02      5.1±0.10ms     5.0 MB/sec    1.00      5.0±0.07ms     5.1 MB/sec
linter/default-rules/large/dataset.py      1.05      6.1±0.08ms     6.7 MB/sec    1.00      5.8±0.06ms     7.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01  1228.9±21.87µs    13.5 MB/sec    1.00  1212.1±17.84µs    13.7 MB/sec
linter/default-rules/numpy/globals.py      1.00    134.1±0.98µs    22.0 MB/sec    1.00    133.9±0.82µs    22.0 MB/sec
linter/default-rules/pydantic/types.py     1.03      2.6±0.05ms     9.6 MB/sec    1.00      2.6±0.06ms     9.9 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      9.8±0.10ms     4.1 MB/sec    1.02     10.0±0.14ms     4.1 MB/sec
formatter/numpy/ctypeslib.py               1.00  1934.6±63.54µs     8.6 MB/sec    1.00  1935.5±29.37µs     8.6 MB/sec
formatter/numpy/globals.py                 1.00   216.0±10.87µs    13.7 MB/sec    1.00    215.1±6.97µs    13.7 MB/sec
formatter/pydantic/types.py                1.00      4.2±0.06ms     6.0 MB/sec    1.03      4.3±0.09ms     5.9 MB/sec
linter/all-rules/large/dataset.py          1.00     13.4±0.14ms     3.0 MB/sec    1.03     13.8±0.29ms     3.0 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.5±0.05ms     4.7 MB/sec    1.01      3.6±0.07ms     4.6 MB/sec
linter/all-rules/numpy/globals.py          1.00    433.1±5.26µs     6.8 MB/sec    1.00    435.1±7.13µs     6.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.0±0.07ms     4.2 MB/sec    1.01      6.1±0.09ms     4.2 MB/sec
linter/default-rules/large/dataset.py      1.00      7.3±0.11ms     5.5 MB/sec    1.04      7.6±0.13ms     5.3 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1489.5±25.78µs    11.2 MB/sec    1.03  1529.8±38.35µs    10.9 MB/sec
linter/default-rules/numpy/globals.py      1.00    167.2±3.07µs    17.6 MB/sec    1.01    169.4±4.16µs    17.4 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.2±0.04ms     8.0 MB/sec    1.04      3.3±0.07ms     7.7 MB/sec

@charliermarsh charliermarsh added the formatter Related to the formatter label Aug 1, 2023
Comment thread crates/ruff_python_formatter/src/comments/placement.rs Outdated
Comment thread crates/ruff_python_formatter/src/comments/placement.rs Outdated
Comment thread crates/ruff_python_formatter/src/comments/placement.rs Outdated
Comment thread crates/ruff_python_formatter/src/statement/stmt_import_from.rs Outdated
Comment thread crates/ruff_python_formatter/src/comments/placement.rs Outdated
Comment thread crates/ruff_python_formatter/src/comments/placement.rs Outdated
@charliermarsh charliermarsh force-pushed the charlie/comment-in-import branch 4 times, most recently from bd8f7dd to c5732e8 Compare August 1, 2023 15:33
@charliermarsh
Copy link
Copy Markdown
Member Author

Okay, hopefully good to go.

Comment on lines +1141 to +1143
let mut tokens =
SimpleTokenizer::up_to_without_back_comment(comment.start(), locator.contents())
.skip_trivia();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: I would love to remove the backward lexing if possible in the future. It kind of works, but I'm worried that we'll run into issues sooner or later (e.g. magic commands are interesting...) . Could we instead search forward from the end of the identifier (or the start of the statement if absent)?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think I figured out a solution that doesn't require lexing. If the comment is end-of-line, and between the start of the statement and the first member, it has to be immediately following the parenthesis.

@charliermarsh charliermarsh force-pushed the charlie/comment-in-import branch from c5732e8 to 2ebf7b2 Compare August 1, 2023 18:51
@charliermarsh charliermarsh enabled auto-merge (squash) August 1, 2023 18:53
@charliermarsh charliermarsh merged commit 7842c82 into main Aug 1, 2023
@charliermarsh charliermarsh deleted the charlie/comment-in-import branch August 1, 2023 18:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

formatter Related to the formatter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Formatter: Preserve end-of-line comments in imports

3 participants