Skip to content

Do not wrap implicitly concatenated strings used as func args in parens.#3640

Merged
JelleZijlstra merged 1 commit intopsf:mainfrom
yilei:stringwrapping
Apr 28, 2023
Merged

Do not wrap implicitly concatenated strings used as func args in parens.#3640
JelleZijlstra merged 1 commit intopsf:mainfrom
yilei:stringwrapping

Conversation

@yilei
Copy link
Copy Markdown
Contributor

@yilei yilei commented Apr 7, 2023

Description

This reverts the change in #3307, as the changes that would have been introduced in the 2024 style are too destructive based on feedback. See #2188 (comment).

Checklist - did you ...

  • Add an entry in CHANGES.md if necessary?
  • Add / update tests if necessary?
  • Add new / update outdated documentation?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 7, 2023

diff-shades results comparing this PR (968bb6c) to main (a552f70). The full diff is available in the logs under the "Generate HTML diff report" step.

╭───────────────────────────── Summary ──────────────────────────────╮
│ 16 projects & 517 files changed / 40 051 changes [+15 966/-24 085] │
│                                                                    │
│ ... out of 2 425 728 lines, 11 516 files & 23 projects             │
╰────────────────────────────────────────────────────────────────────╯

Differences found.

What is this? | Workflow run | diff-shades documentation

@yilei
Copy link
Copy Markdown
Contributor Author

yilei commented Apr 28, 2023

@JelleZijlstra any opinion on this? I think the preview string processing overall is a very big change, so I think reverting #3307 would be beneficial to get it in the 2024 stable style.

Copy link
Copy Markdown
Collaborator

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

OK, let's change this back for now.

@JelleZijlstra JelleZijlstra merged commit e712e48 into psf:main Apr 28, 2023
@yilei yilei deleted the stringwrapping branch June 15, 2023 18:19
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jul 21, 2023
Summary:
I thought using the `--preview` flag would make the future transition from black 23.X to 24.X smoother, but unfortunately it is not as stable as I imagined. There is no guarantee that the result of running the linter with this flag will not change on minor upgrade, as we just saw with black 23.7.0 reverting a rule for string formatting (psf/black#3640).

The result in this diff was obtained in two steps:
 - adopt the latest expected future style for multiline concatatenated strings, by first running `arc lint --everything` after upgrading black to 23.7.0
 - remove the `--preview` flag from `.arclint`, and rerun `arc lint --everything`

The second step does not revert the first one because manually splitting long strings over multiple lines is already compatible with `black` 23.X (it just won't do it automatically for you).

Test Plan:
check we get the same result for all 23.X versions:
```
pip index versions black
for version in 23.1.0 23.3.0 23.7.0
do
    pip install black==${version}
    arc lint --everything
done
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14294
Fabcien pushed a commit to Bitcoin-ABC/ElectrumABC that referenced this pull request Jul 21, 2023
Summary:
I thought using the `--preview` flag would make the future transition from black 23.X to 24.X smoother, but unfortunately it is not as stable as I imagined. There is no guarantee that the result of running the linter with this flag will not change on minor upgrade, as we just saw with black 23.7.0 reverting a rule for string formatting (psf/black#3640).

The result in this diff was obtained in two steps:
 - adopt the latest expected future style for multiline concatatenated strings, by first running `arc lint --everything` after upgrading black to 23.7.0
 - remove the `--preview` flag from `.arclint`, and rerun `arc lint --everything`

The second step does not revert the first one because manually splitting long strings over multiple lines is already compatible with `black` 23.X (it just won't do it automatically for you).

Test Plan:
check we get the same result for all 23.X versions:
```
pip index versions black
for version in 23.1.0 23.3.0 23.7.0
do
    pip install black==${version}
    arc lint --everything
done
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14294
Fabcien pushed a commit to Bitcoin-ABC/ElectrumABC that referenced this pull request Jul 22, 2023
Summary:
I thought using the `--preview` flag would make the future transition from black 23.X to 24.X smoother, but unfortunately it is not as stable as I imagined. There is no guarantee that the result of running the linter with this flag will not change on minor upgrade, as we just saw with black 23.7.0 reverting a rule for string formatting (psf/black#3640).

The result in this diff was obtained in two steps:
 - adopt the latest expected future style for multiline concatatenated strings, by first running `arc lint --everything` after upgrading black to 23.7.0
 - remove the `--preview` flag from `.arclint`, and rerun `arc lint --everything`

The second step does not revert the first one because manually splitting long strings over multiple lines is already compatible with `black` 23.X (it just won't do it automatically for you).

Test Plan:
check we get the same result for all 23.X versions:
```
pip index versions black
for version in 23.1.0 23.3.0 23.7.0
do
    pip install black==${version}
    arc lint --everything
done
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14294
Fabcien pushed a commit to Bitcoin-ABC/ElectrumABC that referenced this pull request Jul 23, 2023
Summary:
I thought using the `--preview` flag would make the future transition from black 23.X to 24.X smoother, but unfortunately it is not as stable as I imagined. There is no guarantee that the result of running the linter with this flag will not change on minor upgrade, as we just saw with black 23.7.0 reverting a rule for string formatting (psf/black#3640).

The result in this diff was obtained in two steps:
 - adopt the latest expected future style for multiline concatatenated strings, by first running `arc lint --everything` after upgrading black to 23.7.0
 - remove the `--preview` flag from `.arclint`, and rerun `arc lint --everything`

The second step does not revert the first one because manually splitting long strings over multiple lines is already compatible with `black` 23.X (it just won't do it automatically for you).

Test Plan:
check we get the same result for all 23.X versions:
```
pip index versions black
for version in 23.1.0 23.3.0 23.7.0
do
    pip install black==${version}
    arc lint --everything
done
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14294
PiRK added a commit to Bitcoin-ABC/ElectrumABC that referenced this pull request Jul 24, 2023
Summary:
I thought using the `--preview` flag would make the future transition from black 23.X to 24.X smoother, but unfortunately it is not as stable as I imagined. There is no guarantee that the result of running the linter with this flag will not change on minor upgrade, as we just saw with black 23.7.0 reverting a rule for string formatting (psf/black#3640).

The result in this diff was obtained in two steps:
 - adopt the latest expected future style for multiline concatatenated strings, by first running `arc lint --everything` after upgrading black to 23.7.0
 - remove the `--preview` flag from `.arclint`, and rerun `arc lint --everything`

The second step does not revert the first one because manually splitting long strings over multiple lines is already compatible with `black` 23.X (it just won't do it automatically for you).

Test Plan:
check we get the same result for all 23.X versions:
```
pip index versions black
for version in 23.1.0 23.3.0 23.7.0
do
    pip install black==${version}
    arc lint --everything
done
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14294
Fabcien pushed a commit to Bitcoin-ABC/ElectrumABC that referenced this pull request Jul 26, 2023
Summary:
I thought using the `--preview` flag would make the future transition from black 23.X to 24.X smoother, but unfortunately it is not as stable as I imagined. There is no guarantee that the result of running the linter with this flag will not change on minor upgrade, as we just saw with black 23.7.0 reverting a rule for string formatting (psf/black#3640).

The result in this diff was obtained in two steps:
 - adopt the latest expected future style for multiline concatatenated strings, by first running `arc lint --everything` after upgrading black to 23.7.0
 - remove the `--preview` flag from `.arclint`, and rerun `arc lint --everything`

The second step does not revert the first one because manually splitting long strings over multiple lines is already compatible with `black` 23.X (it just won't do it automatically for you).

Test Plan:
check we get the same result for all 23.X versions:
```
pip index versions black
for version in 23.1.0 23.3.0 23.7.0
do
    pip install black==${version}
    arc lint --everything
done
```

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14294
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants