-
Notifications
You must be signed in to change notification settings - Fork 1.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
Quick Fix: Add trailing commas #27084
Comments
cc @pq |
This will be very useful for us since we are using the dart linter and formatter in our precommit-ci workflow. Trailing commas provide better formatting for Flutter codebases and we want to enforce this. |
Wouldn't it be better to fix the formatter so that it works equally well either with or without a trailing comma, rather than to require all the extra trailing commas just to work around a formatter behavior you dislike? |
@bwilkerson, you're right. Maybe I should I reword: it would be a useful "flag" to enable for us. |
I don't think it's a matter of formatting. The Flutter codebase wants to keep every parameter/argument on a line by itself (at least for some code) and require a trailing comma so it's easy to move lines around. |
dart-lang/dart_style#446 contains some of the discussion |
Just to be clear, I have no objection to adding a quick-fix / assist, I just question whether there might be a better way to support this use case in the formatter than requiring extra characters in the code. |
That's a good question, but a hard one to hash out on a GitHub issue. If you want, throw a VC on my calendar and I'd be happy to talk about it. :) |
A bunch of things have changed on this front. In particular, we now generate code more liberally with trailing commas. Given that I'm wondering if we are "good enough" or would a cleanup / fix still add value? |
I'm still continually adding lots of commas on old code to improve formatting. |
@munificent : would this be something you'd consider for |
Ugh, this is a surprisingly deep rathole of an issue. It is not at all clear which places an ideal dartfmt should or should not place trailing commas. If you look inside Flutter's own framework code, which is presumably pretty scrupulous about this style, there is no clear rule for which argument lists are supposed to have them and which aren't. |
@munificent I wonder if we could take some inspiration from black. It seems to add a trailing comma, only if the line of code exceeds the character limit. So while this func("foobar", "barfoo", "asdf1234", "orange", "apple", "banana", "foobar", "barfoo", "asdf1234", "orange", "apple", "banana") Gets formatted to this func(
"foobar",
"barfoo",
"asdf1234",
"orange",
"apple",
"banana",
"foobar",
"barfoo",
"asdf1234",
"orange",
"apple",
"banana",
) This stays as-is. func("foobar", "barfoo") Similar rules are applied to parameter declarations, collection literals, etc. OTOH, dartfmt produces this: func(
"foobar",
"barfoo",
"asdf1234",
"orange",
"apple",
"banana",
"foobar",
"barfoo",
"asdf1234",
"orange",
"apple",
"banana"); Which IMO looks weird. |
There is now a quick fix for the require_trailing_commas lint rule |
Can this autofix during the formatter? |
No, sorry. Just during |
@srawlins With what version of Dart? |
It looks like this landed 12 months ago. I'm not sure what version of Dart that would be. 532c6d9 |
Now that the formatter respects trailing commas, our users are asking if they can apply a "quick fix" to add trailing commas to their code.
See the nice effect of having trailing commas here: https://github.com/flutter/flutter/wiki/Early-Access:-Auto-Formatting
(This quick fix should also be plumbed into Atom and IntelliJ)
Thanks!!
The text was updated successfully, but these errors were encountered: