Do not wrap implicitly concatenated strings used as func args in parens. by yilei · Pull Request #3640 · psf/black (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation3 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

yilei

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 ...

@yilei

@github-actions

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

@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.

JelleZijlstra

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.

@yilei yilei deleted the stringwrapping branch

June 15, 2023 18:19

@kkom kkom mentioned this pull request

Jul 11, 2023

Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request

Jul 21, 2023

@PiRK

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:

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

@PiRK

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:

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

@PiRK

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:

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

@PiRK

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:

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

@PiRK

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:

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

@PiRK

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:

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

2 participants

@yilei @JelleZijlstra