Black: Splitting of a function across multiple lines doesn't add a trailing comma

[hawkowl]

Operating system: Arch Linux
Python version: 3.6.5
Black version: master
Does also happen on master: yes

Consider the following line (unformatted):

return make_deferred_yieldable(threads.deferToThreadPool(self.hs.get_reactor(), self.hs.get_reactor().getThreadPool(), _do_hash))

Black formats it as thus:

return make_deferred_yieldable(
    threads.deferToThreadPool(
        self.hs.get_reactor(), self.hs.get_reactor().getThreadPool(), _do_hash
    )
)

My coworker @richvdh gathers that it should be formatted with a trailing comma after the threads.deferToThreadPool call for consistency with how multi-line lists are formatted, producing the following:

return make_deferred_yieldable(
    threads.deferToThreadPool(
        self.hs.get_reactor(), self.hs.get_reactor().getThreadPool(), _do_hash
    ),
)

Is this something Black should do? PEP8's trailing comma examples don't really cover this specific case, so I'm not sure :)

[zsol]

This is because black can't detect that the file you're formatting is python3.6. Trailing commas are invalid syntax in some cases in older versions of python. If you run black with --py36, you should get the result you want

[hawkowl]

@zsol This is a function call, not a definition -- I can verify that this syntax is very much Python 2.7 compliant :)

[zsol]

You're absolutely right, maybe I should read the code we're talking about ;)

[ambv]

On this kind of split I'm not putting trailing commas because any change to line will result in the following:

• if you remove an argument, the diff is only touching the changed line;
• if you add another argument, and it still fits on the same line, the diff is only touching the changed line;
• if you add another argument, and args no longer fit on the same line, all arguments are exploded so the diff touches the old line anyway.

In other words, the trailing comma doesn't add anything here. Better yet, it does up a character, making it more likely your arg list wouldn't fit in a single line.

Does this explanation make sense?

(note, we currently have a bug where a trailing comma is missing also from single-element collection literals that are exploded: https://github.com/ambv/black/issues/274; that will be fixed)

[richvdh]

Aren't the args to make_deferred_yieldable already exploded?

The goal here is to be able to add new args to that call without touching existing lines.

(I'd also argue that there should be a comma after _do_hash for consistency, but that is a different case and your logic is valid there)

[ambv]

Aren't the args to make_deferred_yieldable already exploded?

Yes, there is one argument and it doesn't fit in one line, hence there is a trailing comma at the level of this function. However, arguments to threads.deferToThreadPool all fit in one line, hence the rules outlined by me above apply.

[richvdh]

Yes, there is one argument and it doesn't fit in one line, hence there is a trailing comma at the level of this function.

I think you are mistaken. Have another look at @hawkowl's report.

[ambv]

Oh, yes, you're right. A single argument doesn't generate a trailing comma. I guess it should if that argument itself doesn't fit in one line.

Good catch, we'll fix it!

[ambv]

This will likely get solved with https://github.com/python/black/pull/826.

[ambv]

This will be handled as part of #1288.