Closed rrousselGit closed 1 month ago
I have tried the new formatter on a large code base, and I love the automatic addition and removal of comma's. I only found one case where I didn't agree with the tall style decisions (https://github.com/dart-lang/dart_style/issues/1540). In all other cases I am happy with the changes.
So FWIW: I am strongly in favor of automating the addition and removal of comma's.
(Side note: Maybe if we had lints with auto-fixes, we could do the automatic addition and removal of comma's there. But you'd probably need the whole implementation of the formatter in such lint to decide if something fits on one line or not.)
As I said, I'm also for automating it. That's not my point. I'm saying to not do it in the formatter ;)
having dartfmt automatically handle trailing comma always seemed like one of those ideas that sounds better than it is. a lot of decisions about formatting are controlled by these trailing commas and im not convinced any one algorithm will produce good results in all cases. meanwhile as you say, a configurable linter does a great job of managing this
It also doesn't solve the problem of "no more style issues during reviews", and can also make refactoring worse.
For example, the following:
final content = json.decode(
await rootBundle.loadString('assets/configurations.json'),
) as Map<String, Object?>;
Now formats as:
final content =
json.decode(await rootBundle.loadString('assets/configurations.json'))
as Map<String, Object?>;
I don't think that's acceptable to me. So I'd end-up refactoring my code to:
final content = json.decode(...);
content as Map<...>;
// TO-DO use content
End result?
,
vs splitting an expression in multiple lines)require_trailing_commas
that'd suggest splitting a line into two)I really don't know what to say here. Literally the first sentence in the proposal for the new style (#1253) is:
The main change is that you no longer need to manually add or remove trailing commas.
IMO part of the issue of dealing with commas in the formatter is: The rules behind when a comma should/shouldn't be added can be very complex. There will be the equivalent of "false positives" – where the formatter decided to add/remove a comma in a place where there is a more appropriate solution.
Yes, but that's true of every other whitespace change the formatter applies. Given an expression where there are multiple places it could split, the rules behind choosing which places to split are also very complex.
In fact, they are the same rules. The formatter already has to decide whether an argument list or collection literal should split or not. The only real differences compared to the old formatter with respect to commas are:
This strictness is useful for raising floor of how readable Dart code can be. Unreadable Dart code will likely become more readable with the Tall Style. But due to false positives, this strictness also lowers the ceiling of how readable Dart code can be. Nicely formatted code today can become worse.
Again, this is exactly true of all whitespace changes as well. The goal of the formatter is not to produce code that is more beautiful and readable than a diligent human spending unbounded time and using the full context of the surrounding code could hand format. The goal is to get close enough to that high bar that the remaining difference in readability isn't worth the large human cost to get it.
It's like driving a manual transmission. A skilled driver might drive better driving a stick. But is that the most valuable thing they can be doing with their time?
At the same time, fixing the formatter when there is an issue can be difficult. Millions of lines can be impacted.
I am well aware! 🫠 That's why I have gone through an absolutely laborious process over the past year to determine if moving to this style is a net win for the entire Dart community. We have pretty clear feedback that it is.
Also, style guides can also be context dependent. A Flutter Widget may want to use different style rules than pure Dart code. And to a lesser extent, packages may also come with their own styleguides within the confines of their public API. Yet the formatter cannot realistically decide to format things differently based on that context.
Yes. Again, this is equally true of whitespace. But I have found in practice that context rarely makes that big of a difference in how someone would format a piece of code outside of a few edge cases like tabular data in collection literals.
I really don't know what to say here. Trailing commas are effectively whitespace. They have absolutely no semantics in the language. Your argument against automating them is, as far as I can tell, an argument against automated formatting in general. Which, sure, if you don't like automated formatting, that's fine. But if you do want to use an opinionated automated formatter, you may as well let it handle your trailing commas too.
For example, we have diagnostics to:
- Sort imports, named parameters and members of a class.
- Differentiate
import relative.dart
vsimport 'package:my_package/absolute.dart
- Suggest using
'
vs"
vsr'
vsr"
based on the content of a String- Warn between
iterable.forEach
vsfor (final a in iterable)
- Constraint the name of function parameters/class members/files ...
And many of those capability have are more or less related to formatting. Here are some examples:
- unnecessary_parenthesis
- curly_braces_in_flow_control_structures
- cascade_invocations
- always_put_control_body_on_new_line
- prefer_expression_function_bodies
All of those rules can be considered fairly similar to trailing commas. In fact, I could see a world where someone would argue that those lint rules should be built directly into Tall Style.
One of the main reasons they are not is because of reversibility. For things like braces on control flow constructs and =>
for function bodies, I think users would be pretty unhappy if the formatter removed the braces on every if
that fit on one line or every function that could use =>
.
Basically, there is a line where we say everything below this is not semantically interesting enough to be worth requiring a human to hand maintain it. Everything above that line the formatter doesn't touch. Everything below that line, the formatter does.
What I think leads to a particularly bad user experience is when something crosses that line. If humans are able to author some formatting choices, but the formatter can output those same choices too, then you end up in a situation where the formatter can't tell "Did a human put this here, or did I put it here the last time I was run?" It's not a good tool if the formatter can create a mess that a human doesn't want, and then not even be able to clean up after itself.
The new style is explicitly designed to avoid that. We know from asking users and looking at how they format their code that most prefer multi-line argument lists to have trailing commas. That implies that when the formatter splits an argument list, it should add a trailing comma. Otherwise, if the formatter splits the argument list and doesn't add the trailing comma, it's just leaving a pointless task for a human to perform.
But if the formatter adds trailing commas but doesn't remove them, then, again, it ends up leaving work behind for a human.
The high level question I want to ask is: Today, when you decide to insert a trailing comma to manually force an argument list to split that wouldn't split otherwise, what process is going through your head? What is the criteria you are using to decide that, "Yes, it would fit, but this one particular argument list looks better if it's split anyway."?
I'm not asking rhetorically. Literally, what are the heuristics you're applying? And then, once you've answered that... can the formatter just implement them? If we had a version of the formatter that could literally read your mind and would split every argument list you wanted it to split, I presume you would be happy for it to add and remove the trailing commas.
How close can we get the formatter to that? Are the rules so complex and subtle (and the effect on readability so massive) that it's simply impossible to automate them? Or can we get close enough and then you can focus on the actual semantics of your code instead of worrying about commas and newlines?
There is a way to force a trailing comma: add an empty comment to the construction:
void main() async {
fn(
//
param1: 1,
param2: 2,
);
final map = {
//
'a': 1,
'b': 2,
};
}
This has long been the way to preserve line breaks in list and map literals. Since most regrets come from the formatter removing the trailing comma rather than adding it, this trick should cover a lot of regressions and maybe it should be promoted.
@munificent I think you didn't quite respond to the heart of this proposal.
I do want automated commas. I'm saying to move your heuristic to the analyzer_server instead of inside dart_style.
The end-result is mostly the same for whoever has those lints enabled, and I'd suggest enabling those rules by default.
We should be discussing lint vs dartfmt instead. Not "should we automate commas or not".
Regardless of what my heuristics are, I'd still prefer having the logic inside lints over dartfmt.
I am well aware! 🫠 That's why I have gone through an absolutely laborious process over the past year to determine if moving to this style is a net win for the entire Dart community. We have pretty clear feedback that it is.
People couldn't play with it until now either :).
I personally didn't foresee the consequences when looking at the proposal, and only started questioning it after using it.
I don't think upvotes on the issue are representative of this.
One of the main reasons they are not is because of reversibility. For things like braces on control flow constructs and => for function bodies, I think users would be pretty unhappy if the formatter removed the braces on every if that fit on one line or every function that could use =>.
I'm not sure. I think some people would argue differently. For example, @nex3 argued for automating function bodies: https://github.com/dart-lang/dart_style/issues/1532#issuecomment-2294602775
My understanding is, if we could automate =>
vs {}
, she'd prefer that.
I assume she believes the same for single-line if
statements (so without {}
) and co.
Similarly the very reason why this issue exists is because I am unhappy that the formatter removed a comma.
To me, dartfmt is inconsistent. We want to automate commas, regardless of if removing them makes some users unhappy. But we don't wan't to automate function bodies or braces because it could make some users unhappy.
I'm not asking rhetorically. Literally, what are the heuristics you're applying? And then, once you've answered that... can the formatter just implement them? If we had a version of the formatter that could literally read your mind and would split every argument list you wanted it to split, I presume you would be happy for it to add and remove the trailing commas.
I'd be happy if Dartfmt matched exactly what I want.
The gist of the issue is that the formatter tries to offer only one solution to please for everyone.
Dartfmt used to offer ways to please everyone, and we decided to remove it.
Although I'm more than happy to share my heuristics. I already did so on multiple occasions, in other issues. For example, "add a trailing comma if a multiline closure/list literal isn't the last argument". https://github.com/dart-lang/dart_style/issues/1528#issuecomment-2294447873
Anyway as I said, IMO we should be discussing lints vs dart_style as a way to automate commas instead. IMO Lint rules are a more flexible way to manage commas.
Dartfmt used to offer ways to please everyone, and we decided to remove it.
I think this is deeply inaccurate. Just take a look at the FAQ: the three highest-priority goals are consistency, ending debates, and freeing users from thinking about formatting: all things that are accomplished better by doing the same thing everywhere than they are by pleasing everyone. In case that's not explicit enough, the very next header is "I don't like the output!" which points out that that "may be a deliberate stylistic choice of the formatter that you disagree with". The header after that discusses why the formatter is not configurable, and that "the goal of dartfmt is not to automatically make your code look the way you like. It's to make everyone's Dart code look the same."
The formatter never tried to please everyone. The only reason there was ever an exception for commas was because Flutter-style code was unreadable without something like tall style but there wasn't yet consensus on whether it was worth making everyone else's styles match. And this was a notable aberration: in essentially every other case, the formatter has always been strictly opinionated, prioritizing consistency over customizability, and over pleasing everyone.
And there's substantial evidence beyond just the number of upvotes on #1253 that this is the correct approach, and one that's broadly popular with users over the long term. In the frontend world, by far the most popular formatter is Prettier which bills itself front and center as an "opinionated code formatter". Its option philosophy is clearly aligned with dart_style
: "Prettier has a few options because of history. But we won’t add more of them." "By far the biggest reason for adopting Prettier is to stop all the ongoing debates over styles."
Similarly, Python's most popular formatter is Black, whose tagline is "the uncompromising code formatter". Its very name comes from the Henry Ford quote against customizability, "Any customer can have a car painted any color that he wants so long as it is black".
Not only is pleasing everyone not a goal, trying to do it is actively harmful to the formatter's goals, and there's good reason to think that dart_style
's goals promote long-term user happiness and ecosystem health.
I'm not sure. I think some people would argue differently. For example, @nex3 argued for automating function bodies: #1532 (comment) My understanding is, if we could automate
=>
vs{}
, she'd prefer that. I assume she believes the same for single-lineif
statements (so without{}
) and co.
This is true! In my ideal world, the only style choices I'd ever make would be which sub-expressions to pull out into variables (none of them 😈) and where to put empty lines. Even if all the style choices were the precise opposite of what I do by hand, the value of having them standardized and automated is so high that I'd still prefer it.
But, critically, I also understand that different people have different thresholds for what they consider "semantically relevant" changes. I believe that reflecting the community consensus there—as with individual formatting decisions—is more important than meeting my personal preferences. I think 94.8% 👍s on a proposal that begins "you no longer need to manually add or remove trailing commas" is a very strong indication that most people don't consider trailing commas to be semantically relevant.
About the specific question of whether this should be a lint instead: leaving aside the philosophical debate of whether trailing commas count as "formatting", the empirical investigation of whether the formatter can determine often enough what to do with them, and the pragmatic concerns about whether splitting this out would provide enough value to be worth throwing out and rewriting all the work that's already gone into handling commas in the dart_style
, I think this is pragmatically infeasible.
Let's suppose for a moment your proposal is adopted. Consider the following code at width 20:
// 20 char limit: |
void main() {
myFunction(
"foo"
);
}
If you run dart fix
before you run dart format
, it'll produce this output:
// 20 char limit: |
void main() {
myFunction(
"foo",
);
}
...which dart format
can't pull into one line because there's a comma it can't remove. On the other hand, if you run dart format
before dart fix
it'll produce this output:
// 20 char limit: |
void main() {
myFunction("foo");
}
Even if you're willing to bite the bullet and accept making ordering significant (and the corresponding failure of reversibility), consider this:
// 20 char limit: |
void main() {
myFunction(
nested("foobar")
);
}
According to dart format
, this is fine—the only issue is that there's no comma after the argument to myFunction
, which isn't the formatter's job. So you run dart fix
, which doesn't know or care about line lengths, and it produces:
// 20 char limit: |
void main() {
myFunction(
nested("foobar"),
);
}
...but now you've gone over 20 characters! Okay, just run dart format
after dart fix
. That gives you
// 20 char limit: |
void main() {
myFunction(
nested(
"foobar"
),
);
}
Oh no! We're in violation of the comma rules again! There's no way to fix this in general without running dart format
and dart fix
one after the other over and over until they finally stop making changes. It's even possible that there's a case—or could be a case in the future—where they enter a loop and this process never terminates.
The core issue here is that trailing commas both affect and are affected by line length. Other fixable lints don't have this issue—even if they change the way dart format
ends up formatting code, the choice about whether to apply the fixes themselves isn't recursively dependent on details of whitespace. Moving this into the linter would fundamentally break the layering between the linter and the formatter.
And there's substantial evidence beyond just the number of upvotes on #1253 that this is the correct approach, and one that's broadly popular with users over the long term. In the frontend world, by far the most popular formatter is Prettier which bills itself front and center as an "opinionated code formatter". Its option philosophy is clearly aligned with
dart_style
: "Prettier has a few options because of history. But we won’t add more of them." "By far the biggest reason for adopting Prettier is to stop all the ongoing debates over styles."
Prettier is a lot closer to the current Dartfmt than the upcoming Tall style. Prettier does have an equivalent of trailing commas.
For example:
const x = { a: 42 };
const x = {
a: 42,
};
Both of these are valid formats. Removing/adding a newline after the {
would switch between one or the other.
That's exactly like trailing commas (but \n
is the trigger instead of ,
)
So Prettier is opinionated and offers controls over single vs multiline.
What Prettier automates is ironically the opposite of what Dartfmt wants to automate.
Prettier automates things like '
vs "
, or sometimes add (\n)
around an expression:
return <multiline>
<div>foo</div>
</multiline>;
becomes:
return (
<multiline>
<div>foo</div>
</multiline>
);
So in a sense, Prettier and Tall Style are taking opposite decisions here.
With regards to your examples about why the lint may produce incorrect outputs: The goal of those lints would be to have the end-result be strictly the same as how Tall Style currently works. The existing lint would change to match.
As such, I disagree with the assumption that the lint would introduce a comma before the formatter can format the file. The new linter probably would have for heuristic something among the lines of "Suggest a trailing comma only if, when formatting the file as is, dart_style doesn't remove some newlines".
So given:
// 20 char limit: |
void main() {
myFunction(
"foo"
);
}
Then the linter shouldn't suggest anything. And formatting will remove the newline.
And even if we don't do this change, this issue also assumes that we add a remove_unnecessary_comma
lint. As such, given:
// 20 char limit: |
void main() {
myFunction(
nested("foobar"),
);
}
The trailing comma would be flag as extra, and would be removed by Dart fix, wrapping whole function invocation in a single line.
Reversibility is fundamental to what's discussed after-all. The reversibility of trailing comma rules that we'd always end-up with the single line approach here if both require_trailing_comma
and remove_unnecessary_comma
are enabled.
The key is that if someone manually adds a comma and doesn't enable remove_unnecessary_comma
, then they can stick to multilines on purpose.
Also quick note:
I think 94.8% 👍s on a proposal that begins "you no longer need to manually add or remove trailing commas" is a very strong indication that most people don't consider trailing commas to be semantically relevant.
First, this isn't representative, as those :+1: are from before we could play around with Tall Style. I too upvoted the issue months ago.
By that standard, from the moment were able to play with Tall Style, many people have expressed that they don't like how Dartfmt removes trailling commas.
And if you look at all my comments in the issue starting from this one (https://github.com/dart-lang/dart_style/issues/1253#issuecomment-2292476724), they all received a fairly decent amount of upvotes. Not all of them are about trailing commas. But clearly it shows that a non negligible amount of people aren't quite satisfied with the current state of Tall Style. So I wouldn't take for granted that people are satisfied.
In fact, one of the very last comment is about using //
as a trailing-comma-like. https://github.com/dart-lang/dart_style/issues/1253#issuecomment-2298319028
If we remove trailing commas, only to have folks use //
, that's a downgrade.
I don't think you're actually addressing the points I'm making. The point isn't what Prettier's specific style looks like—different tools for different languages are always going to make somewhat different decisions. The point is that its philosophy is evidence of the value of opinionated formatters, and the high cost of pleasing everyone.
(In point of fact, though, Prettier's documentation is clear that "The semi-manual formatting for object literals is in fact a workaround, not a feature. It was implemented only because at the time a good heuristic wasn’t found and an urgent fix was needed." This is not an example of desirable behavior worth emulating.)
Similarly, it doesn't matter to what I'm saying whether or not the 👍s on the initial issue are specifically endorsing the current formatting (although I don't think 18 reacts are a reason to think many people are dissatisfied). The critical question is whether people think of trailing commas in principle as semantically meaningful, and the answer is clearly that they do not.
Finally, if you need to have a linter's behavior know how dart_style
will format a file, you've already lost—at that point you're just making a formatter option that's simply masquerading as a lint rule. And you can't remove a comma in the example you cite because a core role of tall style—if indeed the lint is "strictly the same as how Tall Style currently works"—is that all tall argument lists end in commas.
The point isn't what Prettier's specific style looks like—different tools for different languages are always going to make somewhat different decisions. The point is that its philosophy is evidence of the value of opinionated formatters, and the high cost of pleasing everyone.
I wasn't aware of the documentation saying that Prettier doesn't count this as a feature but a workaround. Fair enough.
I personally view Prettier's behaviour as very cool, and like how Dartfmt matched that.
Although I'd be curious to see how the JS ecosystem would react if Prettier decided to remove that behaviour. I'm sure there would be a decent amount of pushback against it. But that's all theoretical.
The critical question is whether people think of trailing commas in principle as semantically meaningful, and the answer is clearly that they do not.
It sounds like we have a significantly different interpretation of the events from the past week then. My understanding is that the recent reactions show that people clearly do view commas as semantically meaningful.
Although I base my opinion on a bit more than the issue. There have bit some discussions with various GDEs and co on private channels.
My understanding is that:
Of course, all of that is subjective. But I don't think it's as clear-cut as you're making it out to be.
Finally, if you need to have a linter's behavior know how dart_style will format a file, you've already lost—at that point you're just making a formatter option that's simply masquerading as a lint rule.
They share the heuristic. I don't think that's unreasonable.
Viewing it as "a formatter option that's masquerading as a lint rule" is interesting. I agree that there's a decent amount of overlap. But lint rules have less strict requirements on how often they can change, and how broad they are applied. Considering the complexity of the heuristics, I think there's value in loosening the process.
And to me that's consistent with other topics, such as other lint-rules or () =>
vs () {}
.
My point is: Either have Dartfmt automate everything, or nothing. I don't like the in-between state that Tall style is.
And you can't remove a comma in the example you cite because a core role of tall style—if indeed the lint is "strictly the same as how Tall Style currently works"—is that all tall argument lists end in commas.
Given:
// 20 char limit: |
void main() {
myFunction(
nested("foobar"),
);
}
Tall style would remove the comma and format it in a single line.
So a remove_unnecessary_comma
would suggest removing the comma, and the formatter would then wrap the code in a single line.
The "dart fix + dart format" combo with lint rules is supposed to have the same output as built-in comma manipulation. If the outcome is different, the lint rule was likely incorrect.
I think we have wandered off the path into philosophy. Earlier, I asked:
Today, when you decide to insert a trailing comma to manually force an argument list to split that wouldn't split otherwise, what process is going through your head? What is the criteria you are using to decide that, "Yes, it would fit, but this one particular argument list looks better if it's split anyway."?
I'm not asking rhetorically. Literally, what are the heuristics you're applying? And then, once you've answered that... can the formatter just implement them? If we had a version of the formatter that could literally read your mind and would split every argument list you wanted it to split, I presume you would be happy for it to add and remove the trailing commas.
I still don't have an answer to that.
@rrousselGit, if you want explicit control over splitting argument lists... why? Can you show me examples where you think the current tall style formatter is doing a bad job? For those examples, can you explain why it would be impossible to tweak the formatter's splitting heuristics to do a better job?
@munificent I already answered. See https://github.com/dart-lang/dart_style/issues/1539#issuecomment-2298166019 (the last paragraph).
Dartfmt has always been opinionated, but it's not a dictator either. It has some jiggle room.
Even if I raise an issue, there's no guarantee that Dartfmt will fix it. You're totally entitled to close an issue as "won't fix". The problem is in the "What can I do now that I've been rejected?".
Let's not forget that Dartfmt has a monopoly over formatting. Folks upset by Dartfmt changes don't have many alternatives. That jiggle room is currently the only way to move forward for folks who don't like a specific formatting.
And as I said multiple times: I dislike how inconsistent about customisation Dartfmt is.
Tall style only removes trailing commas, but keeps other ways to control formatting.
For example, whenever a () =>
spans over multiple lines, I always refactor it to () {}
. So given () => identifier
, if I rename identifier
, I have may to switch to () {}
.
That's the same problem Tall style is trying to solve by automating commas. But Dartfmt currently rejects solving that problem because it can upset some people. Yet we don't mind upsetting some people by removing trailing commas.
IMO it should be all or nothing.
As it is, I don't really feel the benefit of automating commas. Because I still have to make the mental effort of going through all changes to validate that I don't need any further change.
Given:
// 20 char limit: | void main() { myFunction( nested("foobar"), ); }
Tall style would remove the comma and format it in a single line.
That's not accurate. Formatting this on a single line would produce:
// 20 char limit: |
void main() {
myFunction(nested("foobar"));
}
which is well past the line length limit. The only correct tall-style formatting, and what the prototype actually produces with --line-length 20
, is this:
// 20 char limit: |
void main() {
myFunction(
nested(
"foobar",
),
);
}
There's no way for your proposed separation of concerns to produce this output without either making the formatter comma-aware or making the linter formatting-aware. Similarly, you can construct cases where there's a comma that the current tall style would remove, but that the linter can't possibly know should be removed without running a full formatting pass.
(In case you're willing to bite the bullet on that as well: note that it takes dart fix
about 3s to run over the moderately-sized Dart Sass codebase, and it takes dart format
about 8s to do the same. I don't think anyone would be pleased if linting time quadrupled just to support this lint.)
@munificent I already answered. See #1539 (comment) (the last paragraph).
OK, I'm working on #1528 and related issues now. If those are fixed, does that change how you feel about automating removing commas?
It would be more bearable. But I still think that makes Dartfmt a bit of dictator, and still believe that Dartfmt is inconsistent in that regard.
I'm not fan of how it was argued not to automate () =>
vs () {}
to not upset people, when it's the same with commas. I don't think this has been addressed yet.
I strongly oppose automatic trailing commas. Examples of what I don't like:
@override
Widget build(BuildContext context) {
return SizedBox(
width: 1.sw,
child: Column(
children: [
Container(
margin: EdgeInsets.only(
//...
),
//...
decoration: BoxDecoration(
//...
),
),
],
),
Text(
title,
style: TextStyle(
fontSize: 14.sp,
color: const Color(0xFF999999),
),
),
DelayButton(
//....
padding: EdgeInsets.symmetric(
horizontal: 18.w,
),
onTap: () {
refreshMethod();
},
decoration: BoxDecoration(
//...
),
),
],
),
mainWidget: Text(
//...
style: TextStyle(
//...
),
),
)
],
),
);
}
When writing complex programs in flutter, it is very easy to form deep nesting. At this time, reasonable indentation is the most important. I don't want my code to be occupied by a lot of meaningless brackets. The code structure level of the program should be intuitive and meaningful. The semantics of the code should not be split due to symbols and structures. The code style I like:
@override
Widget build(BuildContext context) {
return MaterialApp(
title: 'smart erp',
theme: ThemeData(
colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
useMaterial3: true),
home: Scaffold(
appBar: AppBar(
toolbarHeight: 32,
leading: IconButton(
onPressed: () {},
icon: Icon(Icons.home)),
leadingWidth: 48,
title: Row(
children: [
MenuBarWidget(),
Text('Smart Erp')]),
titleSpacing: 0,
actions: [
Icon(Icons.alarm),
Icon(Icons.add),
Icon(Icons.air)],
bottom: PreferredSize(
preferredSize: Size(100, 32),
child: ToolbarWidget())),
body: UiFramework(
explorer: ExplorerWidget(),
statusBar: StatusBarWidget())));
}
I'm going to go ahead and close this because I don't see anything concrete here that I can take action on. The overall proposal to automate adding and remove trailing commas has already gone through a thorough change process and we have clear feedback that the community overall approves of it.
I am of course 100% interested in other issues being filed for how we can approve the style that the formatter applies, but I believe the overall question of whether it should remove commas is settled.
Hello!
In my opinion, the Tall style changes tries to do two completely distinct things.
Both goals are valuable. But in my opinion, it isn't the responsibility of Dartfmt to add/remove commas. That's up to the linter. For example, by enabling require_trailing_commas.
I propose the following plan:
unnecessary_trailing_comma
lint rule, which would be comparable torequire_trailing_commas
. This enables the so-called "reversability" of the styleguide discussed in other issues.require_trailing_commas
/unnecessary_trailing_comma
accordingly to match the new style.unnecessary_trailing_comma
andrequire_trailing_commas
to core lint rules, such that they are enabled by default on all Flutter projects.dart fix
on save/format)() =>
vs() {}
;for
vswhile
; earlyreturn
vsif/else
;switch
statement vs expression ; ...This would achieve the same goal as automating commas through DartFmt. But lint rules can be toggled. New rules can be added at any time. And rules can do much more than adding or removing a comma (for example sorting named parameters, refactoring to early returns instead of if/else, refactoring between for/while, ...)
Why I think it is a bad idea to have the formatter deal with commas
IMO part of the issue of dealing with commas in the formatter is: The rules behind when a comma should/shouldn't be added can be very complex. There will be the equivalent of "false positives" – where the formatter decided to add/remove a comma in a place where there is a more appropriate solution.
This strictness is useful for raising floor of how readable Dart code can be. Unreadable Dart code will likely become more readable with the Tall Style. But due to false positives, this strictness also lowers the ceiling of how readable Dart code can be. Nicely formatted code today can become worse.
At the same time, fixing the formatter when there is an issue can be difficult. Millions of lines can be impacted. Also, style guides can also be context dependent. A Flutter Widget may want to use different style rules than pure Dart code. And to a lesser extent, packages may also come with their own styleguides within the confines of their public API. Yet the formatter cannot realistically decide to format things differently based on that context.
Lint rules do not suffer from those issues:
Similarly, lint rules are already commonly used for style guide purposes. While some diagnostics can be categorised as "here's a possible bug", many diagnostics are purely stylistic.
For example, we have diagnostics to:
import relative.dart
vsimport 'package:my_package/absolute.dart
'
vs"
vsr'
vsr"
based on the content of a Stringiterable.forEach
vsfor (final a in iterable)
And many of those capability have are more or less related to formatting. Here are some examples:
All of those rules can be considered fairly similar to trailing commas. In fact, I could see a world where someone would argue that those lint rules should be built directly into Tall Style.
The point being: Lint rules offer a much higher degree of flexibility for enforcing styleguides while offering a similar developer experience.
If we were to enable
require_trailing_commas
and the non-existingunnecessary_trailing_comma
by default in the Dart/Flutter SDK, we would still solve the "cons" listed in the Tall Style rfc.But we wouldn't lose out on the "pros". And we would be much more reactive to whatever issue that arises.