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

Activity

1. bersbersbers

   added

   T: bugSomething isn't workingSomething isn't working

   on Sep 10, 2023
2. JelleZijlstra commented on Sep 10, 2023

   

   JelleZijlstra

   on Sep 10, 2023

   Collaborator

   More actions

   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.
3. hauntsaninja

   added

   C: preview styleIssues with the preview and unstable style. Add the name of the responsible feature in the title.Issues with the preview and unstable style. Add the name of the responsible feature in the title.

   on Sep 11, 2023
4. hauntsaninja commented on Sep 11, 2023

   

   hauntsaninja

   on Sep 11, 2023

   Last edited by hauntsaninja

   Collaborator

   More actions

   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.
5. AlexWaygood commented on Sep 13, 2023

   

   AlexWaygood

   on Sep 13, 2023

   Last edited by AlexWaygood

   Contributor

   More actions

   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!
6. psf

   deleted a comment from on Oct 28, 2023
7. JelleZijlstra commented on Oct 28, 2023

   

   JelleZijlstra

   on Oct 28, 2023

   Collaborator

   More actions

   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.
8. JelleZijlstra commented on Jan 11, 2024

   

   JelleZijlstra

   on Jan 11, 2024

   Collaborator

   More actions

   #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.
9. hauntsaninja commented on Jan 11, 2024

   

   hauntsaninja

   on Jan 11, 2024

   Collaborator

   More actions

   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

   👍React with 👍2Reacted by Alex Waygood and GiGaGon
10. JelleZijlstra

    closed this as completedon Jan 30, 2024