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

Please honor intended *horizontal* grow with magic comma #2409

Closed
complyue opened this issue Jul 30, 2021 · 10 comments
Closed

Please honor intended *horizontal* grow with magic comma #2409

complyue opened this issue Jul 30, 2021 · 10 comments
Labels
R: rejected This will not be worked on T: style What do we want Blackened code to look like?

Comments

@complyue
Copy link

complyue commented Jul 30, 2021

Is your feature request related to a problem? Please describe.

#826 (comment)

... for collections that you know will grow, there was no way to anticipate it by exploding them into one-element-per-line. Why would you want that? Because it minimizes diffs and that is a thing Black advertises as a feature: minimizing diff noise.

Now with #826 effected, my short collections with trailing commas will all explode into multiple lines, introducing way too loud noise (as well as waste of vertical space) to my codebase.

I know they'll grow, but only within single line, in near future, I don't anticipate most of them grow into multiple lines. And when one collection shall pass the threshold from single line to multi line in farther future, that change should deserves a larger diff to give the correct visual.

My situation can not be solved with --skip-magic-trailing-comma introduced by #1288, as it'll remove my trailing "magic" comma, then either the last element or the first one, will appear unequal to other elements when I adjust their relative positions.

Describe the solution you'd like

I suggest that anticipated vertical grow to be expressed by the magic trailing comma plus at least 2 lines of elements, effectively making anticipated horizontal grow expressible & honored, by the magic trailing comma plus a single line of elements (within line length limit of course).

@complyue complyue added the T: enhancement New feature or request label Jul 30, 2021
@JelleZijlstra
Copy link
Collaborator

Could you give some examples of code that you'd like to work differently? I don't understand from your description why you can't use --skip-magic-trailing-comma.

@complyue
Copy link
Author

complyue commented Jul 30, 2021

With --skip-magic-trailing-comma, black will format

seq = [3, 2, 5,]

into

seq = [3, 2, 5]

while without it, black would give

seq = [
    3,
    2,
    5,
]

What I want is seq = [3, 2, 5,] kept intact. And black only give the later result if I write

seq = [
    3, 2, 5, ]

or

seq = [3, 2, 
    5, ]

or

seq = [3, 2, 5, 
    ]

(Actually seq = [ 3, 2, 5, ] is more preferable to me, but that's not this feature request is asking for)

@JelleZijlstra
Copy link
Collaborator

Thanks for the explanation. That's a change in formatting behavior that I'd be very unlikely to support. The existing magic trailing comma behavior already introduces more dependencies on prior formatting than I'd like, and this proposal would make it worse, which is going to be confusing for users and make Black-formatted code less consistent.

@complyue
Copy link
Author

To me this change is not extending the semantics of "magic trailing comma", but to polish a corner case where it made overly strong an assumption - the element list of a collection can only grow vertically.

@cooperlees
Copy link
Collaborator

I agree with Jelle here. I don't think the behavior you want is intuitive to what black users are now accustom too.

Just for my interest, what is the advantage of the trailing comma when a list is on one line? Or is it just the fact you have code with this trailing comma and you want to avoid changes?

  • i.e. seq = [3, 2, 5,]

@complyue
Copy link
Author

complyue commented Jul 30, 2021

@cooperlees That way I can intuitively select one or more elements together with the respective comma(s), and drag&drop to move the whole within the collection; that's at par with the vertical cases where Option ↓ and Option ↑ can be used to shift the whole line(s) up or down.

Without the uniformly present comma trailing each element, there's a bit more mental overhead in doing this.

And actually I did put trailing comma after last elements, more or less as a reminder that the function signature (i.e. argument list) is not stable yet. So lacking of final trailing comma in my codebase may give signal to myself & my team members, that changing that function signature should go over more careful impact analysis during the refactoring.

@complyue
Copy link
Author

complyue commented Jul 30, 2021

@cooperlees

I don't think the behavior you want is intuitive to what black users are now accustom too.

I don't think they'll keep all elements on a single line, plus the final trailing comma, if that's really "vertical grow" already on their mind, so I suggest their intuition will be maintained.

Actually I believe the original problem "magic trailing comma" solves, is black keeping contracting user's multi-lines-as-written, back into a single line, force-exploding might be a side effect that not directly demanded.

@153957
Copy link

153957 commented Aug 1, 2021

That way I can intuitively select one or more elements together with the respective comma(s), and drag&drop to move the whole within the collection;

But that won't work unless you also have a trailing space after the final comma which you can select and 'drag&drop'. Otherwise there will still be overhead/additional work when you, for example, move the last item to be the first.

@complyue
Copy link
Author

complyue commented Aug 1, 2021

Yeah, in my own language's formatter, both leading and trailing spaces - as written by user - are honored as well, so the practice works perfectly well. But black is not willing to respond to user directions by design, so I hesitate to make a similar (but orthogonal to this request) feature request.

Yet "magic trailing comma" is an exception to black's general opinion, and with black formatting Python, at least the result will be consistent & stable, so long as I perform a format action each time such a move is done. All rest are already acceptable, just left this bit, that for the trailing comma be preserved (and no explosion from originally single-line src, of course).

@felix-hilden felix-hilden added R: rejected This will not be worked on T: style What do we want Blackened code to look like? and removed T: enhancement New feature or request labels Jan 29, 2022
@felix-hilden
Copy link
Collaborator

Hi! As Jelle and Cooper have outlined already, this is likely not something we're willing to change in the style, so I'll be closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R: rejected This will not be worked on T: style What do we want Blackened code to look like?
Projects
None yet
Development

No branches or pull requests

5 participants