Skip to content

Format while Statement#4810

Merged
MichaReiser merged 2 commits intomainfrom
format-while-statement
Jun 5, 2023
Merged

Format while Statement#4810
MichaReiser merged 2 commits intomainfrom
format-while-statement

Conversation

@MichaReiser
Copy link
Member

@MichaReiser MichaReiser commented Jun 2, 2023 •
edited
Loading

Summary

This PR implements formatting of while statements.

Test Plan

I reviewed the black changes and added our own tests to verify the correct placement of comments.

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

MichaReiser commented Jun 2, 2023 •
edited by konstin
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@MichaReiser MichaReiser mentioned this pull request Jun 2, 2023
Merged
@github-actions
Copy link
Contributor

github-actions bot commented Jun 2, 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     14.6±0.04ms     2.8 MB/sec    1.03     15.1±0.13ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.5±0.00ms     4.7 MB/sec    1.00      3.5±0.01ms     4.7 MB/sec
linter/all-rules/numpy/globals.py          1.00    360.1±1.16µs     8.2 MB/sec    1.01    362.0±1.62µs     8.2 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.1±0.01ms     4.2 MB/sec    1.00      6.1±0.02ms     4.2 MB/sec
linter/default-rules/large/dataset.py      1.00      7.3±0.01ms     5.6 MB/sec    1.00      7.3±0.05ms     5.5 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1533.1±3.63µs    10.9 MB/sec    1.00   1533.5±3.42µs    10.9 MB/sec
linter/default-rules/numpy/globals.py      1.00    164.2±0.24µs    18.0 MB/sec    1.00    165.0±0.23µs    17.9 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.3±0.00ms     7.8 MB/sec    1.00      3.3±0.01ms     7.8 MB/sec
parser/large/dataset.py                    1.00      5.8±0.00ms     7.0 MB/sec    1.01      5.9±0.01ms     6.9 MB/sec
parser/numpy/ctypeslib.py                  1.00   1126.7±0.71µs    14.8 MB/sec    1.13   1270.0±0.80µs    13.1 MB/sec
parser/numpy/globals.py                    1.00    114.2±0.28µs    25.8 MB/sec    1.04    118.3±0.45µs    24.9 MB/sec
parser/pydantic/types.py                   1.00      2.5±0.00ms    10.3 MB/sec    1.01      2.5±0.00ms    10.2 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
linter/all-rules/large/dataset.py          1.00     15.3±0.08ms     2.7 MB/sec    1.00     15.3±0.20ms     2.7 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.6±0.03ms     4.7 MB/sec    1.00      3.6±0.02ms     4.7 MB/sec
linter/all-rules/numpy/globals.py          1.00    383.8±3.52µs     7.7 MB/sec    1.00    384.9±2.12µs     7.7 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.2±0.04ms     4.1 MB/sec    1.01      6.3±0.03ms     4.1 MB/sec
linter/default-rules/large/dataset.py      1.00      7.6±0.03ms     5.3 MB/sec    1.00      7.6±0.03ms     5.3 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1534.9±11.99µs    10.8 MB/sec    1.00   1540.7±5.83µs    10.8 MB/sec
linter/default-rules/numpy/globals.py      1.00    167.2±1.28µs    17.6 MB/sec    1.00    167.8±1.03µs    17.6 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.3±0.03ms     7.6 MB/sec    1.00      3.3±0.02ms     7.6 MB/sec
parser/large/dataset.py                    1.00      6.1±0.03ms     6.6 MB/sec    1.02      6.3±0.02ms     6.5 MB/sec
parser/numpy/ctypeslib.py                  1.00   1141.2±4.79µs    14.6 MB/sec    1.02   1159.0±4.66µs    14.4 MB/sec
parser/numpy/globals.py                    1.00    116.7±0.64µs    25.3 MB/sec    1.01    118.4±0.68µs    24.9 MB/sec
parser/pydantic/types.py                   1.00      2.6±0.01ms     9.9 MB/sec    1.02      2.6±0.01ms     9.8 MB/sec

@MichaReiser MichaReiser force-pushed the join-nodes-builder branch from d113ea6 to 530cb36 Compare June 2, 2023 14:03
Base automatically changed from join-nodes-builder to main June 2, 2023 14:14
@MichaReiser MichaReiser force-pushed the format-while-statement branch 2 times, most recently from 2e10b8a to b45ed57 Compare June 3, 2023 18:01
zanieb
zanieb reviewed Jun 3, 2023
View reviewed changes
...s/ruff_python_formatter/src/snapshots/ruff_python_formatter__tests__ruff_test__while_py.snap
Comment on lines +53 to +54
# trailing else body comment
while (
Copy link
Member

@zanieb zanieb Jun 3, 2023

Choose a reason for hiding this comment

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

Should this newline be removed?

Copy link
Member Author

@MichaReiser MichaReiser Jun 3, 2023

Choose a reason for hiding this comment

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

Good catch. No it should not. @konstin is working on keeping the right amount of newlines between top level statements in #4808 I should probably rebase on top of that.

@MichaReiser MichaReiser force-pushed the format-while-statement branch 2 times, most recently from 878709f to b602cdc Compare June 4, 2023 08:50
MichaReiser
MichaReiser commented Jun 4, 2023
View reviewed changes
crates/ruff_python_formatter/src/comments/format.rs

/// Formats the leading comments of a node.
pub(crate) fn leading_comments<T>(node: &T) -> FormatLeadingComments
pub(crate) fn leading_node_comments<T>(node: &T) -> FormatLeadingComments
Copy link
Member Author

@MichaReiser MichaReiser Jun 4, 2023

Choose a reason for hiding this comment

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

I renamed the function because I now needed a way to either format the leading comments of a node OR a specific list of comments.

crates/ruff_python_formatter/src/expression/maybe_parenthesize.rs
}
}

const fn needs_parentheses(expr: &Expr) -> bool {
Copy link
Member Author

@MichaReiser MichaReiser Jun 4, 2023

Choose a reason for hiding this comment

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

This match is not complete yet because it's impossible to test without having implemented some expression formatting.

We should revisit this later. The important thing is that we have this logic centrelized, so that it should be easy to fix later.

@MichaReiser MichaReiser added internal An internal refactor or improvement formatter Related to the formatter labels Jun 4, 2023
@MichaReiser MichaReiser marked this pull request as ready for review June 4, 2023 08:57
@MichaReiser MichaReiser linked an issue Jun 4, 2023 that may be closed by this pull request
Formatter: While #4806
Closed
@MichaReiser
Copy link
Member Author

MichaReiser commented Jun 4, 2023

How do I get pre-commit to ignore my test files? We already exclude the whole resources directory

@MichaReiser MichaReiser mentioned this pull request Jun 4, 2023
Merged
konstin
konstin approved these changes Jun 5, 2023
View reviewed changes
crates/ruff_python_formatter/resources/test/fixtures/ruff/statement/while.py Outdated
# trailing else body comment


while aVeryLongConditionThatSpillsOverToTheNextLineBecauseItIsExtremellyLongAndGoesOnAndOnAndOnAndOnAndOnAndOnAndOnAndOnAndOn: # trailing comment
Copy link
Member

@konstin konstin Jun 4, 2023

Choose a reason for hiding this comment

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

she took the midnight train going anywhere 🎶

@MichaReiser MichaReiser force-pushed the format-while-statement branch from b602cdc to 30574e1 Compare June 5, 2023 08:16
@MichaReiser MichaReiser enabled auto-merge (squash) June 5, 2023 08:17
@MichaReiser MichaReiser merged commit c65f47d into main Jun 5, 2023
@MichaReiser MichaReiser deleted the format-while-statement branch June 5, 2023 08:24
konstin pushed a commit that referenced this pull request Jun 13, 2023
@MichaReiser @konstin
Format while Statement (#4810)
84594f1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zanieb zanieb zanieb left review comments

@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: While

3 participants

@MichaReiser @zanieb @konstin