Skip to content

Format if statements#4961

Merged
MichaReiser merged 2 commits intomainfrom
format-if-statement
Jun 9, 2023
Merged

Format if statements#4961
MichaReiser merged 2 commits intomainfrom
format-if-statement

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Jun 8, 2023

Summary

This PR implements the formatting of if statements. I haven't verified if it covers all edge cases because
that will be easier once we have better coverage (you can simply go througth the black differences), but I believe it covers most reasonable uses pretty well.

Test Plan

A few own unit tests and reviewed (as best as I can) the black differences

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

MichaReiser commented Jun 8, 2023 •
edited
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@github-actions
Copy link
Contributor

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

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      6.8±0.01ms     6.0 MB/sec    1.01      6.9±0.01ms     5.9 MB/sec
formatter/numpy/ctypeslib.py               1.00   1382.1±3.95µs    12.0 MB/sec    1.01   1390.5±3.18µs    12.0 MB/sec
formatter/numpy/globals.py                 1.00    138.3±0.21µs    21.3 MB/sec    1.01    139.1±0.87µs    21.2 MB/sec
formatter/pydantic/types.py                1.00      2.8±0.01ms     9.2 MB/sec    1.01      2.8±0.01ms     9.1 MB/sec
linter/all-rules/large/dataset.py          1.02     15.1±0.06ms     2.7 MB/sec    1.00     14.8±0.02ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      3.6±0.00ms     4.6 MB/sec    1.00      3.6±0.00ms     4.6 MB/sec
linter/all-rules/numpy/globals.py          1.01    369.8±1.42µs     8.0 MB/sec    1.00    367.0±1.74µs     8.0 MB/sec
linter/all-rules/pydantic/types.py         1.01      6.3±0.01ms     4.1 MB/sec    1.00      6.2±0.01ms     4.1 MB/sec
linter/default-rules/large/dataset.py      1.03      7.5±0.01ms     5.4 MB/sec    1.00      7.3±0.02ms     5.6 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.02   1568.8±5.41µs    10.6 MB/sec    1.00   1531.1±3.97µs    10.9 MB/sec
linter/default-rules/numpy/globals.py      1.02    167.7±0.98µs    17.6 MB/sec    1.00    164.0±0.30µs    18.0 MB/sec
linter/default-rules/pydantic/types.py     1.03      3.4±0.01ms     7.6 MB/sec    1.00      3.3±0.00ms     7.8 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      7.8±0.02ms     5.2 MB/sec    1.05      8.2±0.04ms     5.0 MB/sec
formatter/numpy/ctypeslib.py               1.00  1573.3±15.51µs    10.6 MB/sec    1.04  1639.9±21.41µs    10.2 MB/sec
formatter/numpy/globals.py                 1.00    156.3±1.60µs    18.9 MB/sec    1.03    161.1±2.81µs    18.3 MB/sec
formatter/pydantic/types.py                1.00      3.2±0.02ms     7.9 MB/sec    1.04      3.3±0.03ms     7.6 MB/sec
linter/all-rules/large/dataset.py          1.00     16.8±0.15ms     2.4 MB/sec    1.01     17.0±0.12ms     2.4 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      4.3±0.03ms     3.9 MB/sec    1.00      4.3±0.02ms     3.9 MB/sec
linter/all-rules/numpy/globals.py          1.01    434.8±5.22µs     6.8 MB/sec    1.00    430.2±5.98µs     6.9 MB/sec
linter/all-rules/pydantic/types.py         1.00      7.2±0.04ms     3.6 MB/sec    1.00      7.2±0.05ms     3.6 MB/sec
linter/default-rules/large/dataset.py      1.00      8.4±0.04ms     4.8 MB/sec    1.00      8.4±0.03ms     4.8 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1750.3±8.60µs     9.5 MB/sec    1.00  1756.1±11.41µs     9.5 MB/sec
linter/default-rules/numpy/globals.py      1.00    187.9±1.56µs    15.7 MB/sec    1.00    188.3±1.48µs    15.7 MB/sec
linter/default-rules/pydantic/types.py     1.01      3.8±0.03ms     6.6 MB/sec    1.00      3.8±0.03ms     6.7 MB/sec

@MichaReiser MichaReiser linked an issue Jun 8, 2023 that may be closed by this pull request
Formatter: If Statement #4956
Closed
@MichaReiser MichaReiser force-pushed the format-function-definition branch from 60dd48a to 26a4a83 Compare June 8, 2023 15:27
@MichaReiser MichaReiser force-pushed the format-if-statement branch from 2fd8f14 to b25a3ac Compare June 8, 2023 15:27
@MichaReiser MichaReiser mentioned this pull request Jun 8, 2023
Merged
@MichaReiser MichaReiser force-pushed the format-function-definition branch from 26a4a83 to 3be5049 Compare June 8, 2023 15:34
@MichaReiser MichaReiser force-pushed the format-if-statement branch from b25a3ac to 226aabf Compare June 8, 2023 15:34
@MichaReiser MichaReiser force-pushed the format-function-definition branch 2 times, most recently from dcb2aaa to 3b8b3a9 Compare June 8, 2023 15:57
Base automatically changed from format-function-definition to main June 8, 2023 16:07
@MichaReiser MichaReiser force-pushed the format-if-statement branch from 226aabf to b9826c1 Compare June 8, 2023 16:10
@MichaReiser MichaReiser requested a review from konstin June 8, 2023 16:10
@MichaReiser MichaReiser added internal An internal refactor or improvement formatter Related to the formatter labels Jun 8, 2023
This was referenced Jun 9, 2023
Merged
Merged
konstin
konstin approved these changes Jun 9, 2023
View reviewed changes
crates/ruff_python_formatter/src/statement/stmt_if.rs
else_comments = trailing_alternate_comments;
last_node_of_previous_body = body.last();

if let Some(elif) = else_if(orelse) {
Copy link
Member

@konstin konstin Jun 9, 2023

Choose a reason for hiding this comment

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

Does this mean there's no elif node, just if?

Copy link
Member Author

@MichaReiser MichaReiser Jun 9, 2023

Choose a reason for hiding this comment

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

Yep. elif is represented as an else body that contains a single if statement. Rome (and Roslyn) model this very similarly where the alternate (else) is either an if statement (in which it is an else if) or a block (an else).

@MichaReiser MichaReiser merged commit 1accbef into main Jun 9, 2023
@MichaReiser MichaReiser deleted the format-if-statement branch June 9, 2023 08:55
konstin pushed a commit that referenced this pull request Jun 13, 2023
@MichaReiser @konstin
Format if statements (#4961)
1799fb8
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.

Formatter: If Statement

2 participants

@MichaReiser @konstin