Insert trailing comma when function breaks with single argument#8921
Insert trailing comma when function breaks with single argument#8921charliermarsh merged 1 commit intomainfrom
Conversation
|
36b90a4 to
c0f5f67
Compare
|
This leads to decreased similarity in most cases (but the test fixtures show improvement). I'm pretty lost on what Black's logic is for inserting the trailing comma vs. not. For example, Black adds a trailing comma to the second example here, but not the first... def _start_listening(
ws_event_callback: Callable[str, int]
) -> List[example.ExampleConfig]:
pass
def _start_listening(
ws_event_callback: Callable[List[str]]
) -> List[example.ExampleConfig]:
pass |
|
+1 for consistency |
|
I'm now leaning towards proceeding with this change. My best guess is that Black omits the trailing comma if the annotation contains a comma, which seems incorrect to me. |
konstin
left a comment
There was a problem hiding this comment.
+1 for the consistency, since it changes a bunch of files and decreases black compatibility, do we want to guard it behind e.g. preview?
|
Another idea is that we could only add the trailing comma if the argument itself breaks... So: # Add a trailing comma here...
def _start_listening(
ws_event_callback: List[
CallableButImagineItIsVeryLong[str, int]
],
) -> List[example.ExampleConfig]:
pass
# But not here...
def _start_listening(
ws_event_callback: Callable[List[str]]
) -> List[example.ExampleConfig]:
passThis would still be a deviation from Black in some cases, but it would avoid adding the trailing comma in the event that more arguments are added (since those arguments would stay on the same line). Eh, on second thought, I don't see why that would be better. (Filed an issue on: psf/black#4080) |
|
I think this is a bug and we don't need to gate it with preview; perhaps if the formatter was not in beta, I would feel differently. |
…9076) ## Summary In #8921, we changed our parameter formatting behavior to add a trailing comma whenever a single-argument function breaks. This introduced a deviation in the case that a function contains a single argument, but _also_ includes a positional-only or keyword-only separator. Closes #9074.
Summary
Given:
We should be inserting a trailing comma after the argument (as long as it's a single-argument function). This was an inconsistency with Black, but also led to some internal inconsistencies, whereby we added the comma if the argument contained a trailing end-of-line comment, but not otherwise.
Closes #8912.
Test Plan
cargo testBefore:
After: