Skip to content
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

Implement module formatting using JoinNodesBuilder #4808

Merged
merged 2 commits into from
Jun 5, 2023
Merged

Implement module formatting using JoinNodesBuilder #4808

merged 2 commits into from
Jun 5, 2023

Conversation

konstin
Copy link
Member

@konstin konstin commented Jun 2, 2023

This uses JoinNodesBuilder to implement module formatting for #4800

See the snapshots for the changed behaviour. See one PR up for a CLI that i used to verify the trailing new line behaviour

@konstin konstin mentioned this pull request Jun 2, 2023
Merged
@konstin
Copy link
Member Author

konstin commented Jun 2, 2023 •
edited
Loading

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@konstin konstin mentioned this pull request Jun 2, 2023
Merged
@konstin konstin added internal An internal refactor or improvement formatter Related to the formatter labels Jun 2, 2023
@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.01     13.8±0.05ms     2.9 MB/sec    1.00     13.6±0.04ms     3.0 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.3±0.02ms     5.0 MB/sec    1.00      3.3±0.02ms     5.0 MB/sec
linter/all-rules/numpy/globals.py          1.00    409.5±1.18µs     7.2 MB/sec    1.01    412.7±1.65µs     7.1 MB/sec
linter/all-rules/pydantic/types.py         1.00      5.8±0.04ms     4.4 MB/sec    1.02      5.9±0.02ms     4.4 MB/sec
linter/default-rules/large/dataset.py      1.00      6.7±0.01ms     6.1 MB/sec    1.00      6.6±0.01ms     6.1 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1444.9±3.35µs    11.5 MB/sec    1.00   1449.3±1.54µs    11.5 MB/sec
linter/default-rules/numpy/globals.py      1.00    159.5±0.30µs    18.5 MB/sec    1.00    159.5±0.68µs    18.5 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.0±0.00ms     8.5 MB/sec    1.00      3.0±0.01ms     8.5 MB/sec
parser/large/dataset.py                    1.03      5.4±0.01ms     7.6 MB/sec    1.00      5.2±0.00ms     7.8 MB/sec
parser/numpy/ctypeslib.py                  1.01   1035.5±0.77µs    16.1 MB/sec    1.00   1022.1±0.52µs    16.3 MB/sec
parser/numpy/globals.py                    1.02    108.8±0.16µs    27.1 MB/sec    1.00    107.1±0.19µs    27.5 MB/sec
parser/pydantic/types.py                   1.01      2.3±0.00ms    11.2 MB/sec    1.00      2.2±0.00ms    11.4 MB/sec

Windows

group                                      main                                    pr
-----                                      ----                                    --
linter/all-rules/large/dataset.py          1.02     23.4±0.90ms  1783.2 KB/sec     1.00     22.9±0.85ms  1817.0 KB/sec
linter/all-rules/numpy/ctypeslib.py        1.05      5.9±0.35ms     2.8 MB/sec     1.00      5.6±0.29ms     3.0 MB/sec
linter/all-rules/numpy/globals.py          1.04   642.7±41.20µs     4.6 MB/sec     1.00   616.7±18.67µs     4.8 MB/sec
linter/all-rules/pydantic/types.py         1.04      9.6±0.49ms     2.7 MB/sec     1.00      9.2±0.57ms     2.8 MB/sec
linter/default-rules/large/dataset.py      1.03     11.0±0.51ms     3.7 MB/sec     1.00     10.7±0.50ms     3.8 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00      2.2±0.09ms     7.6 MB/sec     1.01      2.2±0.09ms     7.5 MB/sec
linter/default-rules/numpy/globals.py      1.03   267.9±14.15µs    11.0 MB/sec     1.00   260.4±12.16µs    11.3 MB/sec
linter/default-rules/pydantic/types.py     1.00      4.9±0.31ms     5.2 MB/sec     1.00      4.9±0.23ms     5.2 MB/sec
parser/large/dataset.py                    1.00      8.6±0.28ms     4.7 MB/sec     1.00      8.6±0.30ms     4.7 MB/sec
parser/numpy/ctypeslib.py                  1.02  1645.1±111.27µs    10.1 MB/sec    1.00  1605.5±61.51µs    10.4 MB/sec
parser/numpy/globals.py                    1.00    163.8±9.44µs    18.0 MB/sec     1.03   169.1±11.06µs    17.4 MB/sec
parser/pydantic/types.py                   1.00      3.7±0.20ms     6.9 MB/sec     1.03      3.8±0.27ms     6.7 MB/sec

MichaReiser
MichaReiser reviewed Jun 2, 2023
View reviewed changes
crates/ruff_python_formatter/src/module/mod_module.rs Outdated
Comment on lines 13 to 15
f.join_nodes(NodeLevel::TopLevel)
.nodes(&item.body)
.finish()?;
Copy link
Member

@MichaReiser MichaReiser Jun 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Should this use item.body.format().with_options(SuiteLevel::TopLevel) (The Suite formatting) so that it inserts the right amount of new lines before and after classes and functions?

The JoinNodesBuilder only maintains the same number of empty lines (up to 1 or 2) as in the source. The Vec<Stmt> formatting ensures that each function and class has an empty line before and after

Copy link
Member Author

Choose a reason for hiding this comment

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

done

This was referenced Jun 2, 2023
Merged
Merged
Merged
Merged
Merged
Merged
Base automatically changed from join-nodes-builder to main June 2, 2023 14:14
@MichaReiser MichaReiser mentioned this pull request Jun 4, 2023
Merged
@konstin
Implement module formatting using JoinNodesBuilder
446f987 
This uses JoinNodesBuilder to implement module formatting for #4800

See the snapshots for the changed behaviour. See one PR up for a CLI that i used to verify the trailing new line behaviour
@konstin
lint and comment
dd948e6
@konstin konstin enabled auto-merge (squash) June 5, 2023 08:28
@konstin konstin merged commit ff37d7a into main Jun 5, 2023
@konstin konstin deleted the format-module branch June 5, 2023 08:35
konstin added a commit that referenced this pull request Jun 13, 2023
@konstin
Implement module formatting using JoinNodesBuilder (#4808)
bcaa885 
* Implement module formatting using JoinNodesBuilder

This uses JoinNodesBuilder to implement module formatting for #4800

See the snapshots for the changed behaviour. See one PR up for a CLI that i used to verify the trailing new line behaviour
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaReiser MichaReiser MichaReiser 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.
2 participants
@konstin @MichaReiser