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

Unnecessary trailing commas are no longer removed if an expression fits in one line. #1699

Closed
maximdeclercq opened this issue Sep 13, 2020 · 2 comments
Labels
F: trailing comma Full of magic R: not a bug This is deliberate behavior of Black.

Comments

@maximdeclercq
Copy link

maximdeclercq commented Sep 13, 2020

Unnecessary trailing commas should be removed if an expression fits in one line.
This doesn't happen for multiline lists e.g.

To Reproduce:

  1. Run Black on the following code without arguments
test = ["a very short line", "another element", "something long", "lorem ipsum dolor sit amet"]
  1. Black produces the following output as expected
test = [
    "a very short line",
    "another element",
    "something long",
    "lorem ipsum dolor sit amet",
]
  1. Change the last element to "lorem ipsum", and run Black on it without arguments.
  2. Black changes nothing
test = [
    "a very short line",
    "another element",
    "something long",
    "lorem ipsum",
]

Expected behavior
Running Black on

test = [
    "a very short line",
    "another element",
    "something long",
    "lorem ipsum",
]

Should have the same result as running Black on

test = [
    "a very short line",
    "another element",
    "something long",
    "lorem ipsum"
]

Output:

test = ["a very short line", "another element", "something long", "lorem ipsum"]

Environment:

  • Version: 20.8b1
  • OS and Python version: Linux/Python 3.8.5

Does this bug also happen on master?
Yes, see this link.

@maximdeclercq maximdeclercq added the T: bug Something isn't working label Sep 13, 2020
@ichard26 ichard26 added R: not a bug This is deliberate behavior of Black. and removed T: bug Something isn't working labels Sep 13, 2020
@ichard26
Copy link
Collaborator

ichard26 commented Sep 13, 2020

Black used to remove the trailing comma if the expression fits in a single line, but this was changed by #826 and #1288. Now a trailing comma tells Black to always explode the expression. This change was made mostly for the cases where you know a collection or whatever will grow in the future. Having it always exploded as one element per line reduces diff noise when adding elements. Before the "magic trailing comma" feature, you couldn't anticipate a collection's growth reliably since collections that fitted in one line were ruthlessly collapsed regardless of your intentions. One of Black's goals is reducing diff noise, so this was a good pragmatic change.

So no, this is not a bug, but an intended feature. The reason why you're filing this issue is probably since we say the following in the (outdated) style documentation:

Unnecessary trailing commas are removed if an expression fits in one line. This makes it 1% more likely that your line won't exceed the allotted line length limit. Moreover, in this scenario, if you added another argument to your call, you'd probably fit it in the same line anyway. That doesn't make diffs any larger.

We missed that this paragraph became incorrect when the "magic trailing comma" feature was introduced. It was eventually fixed in commit 6b935a3, but that was after the stable documentation was released alongside Black 20.8b1.

Anyway, here's the documentation on the "magic trailing comma". Hopefully that helps and sorry for the possible confusion.

@ichard26 ichard26 added the F: trailing comma Full of magic label Sep 13, 2020
@zzzeek
Copy link

zzzeek commented Sep 28, 2020

it would be super awesome if this were to be the second thing that black allows to be configurable. because right now it's arbirary, if I have code like:

def x():
    foo = ('one', 'two', 'three')

    bar = ('one', 'two', 'three',)


now it becomes:

def x():
    foo = ("one", "two", "three")

    bar = (
        "one",
        "two",
        "three",
    )

that is terrible. edit: obviously we're just going to do the black thing and leave it but this seems prickly enough to have an option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: trailing comma Full of magic R: not a bug This is deliberate behavior of Black.
Projects
None yet
Development

No branches or pull requests

3 participants