-
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
Sensible minimum number of char width "won back" before indentation used. #2954
Comments
Hi! Tell me if I'm off base here, but isn't that essentially solved by our line length parameter? If we wanted to break only four letters after 88, we could just set the line length to 92. Maybe I misunderstood. |
I think what is meant is that Black should only break lines inside brackets if it results in sufficiently shorter lines. For example, Black currently formats f(9876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210) to f(
9876543210987654321098765432109876543210987654321098765432109876543210987654321098765432109876543210
) which results in a longer line. I think the argument is that if this happens (or if breaking lines gives a result that is shorter, but by less than x characters for some small x) then Black should put it onto one line instead. The current behaviour can be particularly problematic with nested lists. To take an extreme example, f([[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]]) goes from one line of 89 characters to 87 lines, 41 of which are over 90 characters! Perhaps it makes sense to be stricter about line lengths near the limit. If it’s 88, then an 88-character line is much more desirable than a 89-character line, but if your line is going to be 200 characters anyway, then is 201 really much of a problem? |
That probably is it, thanks! So I'll respond to that. And to clarify, we're only considering cases where the final formatting still breaks line length limits, right? To entertain the idea: I agree that it's a bit awkward when a line becomes longer from splitting the line. If we were to have some sort of heuristic for it, I'd argue that it should be simply "does it make the line shorter". So any lines that would with maximal splitting still be equally long or longer should not be formatted. Having any other rule seems arbitrary. And if we agree on that, then only the cases where we have raw bracket pairs or calls whose name is 1-2 chars (+2 brackets <= 4 for the indent) are affected. But to criticise: ehmm, I honestly didn't come up with examples where this rule would make formatting worse. I think my objections would boil down to adding complexity. The cases presented should be rare, but that isn't a great argument either. I'm cautiously open to this, but let's hear from other maintainers as well. |
In the original example the line does actually become shorter, although it's a marginal improvement. In @TomFryers's example it's true that we make the lines longer, but those examples seem pretty pathological (especially the deeply nested empty list) so I'm not sure it's worth worrying about. |
Exactly, there should be a definition of the minimum marginal improvement before using the indentation, e.g. if less than 4 chars improvement, then don't apply the indentation, because its not really adding any value, just making the code verbose and even less readible. |
In Django, the _("This is the perfect storm for this feature to often make code more verbose") |
Setting a minmum line width improvement of 4 chars would also solve this example. |
I had a look for cases of this in some of my code, and I think long string literals are by far the most common cause, and they’ll cease to be a problem when string processing leaves |
I have unfortunately no solution but in real code, I just encountered this problem like this approximately :
In this context, it is quite unaesthetic apart from the additional lines, it removes uniformity and hence it is less readable. |
Describe the style change
Black indents items in brackets to help fit more on the screen, however in some cases, one can now see 2 less lines of code, as a tradeoff for seeing 2 more characters. This extra whitespace, along with making the code more verbose could be undesirable. Maybe a sensible default should be chosen that unless the win of pulling the line left is less than x chars, then rather leave it as is. E.g. maybe the minimum win should be say, 4 chars (you guys decide).
Examples in the current Black style
Desired style
Additional context
Love using Black, it's a game changer thank you!
The text was updated successfully, but these errors were encountered: