Consider not adding wrapping parens as a result of a trailing comma (for some concatenated strings)

[kkom]

Describe the style change

I'm not sure if this is intended or not, but I noticed that sometimes in Black >=22.12.0 a trailing comma will trigger wrapping the concatenated string in parens, which otherwise wouldn't take place (if there was no trailing comma).

The suggestion is to make it so that the presence of the trailing comma would have no bearing on whether the concatenated string will or not be wrapped in parens.

PS: In this particular case the trailing commas are added's by Ruff's COM rules: https://beta.ruff.rs/docs/rules/#flake8-commas-com

Examples in the current Black style

(1) No trailing comma

Original string:

class Allow(NamedTuple):
    message: str

Allow(
    "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut"
    " labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco"
    " laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in"
    " voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat"
    " cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum."
)

Stays as is after applying Black 23.1.0 formatting.

(2) Trailing comma

Original string:

class Allow(NamedTuple):
    message: str

Allow(
    "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut"
    " labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco"
    " laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in"
    " voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat"
    " cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.",
)

Gets reformatted to this by Black 23.1.10:

class Allow(NamedTuple):
    message: str

Allow(
    (
        "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt"
        " ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation"
        " ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in"
        " reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur"
        " sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id"
        " est laborum."
    ),
)

Desired style

No wrapping parens would be added – neither in (1) No trailing comma nor in (2) Trailing comma.

I.e. this string would be legal Black formatting and would not be changed:

class Allow(NamedTuple):
    message: str

Allow(
    "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut"
    " labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco"
    " laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in"
    " voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat"
    " cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.",
)

Additional context

Recently, Black has been wrapping code in parens to increase clarity (at the cost of sometimes adding indentation and extra lines, but it's a conscious tradeoff). For context, here are some recent relevant issues and PRs: #3292 and #3307 (shipped in 22.12.0 afaik) CC @yilei @felix-hilden @JelleZijlstra who were active on them

[yilei]

FWIW: the single item case was touched on #3162 (#3162 added the paren wrapping behavior for literals, then #3292 expanded this behavior to function calls too, to be consistent), that this was an intentional choice in #3162 to increase clarity. Wrapping the single long string could indeed feel unnecessary, but since you could get away from it by not having the trailing comma, I decided to keep this behavior.

[JelleZijlstra]

I think the current behavior is fine; it's not inconsistent with how we treat magic trailing commas elsewhere.

[kkom]

Thanks for the context! Good to see that it's something you have already considered.

The only thing I'd like to add is that sometimes the presence of a trailing comma isn't a choice of the developer, but it's consistently enforced by a linting rule. E.g. Ruff's COM, whose goal is to limit git blame noise.

I'm referring to stuff like @ichard26 's comment on #3162 (review) :

but as long as you can remove the trailing comma and Black collapses the wrapping parentheses, it's not a big problem.

In other words, the escape hatch of not putting a comma isn't always there.

Did you consider this as well? Would it change your view?

Either way, thanks for the super quick replies here. I don't know what's the process / convention in this project, so I'll leave it up to you what to do with this issue!

[kkom]

Actually, there is one more thing I'd like to get context on – if you would be able to:

it's not inconsistent with how we treat magic trailing commas elsewhere

Is there a specific reason why trailing commas trigger this behaviour, or is it simply a convenient & consistent convention?

The one thing that comes to my mind is how trailing commas originally disambiguated tuples from parens, but I am not sure if this is the reason.

[JelleZijlstra]

Did you consider this as well? Would it change your view?

That rule comes from the flake8-commas project. The first sentence of its description reads: "Note: Black, the uncompromising Python code formatter, or add-trailing-comma can do all this comma insertion automatically. We recommend you use one of those tools instead." So I don't think it makes sense to adjust Black to match its behavior.

[JelleZijlstra]

See https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#the-magic-trailing-comma for more on the magic trailing comma.

[kkom]

Ah, got it! Thanks so much for the links!

Regarding the second one – the explanation of it being a "please don't collapse me" signal does feel intuitive, but I don't think it's really related to extra parens. Do you think it would make sense to add there something about the relationship between magic trailing commas and wrapping values in parens?

It's a genuine question – I'm personally not sure, as I also don't like to overdocument things that are arguably trivial.

[kkom]

Can this task be closed after #3640 ?