Skip to content

Add an optional configuration to preserve trailing commas. #1672

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

Merged
merged 2 commits into from
Mar 17, 2025

Conversation

munificent
Copy link
Member

In the old short formatter, when a delimited construct had a trailing comma, it had two effects:

  1. The construct would be formatted "Flutter style" where the closing bracket would be moved to the next line and the contents indented +2, as opposed to the earlier style where the bracket stays on the same line as the last element and the elements are indented +4).

  2. The construct is forced to split even if it otherwise wouldn't.

The new formatter always uses a Flutter-like style for commas-separated constructors, so trailing commas are no longer needed for point 1.

When it does split a construct, it always adds a trailing comma. This means that point 2 breaks reversibility. Because of that, and because many users don't want to decide for every construct whether it should split or not, the new tall formatter no longer does point 2. It will remove a trailing comma and collapse a construct if it decides to.

However, some users rely heavily on that feature and strongly prefer control over forcing argument lists and other constructs to split.

This PR restores that functionality with an opt-in configuration:

formatter:
  trailing_commas: preserve

When enabled, if a construct has a trailing comma, the formatter will always split it. Users that preferred that behavior from the old formatter can enable the option and continue to work that way.

This does not mean we are generally moving in the direction of a more configurable formatter. This was a behavior that the formatter already supported and we removed it to the detriment of some users' experience. We're restoring that existing behavior for those users who want it.

Fix #1652.

In the old short formatter, when a delimited construct had a trailing comma, it had two effects:

1.  The construct would be formatted "Flutter style" where the closing bracket would be moved to the next line and the contents indented +2, as opposed to the earlier style where the bracket stays on the same line as the last element and the elements are indented +4).

2.  The construct is forced to split even if it otherwise wouldn't.

The new formatter always uses a Flutter-like style for commas-separated constructors, so trailing commas are no longer needed for point 1.

When it does split a construct, it always adds a trailing comma. This means that point 2 breaks reversibility. Because of that, and because many users don't want to decide for every construct whether it should split or not, the new tall formatter no longer does point 2. It will remove a trailing comma and collapse a construct if it decides to.

However, some users rely heavily on that feature and strongly prefer control over forcing argument lists and other constructs to split.

This PR restores that functionality with an opt-in configuration:

```
formatter:
  trailing_commas: preserve
```

When enabled, if a construct has a trailing comma, the formatter will always split it. Users that preferred that behavior from the old formatter can enable the option and continue to work that way.

This does not mean we are generally moving in the direction of a more configurable formatter. This was a behavior that the formatter already supported and we removed it to the detriment of some users' experience. We're restoring that existing behavior for those users who want it.

Fix #1652.
@rrousselGit
Copy link

Could the option also be a flag on dart format?

This way the IDE could be configured to use dart format --trailing-commas=preserve and the CI could not the flag. Cf #1652 (comment)

@munificent
Copy link
Member Author

Could the option also be a flag on dart format?

This way the IDE could be configured to use dart format --trailing-commas=preserve and the CI could not the flag. Cf #1652 (comment)

Yes this PR adds command-line option syntax to enable it as well with the exactly the syntax you suggest here.

@jellynoone
Copy link

@munificent Thank you for working on this! I can't express the amount of gratitude I feel.

Just a few questions:

  1. How would method calls like this get formatted?
// original snippet
late Service service;

void main() {
  service.request(Request(method: 'GET', method: 'params', path: '/products/123',));
}

Current / default:

// page_width: 80
// trailing_commas: automate
late Service service;

void main() {
  service.request(
    Request(method: 'GET', method: 'params', path: '/products/123'),
  );
}

Expected, when preserved: ?

// page_width: 80
// trailing_commas: preserve
late Service service;

void main() {
  service.request(Request(
    method: 'GET',
    method: 'params',
    path: '/products/123',
  ));
}
  1. Can it be relied upon this new configuration to stay in place for the foreseeable future, or are there plans to eventually remove it?

Personally, I'm hoping this solution will stay in place indefinitely.

Note:
From observing the linked issue and the fuss around the formatter change in general it seems that there are two groups of developers:
a) small teams and solo devs,
b) open source projects, corporations, large teams

I'm in group a). For me, being able to express what I'm thinking as close as possible as I can to code, is crucially important (thus desire to control trailing commas) (it helps me be productive and more creative). But I can totally see how for code being worked on by a large number of contributors, a strict standard is more desirable. I hope dart_style will continue to support both of these groups. (This PR, gives me a great indication this is the case.)

Again thank you!

@munificent
Copy link
Member Author

@munificent Thank you for working on this! I can't express the amount of gratitude I feel.

Happy to help. :)

Just a few questions:

  1. How would method calls like this get formatted?
void main() {
  service.request(Request(method: 'GET', method: 'params', path: '/products/123',));
}

You'll get:

void main() {
  service.request(
    Request(
      method: 'GET',
      method: 'params',
      path: '/products/123',
    ),
  );
}
  1. Can it be relied upon this new configuration to stay in place for the foreseeable future, or are there plans to eventually remove it?

It's not a temporary feature and there are no plans to remove it in the future unless it becomes clear that it isn't useful to users.

@munificent munificent merged commit 71ac085 into main Mar 17, 2025
5 checks passed
@munificent munificent deleted the preserve-trailing-commas branch March 17, 2025 18:03
@Pingear

This comment was marked as abuse.

@maxfrees
Copy link

In which flutter version will this be available, I don't want to automatically remove the comma

@quyenlv-unicloud
Copy link

@maxfrees Flutter 3.32.0 released with this, you can enjoy now 👍

analysis_options.yaml

formatter:
  trailing_commas: preserve

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preserve trailing commas
7 participants