-
Notifications
You must be signed in to change notification settings - Fork 195
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
Add support for optional trailing commas in type argument and type parameter lists. #2430
Comments
While I'm not generally a huge fan of trailing commas, I agree completely that if we support them, we should support them uniformly. Heck, we already even allow them in |
Why? Is it personal preference or is there some non-obvious reason why you have that opinion? |
My personal preference is that the trailing comma has no semantic meaning, so it's effectively dead code. Your source should not contain unnecessary or dead code. The comma does allow you to more easily remove or shuffle elements of the comma-separated sequence without having to special-case the last one, at least when the sequence is formatted with one element per line. Then there is the hack of making dartfmt recognize the comma and putting the elements on a line of their own. Which makes sense, because that's the only situation where the comma has value anyway. That makes the comma meta-semantically significant. It makes a difference on how a tool behaves on the source code, so now it's not really optional any more. But dartfmt is opinionated and doesn't otherwise let you control how it formats the program. The trailing comma breaks that. You can add or remove the, otherwise irrelevant, comma in order to control layout of your program, when formatted by a program which exists solely to prevent you from having to worry about formatting. But, as @munificent said, we've gone down this road, we should take it to its conclusion and allow trailing commas in all comma-separated syntactic constructs. The ones we are lacking today are:
(I thought we might have missed |
Also initializer lists, with-clauses, implements-clauses, and on-clauses. Edit: and show-combinators and hide-combinators. |
The import "foo.dart" show show, hide hide baz; is currently valid, so we'll have to use look-ahead to see whether the With clauses and implements clauses have similar readability issues, but since Initializer lists are fair game. |
Thank you Lasse for the more detailed analysis of both styles.
I want to express the opinion that I don't think that is a good idea because it sounds like consistency for consistency's sake. (Wouldn't trailing semicolons have to be made optional as well for full consistency? where should the line be drawn?) This issue is motivated by the syntactical overhead that variance modifiers will add, and I think that optional trailing commas with the support from the dart formatter will make them more digestible. Furthermore, this is a more abstract argument, but since programming languages seem to evolve towards more expressive type systems, terms and types will eventually have to converge (see: https://en.m.wikipedia.org/wiki/Lambda_cube) |
I can see fixing this particular case without caring about other cases. I'd also be fine with fixing initializer lists at the same time, because they do have similar issues with long lists of verbose expressions. The rest are ... not as important, I agree. |
Here is an example of what that could look like: class Foo {
final int i;
final int j;
final int k;
Foo.a({
required int x,
}) :
i = 0,
j = 1,
k = 3,
;
Foo.b() :
i = 0,
j = 1,
k = 3,
;
Foo.c() :
i = 0,
j = 1,
k = 3, {
//
}
Foo.d() :
i = 0,
j = 1,
k = 3,
{
//
}
} I think |
Like Lasse said, I don't generally like having two ways to say the same thing where the difference isn't really useful. Just seems like a mostly needless bit of syntactic sugar. In your examples, I think Likewise, Personally, I would suggest we don't support trailing commas in initializer lists or show/hide clauses. I think it's reasonable to allow trailing commas when the commas-separated list is in some bracket-delimited construct. But it gets a lot weirder when the list is terminated by Even if we do allow trailing commas in these places, I'd want a lint that asks you to remove them if they are immediately before a |
@munificent, does the new kind of formatting cause trailing commas to be less relevant (perhaps even useless) in the situations mentioned above?:
|
For the record, I have found myself writing enums with both enum Foo {
something(Multiple, "arguments", here),
other(Multiple, "arguments", here),
more(Multiple, "arguments", here),
;
// ...
} I'm doing for the exact same reason as everywhere else: So that all the similar lines have the same syntax, and I can add, remove or reorder with having to change any Same reasoning applies to long lists of anything. (I'd allow trailing commas everywhere we have a comma separated list, and have the formatter automatically add or remove the last comma depending on whether it orders things as one-per-line or not. The one thing I don't want is having to choose, or manually having to add or remove it. It might not be practical for |
Here are 3 UX improvements that become available with optional trailing commas:
Screen.Recording.2024-01-31.at.10.13.00.mov
Screen.Recording.2024-01-31.at.10.13.56.mov
Screen.Recording.2024-01-31.at.10.14.47.mov
(See: dart-lang/dart_style#1253 (comment)) I don't think that it would always be helpful if the formatter had to ability to decide to remove optional trailing commas. I think that interpreting the optional trailing comma as a clear signal to the formatter that "I need things to be on different lines" would be more helpful. This approach wouldn't be able to come in the way of the UX improvements that I've pointed out above.
For the reasons mentioned above, I don't think that any changes to the formatter could entirely replace any optional trailing comma on the grammar level. |
With the new style, the formatter will add and remove trailing commas on your behalf. Whenever the formatter splits a construct that supports trailing commas across multiple lines, it will add a trailing comma. If the constructor is formatted onto a single line, it removes the trailing comma. (There are a couple of edge cases that won't go into here.) So, in general, the new style makes trailing commas more common because they'll appear whenever an argument list, parameter list, collection literal, etc. splits. But it makes them less important because as a user, you don't have to think about whether to add or remove them.
Yes, it will add and remove trailing commas here.
I don't think these do support trailing commas. If so, it will discard them.
Today I learned that catches support trailing commas. Filed dart-lang/dart_style#1424. Currently the formatter fails with a parse error, which is weird since it uses the same parser as the VM, which happily runs it.
Those don't support trailing commas, and I don't think should.
Likewise, those also don't support trailing commas. I really don't want us to support trailing commas in places like |
I would actually want trailing commas in initializer lists, after a final initializer, but not after a final super-constructor invocation. Because it allows reordering initializers. |
I mean... if there's a final super-constructor invocation after it, then those preceding initializers don't have trailing commas. They just have regular old separating commas, and those are most certainly required. :) |
Exactly. I want a comma after a final initializer, one that doesn't have a super constructor invocation after it |
Nooooo. I'm not aware of any widely used style for constructor initializers that doesn't put the subsequent |
Wouldn't it be great if the editor's "move line" was delimiter aware, so that moving a line that ends in Until then, some people will want every line to end in a comma, even if that puts a semicolon on the next line. |
For what it's worth, it was the Flutter team that pushed hardest for supporting trailing commas in parameter and argument lists and the hand-formatted Flutter repo does not use a style for constructor initializers that would benefit from trailing commas. They always put the It's possible that they would move the Or |
This issue is a feature request for allowing optional trailing commas in non-empty type argument lists and type parameter lists. The proposed change is purely syntactical and non-breaking.
Motivation
The dart formatter uses optional trailing commas in formal argument/parameter lists to guide the heuristics that it uses to produce its output. A trailing comma gives the dart formatter a signal that the members of the affected list should be indented and output on a new line (roughly). Optional trailing commas in type argument/parameter lists would allow the dart formatter to apply the same behavior that it does for formal argument/parameter lists to type argument/parameter lists.
#524 will increase the syntactical overhead of declaring type parameters. I believe that optional trailing commas with support from the dart formatter will make that syntactic overhead more manageable.
Example
The proposed change would make the following program a valid Dart program:
Grammar changes
The dart grammar would change from:
to
Related issue:
#1402 proposes a different policy. This is about type argument lists and type parameter lists only.
Edit:
An alternative way to handle this issue could be to make commas entirely optional see: elm/compiler#979
A discussion about optional commas that includes opposing views can be found here: elm-lang/elm-plans#2
The text was updated successfully, but these errors were encountered: