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 a crash when a colon line is marked between # fmt: off and # fmt: on #3439

Merged
merged 2 commits into from
Dec 15, 2022

Conversation

yilei
Copy link
Contributor

@yilei yilei commented Dec 13, 2022

Description

Fixes #3436.

Note that the # fmt: on line is always normalized to the next statement so its indentation is moved in this case. This isn't ideal, but at least it shouldn't crash.

Checklist - did you ...

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

@github-actions
Copy link

github-actions bot commented Dec 13, 2022

diff-shades reports zero changes comparing this PR (f84f374) to main (7d062ec).


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

Copy link
Collaborator

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

I feel like we should write developer docs on how fmt: off/on/skip work. The logic is already complicated as-is and the edge cases are incredibly confusing sometimes...

Also technically this is invalid use of # fmt: off/on but this is just a limitation of Black since there's no way to represent a # fmt: on at the right indent level right between indention levels.

src/black/comments.py Outdated Show resolved Hide resolved
@ichard26
Copy link
Collaborator

Oh where are my manners?... Thank you very much for investigating and filing a PR, it's appreciated! 🖤

@ichard26 ichard26 added C: invalid code Black destroyed a valid Python file F: fmtoff fmt: off implementation labels Dec 14, 2022
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 a282181 into psf:main Dec 15, 2022
@yilei yilei deleted the fmtoff branch December 16, 2022 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: invalid code Black destroyed a valid Python file F: fmtoff fmt: off implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash on conditional inside # fmt: off, depending on comment indentation
3 participants