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 type param formatting in funcdef with trailing comma #4255

Closed
wants to merge 11 commits into from

Conversation

sumezulike
Copy link
Contributor

@sumezulike sumezulike commented Feb 27, 2024

Description

This change adds additional checks to the decision whether to split a funcdef from the left or right-hand side.
Specifically, this adds the following condition to should_split_funcdef_with_rhs:
If a funcdef has type parameters and its parameters also have a trailing comma, return True, to indicate that the funcdef should not be split from the left first.

This fixes the bug described in #4254.

I am currently trying to also fix #3929 and #4071 since they seem related, but that seems to require a little more work. Any suggestions are appreciated.

Checklist - did you ...

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

@sumezulike sumezulike changed the title Fix type param comma Fix type param formatting in funcdef with trailing comma Feb 27, 2024
Copy link

github-actions bot commented Feb 27, 2024

diff-shades reports zero changes comparing this PR (eb47bf0) to main (f03ee11).


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

RedGuy12 and others added 7 commits February 28, 2024 08:27
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>
@sumezulike sumezulike marked this pull request as ready for review March 3, 2024 00:56
@sumezulike
Copy link
Contributor Author

I thought it might be best to just submit a fix for the bug and work on the other changes in a separate PR.
Happy about any feedback!

CHANGES.md Outdated
@@ -10,6 +10,11 @@

<!-- Changes that affect Black's stable style -->

- Don't move comments along with delimiters, which could cause crashes (#4248)

- Fixed a bug where lines were split incorrectly for function definitions with type
Copy link
Collaborator

Choose a reason for hiding this comment

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

This needs to be a change to the preview style instead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(And to be clear, the bigger ask here is that the style change itself should only be applied in the preview style, not just that the changelog entry be moved.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, so sorry about that, I misunderstood.

@sumezulike sumezulike marked this pull request as draft March 7, 2024 13:24
@sumezulike
Copy link
Contributor Author

Closing this in favor of #4272

@sumezulike sumezulike closed this Mar 10, 2024
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 wrapping on generic functions using the type parameter syntax
3 participants