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.

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

Format class definitions #5289

Merged
merged 1 commit into from
Jun 22, 2023
Merged

Format class definitions #5289

merged 1 commit into from
Jun 22, 2023

Conversation

MichaReiser
Copy link
Member

Summary

This PR adds basic formatting for class definitions.

Test Plan

Ran the stability check to verify that the class definition formatting does not introduce any new instabilities.
Added new snapshot tests.

@MichaReiser
Copy link
Member Author

MichaReiser commented Jun 22, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@MichaReiser MichaReiser linked an issue Jun 22, 2023 that may be closed by this pull request
@MichaReiser MichaReiser added formatter Related to the formatter internal An internal refactor or improvement labels Jun 22, 2023
@@ -50,15 +50,15 @@ toml = { version = "0.7.2" }
# v0.0.1
libcst = { git = "https://github.com/charliermarsh/LibCST", rev = "80e4c1399f95e5beb532fdd1e209ad2dbb470438" }
# v0.0.3
ruff_text_size = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "08ebbe40d7776cac6e3ba66277d435056f2b8dca" }
ruff_text_size = { git = "https://github.com/astral-sh/RustPython-Parser.git", rev = "f60e204b73b95bdb6ce87ccd0de34081b4a17c11" }
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Jepee

..
} = self.class_definition;

let separator = format_with(|f| write!(f, [text(","), soft_line_break_or_space()]));
Copy link
Member Author

Choose a reason for hiding this comment

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

We should probably extract a helper at some point that joins a list with , and soft_line_break_or_space and preserves any trailing magic comma.

@MichaReiser MichaReiser requested a review from konstin June 22, 2023 08:10
@MichaReiser MichaReiser force-pushed the format-class-def branch 3 times, most recently from b8cc710 to 5acb7be Compare June 22, 2023 08:19
Comment on lines 89 to 95
// Manually handle parentheses here because the expression is parenthesized incorrectly assumes that the
// parentheses from the inheritance clause belong to the expression.
Copy link
Member

Choose a reason for hiding this comment

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

don't understand that comment

Copy link
Member Author

Choose a reason for hiding this comment

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

I reworded the comment and added an example.

class_definition: &'a StmtClassDef,
}

impl Format<PyFormatContext<'_>> for FormatInheritanceClause<'_> {
Copy link
Member

Choose a reason for hiding this comment

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

Is this effectively formatting a function call?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's similar but it doesn't need to support any of the complexity like chaining.

@MichaReiser MichaReiser enabled auto-merge (squash) June 22, 2023 08:34
@github-actions
Copy link
Contributor

github-actions bot commented Jun 22, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.03      8.2±0.13ms     5.0 MB/sec    1.00      7.9±0.03ms     5.1 MB/sec
formatter/numpy/ctypeslib.py               1.03  1784.7±13.58µs     9.3 MB/sec    1.00  1740.4±10.98µs     9.6 MB/sec
formatter/numpy/globals.py                 1.02    196.6±2.37µs    15.0 MB/sec    1.00    193.1±3.37µs    15.3 MB/sec
formatter/pydantic/types.py                1.03      4.1±0.04ms     6.2 MB/sec    1.00      4.0±0.01ms     6.3 MB/sec
linter/all-rules/large/dataset.py          1.00     15.5±0.24ms     2.6 MB/sec    1.03     15.9±0.12ms     2.6 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.9±0.07ms     4.3 MB/sec    1.02      3.9±0.03ms     4.2 MB/sec
linter/all-rules/numpy/globals.py          1.00    497.8±8.41µs     5.9 MB/sec    1.02    508.1±1.76µs     5.8 MB/sec
linter/all-rules/pydantic/types.py         1.00      6.8±0.12ms     3.8 MB/sec    1.04      7.0±0.05ms     3.6 MB/sec
linter/default-rules/large/dataset.py      1.00      7.9±0.09ms     5.2 MB/sec    1.03      8.1±0.03ms     5.0 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00  1744.8±17.09µs     9.5 MB/sec    1.02   1782.4±4.69µs     9.3 MB/sec
linter/default-rules/numpy/globals.py      1.00    192.6±4.13µs    15.3 MB/sec    1.05    201.4±0.31µs    14.7 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.6±0.05ms     7.1 MB/sec    1.04      3.7±0.00ms     6.9 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.04     12.8±0.47ms     3.2 MB/sec    1.00     12.3±0.59ms     3.3 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.6±0.14ms     6.5 MB/sec    1.01      2.6±0.10ms     6.4 MB/sec
formatter/numpy/globals.py                 1.00   278.7±20.17µs    10.6 MB/sec    1.03   287.2±24.32µs    10.3 MB/sec
formatter/pydantic/types.py                1.03      6.0±0.25ms     4.3 MB/sec    1.00      5.8±0.21ms     4.4 MB/sec
linter/all-rules/large/dataset.py          1.05     21.5±0.62ms  1941.9 KB/sec    1.00     20.5±0.71ms  2036.0 KB/sec
linter/all-rules/numpy/ctypeslib.py        1.01      5.4±0.27ms     3.1 MB/sec    1.00      5.3±0.23ms     3.1 MB/sec
linter/all-rules/numpy/globals.py          1.00   659.5±29.36µs     4.5 MB/sec    1.02   673.5±42.16µs     4.4 MB/sec
linter/all-rules/pydantic/types.py         1.00      9.0±0.38ms     2.8 MB/sec    1.00      9.0±0.42ms     2.8 MB/sec
linter/default-rules/large/dataset.py      1.00     10.7±0.59ms     3.8 MB/sec    1.02     11.0±0.54ms     3.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01      2.2±0.14ms     7.4 MB/sec    1.00      2.2±0.10ms     7.5 MB/sec
linter/default-rules/numpy/globals.py      1.00   273.5±14.28µs    10.8 MB/sec    1.00   274.8±17.86µs    10.7 MB/sec
linter/default-rules/pydantic/types.py     1.01      4.9±0.22ms     5.2 MB/sec    1.00      4.8±0.26ms     5.3 MB/sec

auto-merge was automatically disabled June 22, 2023 08:54

Pull request was closed

@MichaReiser MichaReiser reopened this Jun 22, 2023
@MichaReiser MichaReiser enabled auto-merge (squash) June 22, 2023 08:54
@MichaReiser MichaReiser mentioned this pull request Jun 22, 2023
@MichaReiser MichaReiser merged commit f7e1cf4 into main Jun 22, 2023
@MichaReiser MichaReiser deleted the format-class-def branch June 22, 2023 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Formatter: ClassDef
2 participants