"More concise formatting for dummy implementations" can be less concise

[bersbersbers]

Describe the bug

TLDR: #3796 ("More concise formatting for dummy implementations") can make stuff longer.

To Reproduce

def long_function_with_sine_params_and_long_return(param, another_one) -> ReturnType:
    ...

which is fine in the current style, is now formatted, in the preview style, as

def long_function_with_sine_params_and_long_return(
    param, another_one
) -> ReturnType: ...

which is longer than before.

Expected behavior

I am not sure:

• Accept this new behavior?
• Allow ... on a line of its own if appending it to the previous line leads to line breaks?
• Disregard trailing ... in line-break computations?

Environment

• Black's version: 23.9.0
• OS and Python version: 3.11.5

Additional context

https://black.vercel.app/?version=stable&state=_Td6WFoAAATm1rRGAgAhARYAAAB0L-Wj4ADCAIxdAD2IimZxl1N_WlbvK5V-UA6POmCWAlBigHXJM-vgsJNFnnfVB1-uHJ4V0GUgpC_6VVP7iU_GTn8jdXsR0PplFpGKggfg_JrV_yEDHau7wqaLqM0PEYr5luk1ntOa9ao5e6APRTqQtZ8UMKr7aFf3fB-gB6FhVm9E3IiYA1BFBYgrpdvrEGlSdzRfQjDgAIrLDdzoDymqAAGoAcMBAAAxRUBuscRn-wIAAAAABFla

[JelleZijlstra]

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.

[hauntsaninja]

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.

[AlexWaygood]

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 ... on a line of its own if appending it to the previous line leads to line breaks.

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!

[JelleZijlstra]

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 ... instead always stays in the same place. I think that's better, even if it occasionally means more lines.

[JelleZijlstra]

#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.

[hauntsaninja]

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