Address pre-existing trailing commas when not in the rightmost bracket pair #1621
Conversation
…t pair This required some hackery. Long story short, we need to reuse the ability to omit rightmost bracket pairs (which glues them together and splits on something else instead), for use with pre-existing trailing commas. This form of user-controlled formatting is brittle so we have to be careful not to cause a scenario where Black first formats code without trailing commas in one way, and then looks at the same file with pre-existing trailing commas (that it itself put on the previous run) and decides to format the code again. One particular ugly edge case here is handling of optional parentheses. In particular, the long-standing `line_length=1` hack got in the way of pre-existing trailing commas and had to be removed. Instead, a more intelligent but costly solution was put in place: a "second opinion" if the formatting that omits optional parentheses ended up causing lines to be too long. Again, for efficiency purposes, Black reuses Leaf objects from blib2to3 and modifies them in place, which was invalid for having two separate formattings. Line cloning was used to mitigate this. Fixes #1619
| normal_name = ( | ||
| but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying( | ||
| arg1, arg2, arg3 | ||
| ) |
There was a problem hiding this comment.
Note: formatting improvements!
| normal_name = ( | ||
| but_the_function_name_is_now_ridiculously_long_and_it_is_still_super_annoying( | ||
| [1, 2, 3], arg1, [1, 2, 3], arg2, [1, 2, 3], arg3 | ||
| ) |
| "b": 2, | ||
| }["a"] | ||
| if a == {"a": 1,"b": 2,"c": 3,"d": 4,"e": 5,"f": 6,"g": 7,"h": 8,}["a"]: | ||
| pass |
There was a problem hiding this comment.
This is what this PR is really fixing.
| "g": 7, | ||
| "h": 8, | ||
| }["a"]: | ||
| pass |
| "way to format strings!", | ||
| "Use f-strings instead!", | ||
| ) | ||
| old_fmt_string3 = "Whereas only the strings after the percent sign were long in the last example, this example uses a long initial string as well. This is another %s %s %s %s" % ( |
There was a problem hiding this comment.
Now Black doesn't use optional parentheses that do nothing.
| @@ -2722,6 +2730,7 @@ class StringTransformer(ABC): | |||
|
|
|||
| line_length: int | |||
| normalize_strings: bool | |||
| __name__ = "StringTransformer" | |||
There was a problem hiding this comment.
This gives a name to class-based transformers which otherwise would have to be queried with __class__.__name__ instead.
| result.extend(transform_line(transformed_line, mode=mode, features=features)) | ||
|
|
||
| if not ( | ||
| transform.__name__ == "rhs" |
There was a problem hiding this comment.
That's a bit hacky; maybe we should turn the Transformer type into a pair instead to store this extra information.
There was a problem hiding this comment.
Yes, it's hacky but then again redoing all transformers into classes for this one hack felt like over-engineering.
| # sys.settrace(tracefunc) | ||
| actual = fs(source) | ||
| # sys.settrace(None) | ||
| def _test_wip(self) -> None: |
There was a problem hiding this comment.
Should this be removed?
There was a problem hiding this comment.
I guess it does no harm and I keep recreating this kind of mini-test when poking at a problem, so I just kept it, just disabled. ¯\_(ツ)_/¯
This required some hackery. Long story short, we need to reuse the ability to omit rightmost bracket pairs (which glues them together and splits on something else instead), for use with pre-existing trailing commas.
This form of user-controlled formatting is brittle so we have to be careful not to cause a scenario where Black first formats code without trailing commas in one way, and then looks at the same file with pre-existing trailing commas (that it itself put on the previous run) and decides to format the code again.
One particular ugly edge case here is handling of optional parentheses. In particular, the long-standing
line_length=1hack got in the way of pre-existing trailing commas and had to be removed. Instead, a more intelligent but costly solution was put in place: a "second opinion" if the formatting that omits optional parentheses ended up causing lines to be too long. Again, for efficiency purposes, Black reuses Leaf objects from blib2to3 and modifies them in place, which was invalid for having two separate formattings. Line cloning was used to mitigate this.Fixes #1619