Skip to content

Order Edits by Locations #3905

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
MichaReiser merged 2 commits into from
Apr 11, 2023
Merged

Order Edits by Locations #3905

MichaReiser merged 2 commits into from
Apr 11, 2023

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Apr 7, 2023 •
edited
Loading

This PR changes the Ordering of Edits from content > location > end_location to location > end_location > content.

This is the more natural order in my view and has the added benefit that comparing the content is only necessary for edits that start and end at the same location, which should be rare.

Changing the field order, unfortunately, changes all snapshots... A good example why we shouldn't use the serialized Diagnostic in test assertions.

I used this PR as a chance to type content as Option<Box<str>> because we don't intend to modify the edit (reduces the size of every edit by 8 bytes).

This was referenced Apr 7, 2023
Merged
Merged
Merged
Merged
Merged
@MichaReiser
Copy link
Member Author

MichaReiser commented Apr 7, 2023 •
edited
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

MichaReiser
MichaReiser commented Apr 7, 2023
View reviewed changes
MichaReiser
MichaReiser commented Apr 7, 2023
View reviewed changes
crates/ruff/src/rules/flake8_type_checking/rules/empty_type_checking_block.rs
) {
Ok(fix) => {
if fix.content.is_empty() || fix.content == "pass" {
if fix.is_deletion() || fix.content() == Some("pass") {
Copy link
Member Author

@MichaReiser MichaReiser Apr 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Replacing or inserting "pass" is a deletion...?

@MichaReiser MichaReiser mentioned this pull request Apr 7, 2023
Merged
@github-actions
Copy link
Contributor

github-actions bot commented Apr 7, 2023 •
edited
Loading

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     15.1±0.03ms     2.7 MB/sec    1.00     15.1±0.02ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.8±0.00ms     4.4 MB/sec    1.00      3.8±0.01ms     4.4 MB/sec
linter/all-rules/numpy/globals.py          1.01    415.5±1.16µs     7.1 MB/sec    1.00    411.7±1.76µs     7.2 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.5±0.01ms     3.9 MB/sec    1.00      6.4±0.01ms     4.0 MB/sec
linter/default-rules/large/dataset.py      1.02      7.9±0.02ms     5.1 MB/sec    1.00      7.8±0.01ms     5.2 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1731.6±7.77µs     9.6 MB/sec    1.00   1732.7±5.19µs     9.6 MB/sec
linter/default-rules/numpy/globals.py      1.00    181.8±0.46µs    16.2 MB/sec    1.00    181.2±2.74µs    16.3 MB/sec
linter/default-rules/pydantic/types.py     1.01      3.6±0.01ms     7.0 MB/sec    1.00      3.6±0.01ms     7.1 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     23.0±0.48ms  1812.3 KB/sec    1.02     23.5±0.54ms  1776.0 KB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      5.8±0.18ms     2.9 MB/sec    1.00      5.8±0.17ms     2.9 MB/sec
linter/all-rules/numpy/globals.py          1.00   678.5±26.10µs     4.3 MB/sec    1.03   695.6±58.22µs     4.2 MB/sec
linter/all-rules/pydantic/types.py         1.00      9.8±0.36ms     2.6 MB/sec    1.01      9.8±0.25ms     2.6 MB/sec
linter/default-rules/large/dataset.py      1.00     11.5±0.26ms     3.5 MB/sec    1.03     11.8±0.34ms     3.4 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00      2.5±0.08ms     6.7 MB/sec    1.05      2.6±0.07ms     6.4 MB/sec
linter/default-rules/numpy/globals.py      1.00   282.9±11.41µs    10.4 MB/sec    1.06   298.5±24.94µs     9.9 MB/sec
linter/default-rules/pydantic/types.py     1.00      5.2±0.12ms     4.9 MB/sec    1.02      5.3±0.12ms     4.8 MB/sec

@MichaReiser MichaReiser added the internal An internal refactor or improvement label Apr 11, 2023
Base automatically changed from code-diff to main April 11, 2023 08:28
@MichaReiser MichaReiser enabled auto-merge (squash) April 11, 2023 08:30
@MichaReiser MichaReiser disabled auto-merge April 11, 2023 08:42
@MichaReiser MichaReiser changed the title Order Edits by Locations Order Edits by Locations Apr 11, 2023
@MichaReiser MichaReiser enabled auto-merge (squash) April 11, 2023 08:50
@MichaReiser MichaReiser merged commit 210083b into main Apr 11, 2023
@MichaReiser MichaReiser deleted the edit-operation branch April 11, 2023 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charliermarsh charliermarsh charliermarsh approved these changes

Assignees

No one assigned

Labels

internal An internal refactor or improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@MichaReiser @charliermarsh