Skip to content
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

Allow trailing commas in parameter and argument lists. #26644

Closed
5 of 6 tasks
lrhn opened this issue Jun 8, 2016 · 6 comments
Closed
5 of 6 tasks

Allow trailing commas in parameter and argument lists. #26644

lrhn opened this issue Jun 8, 2016 · 6 comments
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). customer-flutter type-enhancement A request for a change that isn't a bug
Milestone

Comments

@lrhn
Copy link
Member

lrhn commented Jun 8, 2016

To improve the maintainability of long argument and parameter lists, we allow the final argument or parameter to be followed by an optional comma. This allows a style where each argument or parameter is written on a line by itself, and moving one item up or down or between functions doesn't require special handling of the final item.
This matches the existing behavior for list and map literals and enum class declarations.

To be precise:

  • The grammar is changed to allow a single comma right after a final parameter or argument.
  • It does not allow multiple commas between items, a leading comma, or a comma in an empty parameter or argument list.
  • The comma goes before the final ')' in parameter lists with no optional parameters, and before the final '}' or ']' in parameter lists with optional parameters.
  • The comma goes before the final ')' in argument lists.
  • The comma is entirely optional and has no semantic meaning, it's purely a syntactic convenience for a particular style of writing parameter or argument lists.

Tracking bugs for individual tasks:

For later:

@harryterkelsen
Copy link
Contributor

Can you have a trailing comma after the '}' in a parameter list? eg:

void foo(
    String x,
    List y,
    {bool z: true},
    ) {
}

@lrhn
Copy link
Member Author

lrhn commented Jun 9, 2016

@hterkelsen
Trailing comma after }:
That was not the plan. This is intended for a style where you have one parameter on each line, and multiple parameters, and you can move the individual parameters around among themselves.

It handles the cases where you move the last parameter somewhere else (leaving a dangling comma on the previous line and, maybe, missing a comma after the moved parameter in the new position), or where you add a new last parameter after the existing one (now missing a comma on the previous line).

In your example, you put the single optional parameter on a line including the braces. You can only move that line to the end of another parameter list, so there is no advantage to allowing a comma after it.

Basically the feature is for anywhere where you could write , anotherparameter, but didn't. After the optional parameter brace isn't such a place.

pq added a commit that referenced this issue Jul 1, 2016
Adds parser support for trailing commas in parameter and argument lists.

Meta-tracking issue: #26644

In most cases, errors were preserved (and tested) across parsers with and without the flag set.  (The exception is a formal paramater list with an extra comma, like `(a,,)` which with trailing comma support enabled only reports a missing identifier; needless to say this is captured in a test and is up for further debate.)

Amazingly (and for better or worse), turning this option on in the parser (`parseTrailingCommas = true`) produces NO errors in the analyzer and server tests.

BUG=26647
R=brianwilkerson@google.com

Review URL: https://codereview.chromium.org/2116853002 .
@sethladd
Copy link
Contributor

For the VM and dart2js, do we need to use a flag to opt-in to the feature?

@harryterkelsen
Copy link
Contributor

No, it should be enabled by default.

On Tue, Jul 26, 2016, 1:18 PM Seth Ladd notifications@github.com wrote:

For the VM and dart2js, do we need to use a flag to opt-in to the feature?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#26644 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AB3uFRov42Of7-BxyL0HYjh0erwUzIpHks5qZkFYgaJpZM4IwrlZ
.

@kevmoo kevmoo added this to the 1.19 milestone Jul 28, 2016
@mit-mit
Copy link
Member

mit-mit commented Aug 8, 2016

From discussion in #26649 the style guide update will follow later, and from discussion in release meeting this feature is indeed complete for 1.19, so closing.

@mit-mit mit-mit closed this as completed Aug 8, 2016
@mit-mit
Copy link
Member

mit-mit commented Aug 8, 2016

Updating changelog in https://codereview.chromium.org/2220963002/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). customer-flutter type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants