feat: Configure automatic trailing commas
Description
This proposal is to change
formatter:
trailing_commas: preserve
to
formatter:
trailing_commas: automate
Personally, I think this is a much cleaner system and enables the formatter to do the work of making code more readable instead of relying on developers.
Requirements
- [ ] All CI/CD checks are passing.
- [ ] There is no drop in the test coverage percentage.
Additional Context
No response
@mtwichel can you post how the before and after will look like? I know we can find this on the docs, but since this is a proposal it would help the discussion to have this information directly on the issue
I’m in favor of preserving commas, although I get @mtwichel's point on it being more consistent I’ve also seen most packages having this configuration for their analysis options and there was a lot of debate when the formatter changed with this feature, so it feels the general consensus is that the community prefers preserving the commas.
also I personally like to have more control over this since it can leave you with odd formatting decisions and in the end the code becomes less readable, so I’m more inclined to leave the devs in charge of this since devs are the ones reading this code later.
in fact, this was initially a community request in #150
As I pointed out previously, I think most users would not expect this package to change the default formatter configurations. From the README:
This package provides lint rules for Dart and Flutter
Even though they're configured in the same file, the formatter is not same thing as the lint rules. I was actually pretty confused when I noticed that my commas weren't being automated.
So, at the least, I think it would make sense to make a note of the formatter configuration in the README. Even better, maybe formatter configs could go into a separate file so it's more explicit in users' analysis_options.yaml. Something like:
include:
- package:very_good_analysis/lints.yaml
- package:very_good_analysis/format.yaml
@Conrad598 thanks for chiming in!
I believe this also was introduced because we were having conflicts with the require trailing commas lint we enforce in our rules.
IRRC, you can encounter the formatter applying automate in cases where the analyzer warns you with this lint.
I'm realizing this feels like a tab vs spaces debate 😝
I completely agree with @Conrad598 that it's weird our lint package also effects the formatting of the code. That was why I originally opened this issue - my code stopped formatting the way it had and I realized my VGA had updated and broke that formatting. Ofc, it's easy to override in analysis options, but it felt weird that I had to make that update to preserve the default formatting.
so it feels the general consensus is that the community prefers preserving the commas.
Idk about this point @marcossevilla - the default that the Dart team has chosen is to treat commas as whitespace. If the community was truly against it, I think Dart would seriously look at their decision and reverse it. It's stood for a few releases now.
I think this comes down to opt-in vs opt-out. When I install VGA into my repo, I'm expecting to opt-in to a bunch of lint rules. I'm not expecting that I'm opting-in to a formatting decision. I'd argue a compromise like @Conrad598 pointed out would be good. At the very least, we should make sure it's well documented that VGA changes the default behavior of the formatter (it may already, but I certainly missed it and I'm pretty in tune with the changelogs).
Thanks for your consideration here :)