-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add --skip-magic-trailing-comma #1824
Conversation
c11d066
to
5306206
Compare
You're right that using global variables won't be acceptable for this. Hiding the global variable by running a context manager is not fundamentally changing this either. Having an attribute on Leaf is what I would look at. |
Thanks! You'll notice from the diff that I meant to say |
@hauntsaninja thanks for following up on this, I'd completely missed the concept ack on the other issue |
@ambv if you have time to take a look, much obliged! |
Just bumping this! |
@@ -438,6 +438,9 @@ into one item per line. | |||
How do you make it stop? Just delete that trailing comma and _Black_ will collapse your | |||
collection into one line if it fits. | |||
|
|||
If you must, you can recover the behaviour of early versions of Black with the option |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we might need to be consistent here at some point:
(py38) mbpt-jelle:black jelle$ git grep behavior | wc -l
7
(py38) mbpt-jelle:black jelle$ git grep behaviour | wc -l
7
any idea when will this be released? |
@uriva unfortunately I can't give you a timeframe, but I do hope it will happen soon! Apologies for the lack of any clear messaging. |
@ichard26 what is this blocked on? or is releasing itself a difficult process? (not demanding anything, just curious) |
@uriva I honestly don't know 100% why a new release is blocked, but the general lack of time and specifically the lack of availability for the ones holding the power to create a release is probably why. Maybe @cooperlees can explain in more detail. |
who has this power? |
@uriva you can see the list here https://pypi.org/project/black/#data |
tangential comment: I found this option |
@indigoviolet you're not alone! I find it confusing too. I am planning to just move the documentation to ReadTheDocs where the stable version will be provided by default to avoid this mess in the first place (it's part of my effort to revamp the docs). Thanks for the feedback though! |
Opened #2064 to add this to CHANGES.md Separately, is there anything blocking a release that I could help with? |
I tried adding: skip_magic_trailing_comma = true to my |
Adding |
Lukasz indicated he'd accept such a PR in #1288 (comment)
This is a pretty straightforward combination of things from before and after #1612 and #826
Feedback I'm looking for:
Obviously the global variable proof of concept is unmergeable, but I'm not sure what the right solution is. Passing it around seems kind of untenable because
Leaf.append
is used in a million places /should_split_body_explode
has a daunting reverse call graph too (You could make store an attribute onLeaf
to make things a little easier, but that's pretty ugly as well). Maybe a context manager to set a global currentMode
?Black doesn't have many options, so there isn't a clear pattern for how to test these. I went with reusing expression.py, but I could do something more akin to
numeric_literals_skip_underscores
. Let me know!