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 implicit string continuation #5328

Merged
merged 2 commits into from
Jun 26, 2023
Merged

Conversation

MichaReiser
Copy link
Member

Summary

This Branch adds support for formatting implicit string continuations. We could probably come up with something faster that combines the preferred_quote with the detection of implicit continuation but I don't think that it's worth optimizing just yet.

Test Plan

I added new unit tests because black's test suite has not a single implicit continuation.

I ran the stability check against the cpython project.

@MichaReiser
Copy link
Member Author

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@MichaReiser MichaReiser force-pushed the format-implicit-continuation branch from cc6019d to affb113 Compare June 23, 2023 09:53
@@ -10,6 +11,14 @@ impl FormatNodeRule<StmtExpr> for FormatStmtExpr {
fn fmt_fields(&self, item: &StmtExpr, f: &mut PyFormatter) -> FormatResult<()> {
let StmtExpr { value, .. } = item;

if let Some(constant) = value.as_constant_expr() {
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 seems that StmtExpr is the only one where we don't want to break string continuations. So I added an explicit exclude.

Copy link
Member

Choose a reason for hiding this comment

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

Using implicit concatenations on statement level feels strange to me, do we have any examples of that in the wild

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm I don't know. Is there a good way finding this out using your scripts? This was probably the hardest part of this PR (figuring out the rule when this is happening), which is why I would prefer not removing it until we're sure that we don't need it. I could add a TODo to experiment removing it once we have a ecosystem check in place.

@MichaReiser MichaReiser linked an issue Jun 23, 2023 that may be closed by this pull request
@github-actions
Copy link
Contributor

github-actions bot commented Jun 23, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

Benchmark

Linux

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.00      6.7±0.02ms     6.1 MB/sec    1.01      6.8±0.04ms     6.0 MB/sec
formatter/numpy/ctypeslib.py               1.00   1551.9±1.39µs    10.7 MB/sec    1.00   1559.0±4.19µs    10.7 MB/sec
formatter/numpy/globals.py                 1.00    185.1±0.20µs    15.9 MB/sec    1.00    185.6±0.16µs    15.9 MB/sec
formatter/pydantic/types.py                1.00      3.5±0.01ms     7.3 MB/sec    1.01      3.5±0.03ms     7.3 MB/sec
linter/all-rules/large/dataset.py          1.00     13.2±0.08ms     3.1 MB/sec    1.00     13.3±0.06ms     3.1 MB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      3.3±0.01ms     5.1 MB/sec    1.00      3.3±0.01ms     5.0 MB/sec
linter/all-rules/numpy/globals.py          1.00    424.3±0.90µs     7.0 MB/sec    1.01    426.9±0.68µs     6.9 MB/sec
linter/all-rules/pydantic/types.py         1.00      5.8±0.04ms     4.4 MB/sec    1.02      5.9±0.07ms     4.3 MB/sec
linter/default-rules/large/dataset.py      1.00      6.6±0.02ms     6.2 MB/sec    1.01      6.7±0.03ms     6.1 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.00   1448.3±3.31µs    11.5 MB/sec    1.01   1462.4±3.55µs    11.4 MB/sec
linter/default-rules/numpy/globals.py      1.00    162.3±0.37µs    18.2 MB/sec    1.00    162.9±0.91µs    18.1 MB/sec
linter/default-rules/pydantic/types.py     1.00      3.0±0.01ms     8.5 MB/sec    1.00      3.0±0.01ms     8.5 MB/sec

Windows

group                                      main                                   pr
-----                                      ----                                   --
formatter/large/dataset.py                 1.02     10.9±0.58ms     3.7 MB/sec    1.00     10.6±0.39ms     3.8 MB/sec
formatter/numpy/ctypeslib.py               1.00      2.4±0.11ms     6.9 MB/sec    1.02      2.5±0.15ms     6.8 MB/sec
formatter/numpy/globals.py                 1.01   302.2±22.77µs     9.8 MB/sec    1.00   300.0±20.41µs     9.8 MB/sec
formatter/pydantic/types.py                1.00      5.4±0.26ms     4.7 MB/sec    1.02      5.5±0.28ms     4.6 MB/sec
linter/all-rules/large/dataset.py          1.00     20.4±1.06ms  2039.4 KB/sec    1.02     20.8±0.84ms  1999.6 KB/sec
linter/all-rules/numpy/ctypeslib.py        1.00      5.4±0.22ms     3.1 MB/sec    1.04      5.6±0.25ms     3.0 MB/sec
linter/all-rules/numpy/globals.py          1.00   634.7±33.63µs     4.6 MB/sec    1.04   659.1±34.71µs     4.5 MB/sec
linter/all-rules/pydantic/types.py         1.02      9.2±0.40ms     2.8 MB/sec    1.00      8.9±0.49ms     2.9 MB/sec
linter/default-rules/large/dataset.py      1.00     10.8±0.45ms     3.8 MB/sec    1.01     10.9±0.43ms     3.7 MB/sec
linter/default-rules/numpy/ctypeslib.py    1.01      2.3±0.10ms     7.3 MB/sec    1.00      2.2±0.11ms     7.4 MB/sec
linter/default-rules/numpy/globals.py      1.03   275.9±16.06µs    10.7 MB/sec    1.00   268.0±23.41µs    11.0 MB/sec
linter/default-rules/pydantic/types.py     1.02      4.8±0.20ms     5.3 MB/sec    1.00      4.7±0.20ms     5.4 MB/sec

@MichaReiser MichaReiser added internal An internal refactor or improvement formatter Related to the formatter labels Jun 23, 2023
@MichaReiser MichaReiser mentioned this pull request Jun 24, 2023
@@ -10,6 +11,14 @@ impl FormatNodeRule<StmtExpr> for FormatStmtExpr {
fn fmt_fields(&self, item: &StmtExpr, f: &mut PyFormatter) -> FormatResult<()> {
let StmtExpr { value, .. } = item;

if let Some(constant) = value.as_constant_expr() {
Copy link
Member

Choose a reason for hiding this comment

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

Using implicit concatenations on statement level feels strange to me, do we have any examples of that in the wild

@MichaReiser MichaReiser force-pushed the format-implicit-continuation branch from affb113 to cae5757 Compare June 26, 2023 12:32
@MichaReiser MichaReiser enabled auto-merge (squash) June 26, 2023 12:34
@MichaReiser MichaReiser merged commit 49cabca into main Jun 26, 2023
@MichaReiser MichaReiser deleted the format-implicit-continuation branch June 26, 2023 12:41
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.

String constant formatting
2 participants