-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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" can be less concise #3872
Comments
This would only happen if the signature is within a few characters of the requested line length. It's a little unfortunate, but I think it's better to stay consistent with other signatures and accept this behavior. |
Yup, what Jelle said. When writing the patch I thought this would be fairly uncommon. I'll leave this issue open since this is still in preview style and maybe this is much more common than I anticipated. But as it stands, this is a won't fix. Black generally prefers fewer, consistent rules. |
This is actually something I've noticed quite a few times when it comes to black's formatting of typeshed's stubs, so I don't think it's that uncommon. I quite like @bersbersbers's suggestion of allowing Having said that, we've had the current formatting rules in typeshed for a long time, and it's not hugely ugly; I don't think this should be a high-priority issue! |
I still think we should stick with the current behavior. With the proposed change, we'd have this sequence: # A short function name has ... right after the :
def short(param, another_one) -> ReturnType: ...
# A medium-length one has it on its own line
def long_function_with_sine_params_and_long_return(param, another_one) -> ReturnType:
...
# With an even longer one, it goes back to right after the colon:
def long_function_with_sine_params_and_long_return(
param: some_type, another_one: some_type
) -> ReturnType: ... With the current behavior, the |
#4042 (comment) brought up a related case that makes us more likely to output less concise formatting: if there is a comment after the # Before
class Parser(typing.Protocol):
def set_defaults(self, **kwargs: typing.Any) -> None:
... # pragma: no cover
# After
class Parser(typing.Protocol):
def set_defaults(
self, **kwargs: typing.Any
) -> None: ... # pragma: no cover In #4063 I originally tried to make it so ellipses with comments didn't count as dummy implementations, but that also caused problems because it changed formatting in stubs. This case seems more obviously bad, and maybe it's enough to justify backing out the dummy implementation change. Or we could add some special casing around comments in non-stub files, but that also feels unprincipled. |
Note that example doesn't repro as-is; you need a shorter line length or longer args or longer comment (and IMO it looks a little less bad if so) I think it's not worth backing out the change. #1797 was a pretty popular issue. This is probably the second biggest upgrade for me in 2024 style (first place being #2316). But given that this has been reported twice, it would be nice if we could still see diff-shades from this change |
Describe the bug
TLDR: #3796 ("More concise formatting for dummy implementations") can make stuff longer.
To Reproduce
which is fine in the current style, is now formatted, in the preview style, as
which is longer than before.
Expected behavior
I am not sure:
...
on a line of its own if appending it to the previous line leads to line breaks?...
in line-break computations?Environment
Additional context
https://black.vercel.app/?version=stable&state=_Td6WFoAAATm1rRGAgAhARYAAAB0L-Wj4ADCAIxdAD2IimZxl1N_WlbvK5V-UA6POmCWAlBigHXJM-vgsJNFnnfVB1-uHJ4V0GUgpC_6VVP7iU_GTn8jdXsR0PplFpGKggfg_JrV_yEDHau7wqaLqM0PEYr5luk1ntOa9ao5e6APRTqQtZ8UMKr7aFf3fB-gB6FhVm9E3IiYA1BFBYgrpdvrEGlSdzRfQjDgAIrLDdzoDymqAAGoAcMBAAAxRUBuscRn-wIAAAAABFla
The text was updated successfully, but these errors were encountered: