-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
More concise formatting for dummy implementations #3796
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Addressed by #3799. |
Thanks for improving the comment! This is my testing PR, was looking at #1797 (comment) and wanted to see what diff-shades has to say about potential changes :-) |
Ah I didn't realize that, haha. I thought you were wondering what was up with the comment looking wonky :P |
10c96b1
to
c38a470
Compare
@@ -165,6 +165,13 @@ def is_def(self) -> bool: | |||
and second_leaf.value == "def" | |||
) | |||
|
|||
@property | |||
def is_stub_def(self) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mirrors is_stub_class
from a couple lines above
src/black/lines.py
Outdated
and self.previous_line.is_stub_def | ||
and (current_line.is_stub_def or current_line.is_decorator) | ||
): | ||
newlines = user_hint_before |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is perhaps controversial, but it allows users to group things as they want (e.g. in the test case, dummy and other are grouped separately) and solves some lookahead problems when dealing with decorators
src/black/lines.py
Outdated
@property | ||
def is_stub_def(self) -> bool: | ||
"""Is this line a function definition with a body consisting only of "..."?""" | ||
return self.is_def and self.leaves[-3:] == [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't this also match something like def f(x): y = ...
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is true (and true for the is is_stub_class
heuristic above as well). I've made the heuristic slightly better by including the colon. I didn't see anything adverse in the diff shades output.
Also note some of the pyi code we now use in linegen.py will trigger in some cases involving other statements, although it's not unreasonable when it happens.
I didn't like the way my previous implementation special cased decorators, so I've taken a more normalised approach in this latest commit. I could still check for current_line.is_stub_def and forcibly remove newlines... this would make Black remove several newlines in Protocols, but wouldn't help with overloads / decorators. I chose not to do this for consistency with pyi mode, where we allow users to insert newlines if they feel like. |
This change isn't in any release, and is currently only in the preview mode. If you don't want such new changes, use the stable style. Also, if you are using Black to format your code, probably best to not also run the PyCharm formatter, as we don't generally attempt to keep Black compatible with it. |
Notably the the preview style for "dummy implementations" is disabled in Pyink as it doesn't play well with our linter (yet). The change in _Black_ is from psf#3796. PiperOrigin-RevId: 573023762
Notably the the preview style for "dummy implementations" is disabled in Pyink as it doesn't play well with our linter (yet). The change in _Black_ is from psf#3796. PiperOrigin-RevId: 573023762
Notably the the preview style for "dummy implementations" is disabled in Pyink as it doesn't play well with our linter (yet). The change in _Black_ is from psf#3796. PiperOrigin-RevId: 573023762
Notably the the preview style for "dummy implementations" is disabled in Pyink as it doesn't play well with our linter (yet). The change in _Black_ is from psf#3796. PiperOrigin-RevId: 573023762
Notably the the preview style for "dummy implementations" is disabled in Pyink as it doesn't play well with our linter (yet). The change in _Black_ is from psf#3796. PiperOrigin-RevId: 573023762
Notably the the preview style for "dummy implementations" is disabled in Pyink as it doesn't play well with our linter (yet). The change in _Black_ is from psf#3796. PiperOrigin-RevId: 573023762
Notably the the preview style for "dummy implementations" is disabled in Pyink as it doesn't play well with our linter (yet). The change in _Black_ is from psf#3796. PiperOrigin-RevId: 574275989
Fixes #1797, in particular, see #1797 (comment)