Make the eager argument list splitting heuristic more conservative. #1700
Conversation
Some folks on the Flutter team pointed out that the previous rule is too aggressive and splits even simple common code like:
```dart
Text('Item 1', style: TextStyle(color: Colors.white));
```
That's probably simple enough to stay on one line. So this tweaks the heuristic to allow that to remain unsplit. Where the old heuristic split any argument list with a named argument that contained a nested named argument, this requires there to be at least *three* named arguments.
It's sort of an arbitrary cutoff, but it seems to do a good job when run on a large corpus.
|
You can see how this rule compares to the previous more aggressive on in this diff: |
|
This looks great, thank you @munificent! |
I don't have any gauge on whether a change like this would push more authors to using the trailing comma config, but IIRC there were examples in the trailing comma threads where they do want to split these cases. My personal preference leans towards the compact version, and I think it's the right choice given the trailing comma config backup option. |
I worry about that a bit too. But I suspect (hope?) that that's a small enough minority of users that this won't move the needle. I worry equally about angering users who want denser code. A somewhat worrisome aspect of eager splitting is that there's no way to opt out. No "dense mode", and no matter how wide you set your page width, if the formatter decides to eagerly split, it will. Given that, I think it makes sense to err on the side of being a little more conservative with the eager splitting.
True, but there were also cases there where a user wanted something to split that almost no one else would want. I think they are mostly outliers.
SGTM, thanks! |
There was a problem hiding this comment.
I'm late but I think this is a big improvement, thanks for following up on this. I even see one example where it fixed the weirdness with trailing dot operators:
Before
).copyWith(
a: TextStyle(color: LinkUtils.hexToColor("015fff")),After
),
).copyWith(a: TextStyle(color: LinkUtils.hexToColor("015fff"))),
Some folks on the Flutter team pointed out that the previous rule is too aggressive and splits even simple common code like:
That's probably simple enough to stay on one line. So this tweaks the heuristic to allow that to remain unsplit. Where the old heuristic split any argument list with a named argument that contained a nested named argument, this requires there to be at least three named arguments.
It's sort of an arbitrary cutoff, but it seems to do a good job when run on a large corpus.