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: Don't move comments while splitting delimiters #4248

Merged
merged 8 commits into from Mar 1, 2024

Conversation

RedGuy12
Copy link
Contributor

@RedGuy12 RedGuy12 commented Feb 22, 2024

Description

Resolves #4220. Black moved the + at the end of the line to the start of the next line, then unintentionally moved any comments as well. This could cause comments to be merged. To fix this, I stopped Black from moving any comments with the delimiter. It only crashed when merging type: ignore comments, but I think it makes sense to keep all comments on their original line instead of following the delimiter. This does not break the stability policy since already-Blackened code will not change.

Depends on #4257 for lint to succeed.

Checklist - did you ...

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

Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com>
@RedGuy12 RedGuy12 marked this pull request as draft February 22, 2024 18:36
@RedGuy12
Copy link
Contributor Author

RedGuy12 commented Feb 22, 2024

Test failures and diff-shades crash are caused by

 bad_split3 = (
-    "What if we have inline comments on "  # First Comment
-    "each line of a bad split? In that "  # Second Comment
-    "case, we should just leave it alone."  # Third Comment
+    "What if we have inline comments on "  # First Comment  # Second Comment  # Third Comment
+    "each line of a bad split? In that "
+    "case, we should just leave it alone."
 )

Copy link

github-actions bot commented Feb 22, 2024

diff-shades reports zero changes comparing this PR (8e1dfd3) to main (0f18001).


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

Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com>
Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com>
Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com>
Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com>
@RedGuy12 RedGuy12 marked this pull request as ready for review February 27, 2024 17:28
@RedGuy12 RedGuy12 changed the title fix: Don't merge comments while splitting delimiters fix: Don't move comments while splitting delimiters Feb 27, 2024
Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com>
Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com>
@RedGuy12
Copy link
Contributor Author

I have extracted the unrelated refactorings to #4257 for ease of review. That needs to be merged before lint passes on this PR.

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

Thank you!

@JelleZijlstra JelleZijlstra merged commit e4bfedb into psf:main Mar 1, 2024
46 checks passed
@RedGuy12 RedGuy12 deleted the gh-4220 branch March 1, 2024 15:29
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.

black internal error on line-broken ==
2 participants