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

Enforce empty lines before classes/functions with sticky leading comments. #3302

Merged
merged 14 commits into from
Oct 26, 2022

Conversation

yilei
Copy link
Contributor

@yilei yilei commented Sep 30, 2022

Description

Enforce empty lines before classes/functions with leading comments:

EmptyLineTracker works on lines that haven't been split yet. When sticky leading comment is used before class/def lines, the tracker must look behind the possible comment lines. So I introduced a new LinesBlock class to keep track of those states.

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 Sep 30, 2022

diff-shades results comparing this PR (36ae752) to main (575220f). The full diff is available in the logs under the "Generate HTML diff report" step.

╭─────────────────────── Summary ────────────────────────╮
│ 6 projects & 33 files changed / 36 changes [+36/-0]    │
│                                                        │
│ ... out of 2 326 398 lines, 10 700 files & 23 projects │
╰────────────────────────────────────────────────────────╯

Differences found.

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

@yilei
Copy link
Contributor Author

yilei commented Oct 5, 2022

I pushed a few new commits to address a few unexpected diffs from diff-shades.

I read through the latest diff-shades results again, and they look all expected.

src/black/lines.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@felix-hilden felix-hilden left a comment

Choose a reason for hiding this comment

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

Thanks a ton! Some comments below, and one discussion point: would it make sense to consider the comments to be in the block? Instead of having an outer "semantic comment" I'm not 100 % sure, but that could simplify things so that we can use ordinary newline rules for the whole block. Maybe? 🤔

tests/data/preview/comments9.py Outdated Show resolved Hide resolved
tests/data/preview/comments9.py Show resolved Hide resolved
@yilei
Copy link
Contributor Author

yilei commented Oct 7, 2022

would it make sense to consider the comments to be in the block? Instead of having an outer "semantic comment" I'm not 100 % sure, but that could simplify things so that we can use ordinary newline rules for the whole block.

This is appealing. But I tried and couldn't yet find a way to actually simplify this by considering the comments to be in the block.

To decide whether a comment belongs to this block, we have to examine what the block is, as well as the newlines in the first leaf prefix. This logic is already part of EmptyLineTracker. So this would require a big surgery to redesign EmptyLineTracker and likely also the LineGenerator.

All the empty line logic is still contained inside the EmptyLineTracker, so I feel this PR's implementation isn't too bad.

@yilei
Copy link
Contributor Author

yilei commented Oct 10, 2022

PR updated to keep the current behavior for inner functions after code block open. This now only fixes #246.

openstack-mirroring pushed a commit to openstack/adjutant that referenced this pull request Jan 4, 2023
This aligns code with psf/black#3302

Depends-On: https://review.opendev.org/866943
Change-Id: Icfbca2fab8adeb677b980d01f797f194f580838e
openstack-mirroring pushed a commit to openstack/openstack that referenced this pull request Jan 4, 2023
* Update adjutant from branch 'master'
  to 6b1856b4eb43f56010988fd464d3077de5c765fd
  - Support tox4, update style to match Black 23.1a1
    
    This aligns code with psf/black#3302
    
    Depends-On: https://review.opendev.org/866943
    Change-Id: Icfbca2fab8adeb677b980d01f797f194f580838e
stanislavlevin added a commit to stanislavlevin/pyproject_installer that referenced this pull request Feb 1, 2023
See psf/black#3302 for details.
Fixes: #37

Signed-off-by: Stanislav Levin <[email protected]>
stanislavlevin added a commit to stanislavlevin/pyproject_installer that referenced this pull request Feb 1, 2023
See psf/black#3302 for details.

Fixes: #37
Signed-off-by: Stanislav Levin <[email protected]>
stanislavlevin added a commit to stanislavlevin/pyproject_installer that referenced this pull request Feb 2, 2023
See psf/black#3302 for details.

Fixes: #37
Signed-off-by: Stanislav Levin <[email protected]>
jcristau added a commit to jcristau/scriptworker-scripts that referenced this pull request Feb 16, 2023
Once upon a time black and pydocstyle disagreed on blank lines before
nested functions, and a comment would appease them both.
Since then, the latter has been changed to not complain about a blank
line (PyCQA/pydocstyle#426), and the former now
complains about the missing blank line even with the comment
(psf/black#3302).
jcristau added a commit to jcristau/scriptworker-scripts that referenced this pull request Feb 17, 2023
Once upon a time black and pydocstyle disagreed on blank lines before
nested functions, and a comment would appease them both.
Since then, the latter has been changed to not complain about a blank
line (PyCQA/pydocstyle#426), and the former now
complains about the missing blank line even with the comment
(psf/black#3302).
jcristau added a commit to jcristau/scriptworker-scripts that referenced this pull request Feb 20, 2023
Once upon a time black and pydocstyle disagreed on blank lines before
nested functions, and a comment would appease them both.
Since then, the latter has been changed to not complain about a blank
line (PyCQA/pydocstyle#426), and the former now
complains about the missing blank line even with the comment
(psf/black#3302).
jcristau added a commit to jcristau/scriptworker-scripts that referenced this pull request Mar 6, 2023
Once upon a time black and pydocstyle disagreed on blank lines before
nested functions, and a comment would appease them both.
Since then, the latter has been changed to not complain about a blank
line (PyCQA/pydocstyle#426), and the former now
complains about the missing blank line even with the comment
(psf/black#3302).
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 doesn't enforce two empty lines before functions on module level when they have a sticky leading comment
3 participants