Skip to content
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

Different formatting of function call with and without trailing comma in arg list #1253

Closed
Jackenmen opened this issue Jan 30, 2020 · 4 comments
Labels
T: bug Something isn't working

Comments

@Jackenmen
Copy link
Contributor

Describe the bug
Black formats function call differently depending on whether the arg list has a trailing comma or not.

To Reproduce

  1. Create file with this code:
from typing import Iterator, Union

import builtins
import itertools

objects: Iterator[Union[builtins.int, builtins.float, builtins.tuple]] = itertools.chain(
    [1, 2, 3],
    [1.5, 2.5, 3.5],
)
  1. Run Black on it with no arguments. Diff will look like this:
-objects: Iterator[Union[builtins.int, builtins.float, builtins.tuple]] = itertools.chain(
-    [1, 2, 3],
-    [1.5, 2.5, 3.5],
+objects: Iterator[
+    Union[builtins.int, builtins.float, builtins.tuple]
+] = itertools.chain(
+    [1, 2, 3], [1.5, 2.5, 3.5],
 )
  1. Create file with this code (only change is lack of trailing comma in function call):
from typing import Iterator, Union

import builtins
import itertools

objects: Iterator[Union[builtins.int, builtins.float, builtins.tuple]] = itertools.chain(
    [1, 2, 3],
    [1.5, 2.5, 3.5]
)
  1. Run Black on it with no arguments. Diff will look like this:
-objects: Iterator[Union[builtins.int, builtins.float, builtins.tuple]] = itertools.chain(
-    [1, 2, 3],
-    [1.5, 2.5, 3.5]
-)
+objects: Iterator[
+    Union[builtins.int, builtins.float, builtins.tuple]
+] = itertools.chain([1, 2, 3], [1.5, 2.5, 3.5])

Expected behavior
I would expect Black to format both of these the same - most likely in the way that second code snippet was formatted in.

Environment (please complete the following information):

  • Version: 19.10b0 and it also happens on master
  • OS and Python version: Windows 10/Python 3.8.1

Does this bug also happen on master?
Yes

Additional context
I imported the builtins module only so that the code is long enough to be split in multiple lines 😄

@Jackenmen Jackenmen added the T: bug Something isn't working label Jan 30, 2020
@sirosen
Copy link
Contributor

sirosen commented Feb 17, 2020

I believe this is a regression in 19.10b. I noticed this when I upgraded some of my projects recently.
Unnecessary trailing commas would be trimmed in 19.3b, but it isn't happening anymore.

I notice mostly when doing refactoring where a multi-line arg list now fits on one line, as I delete several elements and black will often now produce things like

myfunc(x, y, z,)

which looks strange.

I have no problem with the trailing comma being there (even though it looks weird to me), but either it needs to always be added, or it needs to be removed.
Otherwise, black is letting me make a decision about how my code looks! 😱

Looking at the changelog, I wonder if maybe this was related to the positional-only argument handling? It's the only thing which sounds relevant, but that's just a guess.

@JelleZijlstra
Copy link
Collaborator

It's because of #826.

@sirosen
Copy link
Contributor

sirosen commented Feb 19, 2020

Thanks for the link. It seems like this is on purpose, but #1169 makes it sound like this is a bug which is being fixed? But then the proposed fix doesn't seem to be a revert -- it's something else. So I'm confused about what's happening and whether or not this issue is being considered a bug.

When wrapping a collection over multiple lines, black will add a trailing comma to the last element of a collection even if you did not provide one. But when unwrapping a collection which has had elements removed, the presence or absence of the trailing comma changes the behavior.
The asymmetry feels strange and inconsistent to the user, even if it is on purpose and can be explained.

For example, if you add to a collection with no trailing comma, making it wrap, then remove elements later, the result is not the same as removing elements first, then adding.

@Jackenmen
Copy link
Contributor Author

AFAIK, this is an intended behavior of the magic trailing comma feature, and while the example from this issue also presents improper exploding, it has been fixed with #1288.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants