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 normalize whitespace before fmt:skip comments #4146

Merged
merged 9 commits into from Jan 25, 2024

Conversation

RedGuy12
Copy link
Contributor

@RedGuy12 RedGuy12 commented Jan 8, 2024

Description

Resolves #4143. I attempted to fix #2970 (review) while I was at it. However, that's a larger undertaking since _contains_fmt_skip_comment() requires a mode parameter and make_comment() doesn't - I'd have to add it to every caller of make_comment() and each of its callers, etc. Let me know if it's desired anyway.

Checklist - did you ...

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

RedGuy12 and others added 2 commits January 8, 2024 11:23
Signed-off-by: RedGuy12 <paul@reid-family.org>
Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com>
Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com>
@JelleZijlstra
Copy link
Collaborator

Thanks. This will have to go into the preview style as it changes existing formatting, and it'll probably have to wait until the 2024 stable style is out.

I don't understand the statement that you're not addressing https://github.com/psf/black/pull/2970#pullrequestreview-932691073—what else is there in that comment that this PR is not fixing?

Signed-off-by: RedGuy12 <paul@reid-family.org>
@RedGuy12
Copy link
Contributor Author

what else is there in that comment that this PR is not fixing?

#4143 talks about normalizing whitespace before the #. #2970 (review) is about whitespace immediately after the #. I thought I might as well just fix both, but like I said, the content after the # is formatted separately and would be harder to code.

This will have to go into the preview style as it changes existing formatting

Done!

Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com>
Signed-off-by: RedGuy12 <61329810+RedGuy12@users.noreply.github.com>
Copy link
Collaborator

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

This looks great, thank you!

(technically this change does meet the stability policy, since it won't cause changes in already blackened code, but always nice to let things bake in preview...)

Copy link

diff-shades reports zero changes comparing this PR (ffde332) to main (59b9d85).


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

@hauntsaninja hauntsaninja merged commit a5196e6 into psf:main Jan 25, 2024
46 checks passed
@RedGuy12 RedGuy12 deleted the gh-4143 branch January 25, 2024 11:54
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.

Whitespace before fmt: skip comment is normalised
3 participants