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

Prevent f-string merge quote changes with nested quotes #4498

Merged

Conversation

MeGaGiGaGon
Copy link
Collaborator

@MeGaGiGaGon MeGaGiGaGon commented Oct 22, 2024

Description

This is my first PR to black, so any guidance is welcome.

This technically fixes, and is strongly related to, #4493, #4494, and #4495

The core theme between then is what I consider a workaround. Currently almost all of the f-string formatting code is commented out and marked todo, which includes the quote normalization. This can be bypassed by using string merging to forcibly change the quote used in f-strings, ie "" f''. From how it looks to me, this is not desirable, as all the same logic and edge cases will be needed whenever that code is ready, which is suboptimal. Ideally only one place would contain all the f-string logic, as from all the encountered edge cases it is quickly becoming very complex.

This is also why none of those mentioned issues are closed by this PR, since their underlying causes/edge cases aren't fixed, just made inaccessible until the f-string code is worked on/some sort of resolution is made.

Since this is a regression, and a very harsh option to fix these, I'd understand if these changes are rejected, in which case this should mostly serve as a question on what to do instead.

Since this is a regression, some tests had to be removed. I was not able to find any documentation on what to do when removing tests, so I just deleted them. They have now been changed so that the output matches with the altered functionality. I also added a simplified version of #4493 as a test case to make sure this fix actually works.

Checklist - did you ...

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

tests/data/cases/preview_long_strings.py Outdated Show resolved Hide resolved
src/black/trans.py Outdated Show resolved Hide resolved
@MeGaGiGaGon MeGaGiGaGon changed the title Prevent f-string merge quote changes Prevent f-string merge quote changes with nested quotes Nov 5, 2024
@JelleZijlstra JelleZijlstra merged commit 3fab5ad into psf:main Dec 4, 2024
46 checks passed
@MeGaGiGaGon MeGaGiGaGon deleted the prevent-f-string-merge-quote-changes branch December 4, 2024 04:52
luketainton pushed a commit to luketainton/PwnedPW that referenced this pull request Feb 10, 2025
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [black](https://github.com/psf/black) ([changelog](https://github.com/psf/black/blob/main/CHANGES.md)) | dependency-groups | major | `<25.0.0,>=24.10.0` -> `<25.2.0,>=25.1.0` |

---

### Release Notes

<details>
<summary>psf/black (black)</summary>

### [`v25.1.0`](https://github.com/psf/black/blob/HEAD/CHANGES.md#2510)

[Compare Source](psf/black@24.10.0...25.1.0)

##### Highlights

This release introduces the new 2025 stable style ([#&#8203;4558](psf/black#4558)), stabilizing
the following changes:

-   Normalize casing of Unicode escape characters in strings to lowercase ([#&#8203;2916](psf/black#2916))
-   Fix inconsistencies in whether certain strings are detected as docstrings ([#&#8203;4095](psf/black#4095))
-   Consistently add trailing commas to typed function parameters ([#&#8203;4164](psf/black#4164))
-   Remove redundant parentheses in if guards for case blocks ([#&#8203;4214](psf/black#4214))
-   Add parentheses to if clauses in case blocks when the line is too long ([#&#8203;4269](psf/black#4269))
-   Whitespace before `# fmt: skip` comments is no longer normalized ([#&#8203;4146](psf/black#4146))
-   Fix line length computation for certain expressions that involve the power operator ([#&#8203;4154](psf/black#4154))
-   Check if there is a newline before the terminating quotes of a docstring ([#&#8203;4185](psf/black#4185))
-   Fix type annotation spacing between `*` and more complex type variable tuple ([#&#8203;4440](psf/black#4440))

The following changes were not in any previous release:

-   Remove parentheses around sole list items ([#&#8203;4312](psf/black#4312))
-   Generic function definitions are now formatted more elegantly: parameters are
    split over multiple lines first instead of type parameter definitions ([#&#8203;4553](psf/black#4553))

##### Stable style

-   Fix formatting cells in IPython notebooks with magic methods and starting or trailing
    empty lines ([#&#8203;4484](psf/black#4484))
-   Fix crash when formatting `with` statements containing tuple generators/unpacking
    ([#&#8203;4538](psf/black#4538))

##### Preview style

-   Fix/remove string merging changing f-string quotes on f-strings with internal quotes
    ([#&#8203;4498](psf/black#4498))
-   Collapse multiple empty lines after an import into one ([#&#8203;4489](psf/black#4489))
-   Prevent `string_processing` and `wrap_long_dict_values_in_parens` from removing
    parentheses around long dictionary values ([#&#8203;4377](psf/black#4377))
-   Move `wrap_long_dict_values_in_parens` from the unstable to preview style ([#&#8203;4561](psf/black#4561))

##### Packaging

-   Store license identifier inside the `License-Expression` metadata field, see
    [PEP 639](https://peps.python.org/pep-0639/). ([#&#8203;4479](psf/black#4479))

##### Performance

-   Speed up the `is_fstring_start` function in Black's tokenizer ([#&#8203;4541](psf/black#4541))

##### Integrations

-   If using stdin with `--stdin-filename` set to a force excluded path, stdin won't be
    formatted. ([#&#8203;4539](psf/black#4539))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xNjQuMSIsInVwZGF0ZWRJblZlciI6IjM5LjE2NC4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJsaW50aW5nIl19-->

Reviewed-on: https://git.tainton.uk/repos/PwnedPW/pulls/283
Reviewed-by: Luke Tainton <luke@tainton.uk>
Co-authored-by: Renovate [BOT] <renovate-bot@git.tainton.uk>
Co-committed-by: Renovate [BOT] <renovate-bot@git.tainton.uk>
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