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

Fix placement for comments within f-strings concatenations #7047

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

charliermarsh
Copy link
Member

@charliermarsh charliermarsh commented Sep 1, 2023

Summary

Restores the dangling comment handling for f-strings, which broke with the parenthesized expression code.

Closes #6898.

Test Plan

cargo test

No change in any of the similarity indexes or changed file counts:

project similarity index total files changed files
cpython 0.76083 1789 1632
django 0.99957 2760 67
transformers 0.99927 2587 468
twine 0.99982 33 1
typeshed 0.99978 3496 2173
warehouse 0.99818 648 24
zulip 0.99942 1437 32

@charliermarsh charliermarsh marked this pull request as ready for review September 1, 2023 16:12
@charliermarsh
Copy link
Member Author

\cc @davidszotten

@charliermarsh charliermarsh added the formatter Related to the formatter label Sep 1, 2023
@davidszotten
Copy link
Contributor

note that black formats your first 3 examples differently (this may be due to existing/known differences)

(f"{1}" f"{2}")  # comment
(f"{1}" f"{2}")  # comment
(1, (f"{2}"))  # comment

@charliermarsh
Copy link
Member Author

@davidszotten - Ah thank you for checking that! It does make sense to me since we tend to break on trailing comments unlike Black, so I don't think it needs to block here.

@charliermarsh charliermarsh enabled auto-merge (squash) September 1, 2023 16:22
@charliermarsh charliermarsh merged commit dea6553 into main Sep 1, 2023
16 checks passed
@charliermarsh charliermarsh deleted the charlie/format branch September 1, 2023 16:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

formatter panic with comment between joined f-strings
2 participants