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

[4068] Fix: --line-ranges dedents a # fmt: off in the middle of a decorator #4084

Merged
merged 5 commits into from
Dec 4, 2023

Conversation

riyaz489
Copy link
Contributor

@riyaz489 riyaz489 commented Dec 3, 2023

Description

When transforming fmt:off into standalone comments, a new node is generated for fmt:off, and in this procedure, the initial indentation of fmt:off is not retained. Black code consistently converts fmt:off/on into standalone comments before proceeding with additional formatting. Consequently, the formatting of fmt:off is disrupted, even when it is not within the specified line range.

I addressed this issue by retaining the indentation of fmt:off and incorporating it into the prefix of the new node

Additionally, I observed that black consistently formats comments, regardless of whether fmt:off is present or if the comments fall outside the specified line range.
I refrained from raising an issue for this because I was uncertain whether it was intended behaviour or not.

Fixes: #4068

Checklist

  • [ x] Add an entry in CHANGES.md if necessary?
  • [ x] Add / update tests if necessary?

@JelleZijlstra
Copy link
Collaborator

Thanks! cc @yilei

Copy link

github-actions bot commented Dec 3, 2023

diff-shades reports zero changes comparing this PR (a5dbe49) to main (a0e270d).


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

@yilei
Copy link
Contributor

yilei commented Dec 4, 2023

Thank you for the PR, the change looks good to me.

(My original thought would be to add extra info in the Leaf attributes since we already had to do it for a previous change, but this PR also works.)

CHANGES.md Outdated
@@ -8,7 +8,8 @@

### Stable style

<!-- Changes that affect Black's stable style -->
- Fix bug where `# fmt: off` automatically dedents even when it is not within the
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- Fix bug where `# fmt: off` automatically dedents even when it is not within the
- Fix bug where `# fmt: off` automatically dedents when used with the `--line-ranges` option even when it is not within the

It's good to explicitly mention the name of the option, especially since it is new.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for bringing it to my attention. I will make sure to take note of it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The requested modifications above have been completed.

@JelleZijlstra JelleZijlstra merged commit 3416b2c into psf:main Dec 4, 2023
45 of 46 checks passed
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.

--line-ranges dedents a # fmt: off in the middle of a decorator
3 participants