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

Skip trailing comma by default #3273

Closed
smurfix opened this issue Sep 16, 2022 · 6 comments
Closed

Skip trailing comma by default #3273

smurfix opened this issue Sep 16, 2022 · 6 comments
Labels
F: trailing comma Full of magic R: duplicate This issue or pull request already exists T: style What do we want Blackened code to look like?

Comments

@smurfix
Copy link

smurfix commented Sep 16, 2022

Bug description

Changing line lengths is not reversible because Black adds a trailing comma to some argument lists.

To Reproduce

def some_fn(*args):
    pass


some_fn(1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20)

Reformatting with --line-len=20 results in

def some_fn(*args):
    pass


some_fn(
    1,
    2,
    3,
    ...
   20,
)

Unfortunately, due to the trailing comma after the 20 that wasn't there before, re-black-ing with --line-length=80 doesn't undo this transformation.

I have to manually remove the comma to get the old formatting back.

Expected behavior

Changing the line length from X to Y and back to X should be a no-op, at least for reasonable values of X and Y.

@smurfix smurfix added the T: bug Something isn't working label Sep 16, 2022
@Jackenmen
Copy link
Contributor

This handling of trailing commas is intentional and this behavior is called "magic trailing comma", you can read about it here:
https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#the-magic-trailing-comma

@smurfix
Copy link
Author

smurfix commented Sep 16, 2022

This handling of trailing commas is intentional and this behavior is called "magic trailing comma",

@jack1142 Yes I know that Black doesn't reformat lines when there's a trailing comma. That doesn't mean that it's a good idea to add a comma when there wasn't one, esp. without an option to disable that behavior.

@TomFryers
Copy link
Contributor

TomFryers commented Sep 16, 2022

I think this is a duplicate of #1742. The --skip-magic-trailing-comma/-C flag is the best solution right now.

@ichard26 ichard26 added the F: trailing comma Full of magic label Sep 18, 2022
@felix-hilden felix-hilden added T: style What do we want Blackened code to look like? and removed T: bug Something isn't working labels Sep 25, 2022
@felix-hilden felix-hilden changed the title Changing line lengths should be reversible Skip trailing comma by default Sep 25, 2022
@felix-hilden
Copy link
Collaborator

I'd be willing to entertain us flipping the toggle in preview and eventually altogether in v23. But this is just me personally 😅 Not collapsing small collections is more irritating to me than slightly larger diffs, although the config option is an easy fix. This would get us a step closer to our whole "not taking previous formatting into account" philosophy. Thoughts by other maintainers?

@TomFryers
Copy link
Contributor

I think its new title makes it a duplicate of #2135 instead! That one's accumulated a few thumbs-ups, so it seems there is at least some support for this amongst users (including me). (Not that that means particularly much; it's a rather biased sample.)

@felix-hilden
Copy link
Collaborator

Oh good find, I'll close this and post my message there as well 👍

@felix-hilden felix-hilden added the R: duplicate This issue or pull request already exists label Oct 7, 2022
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: duplicate This issue or pull request already exists T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

5 participants