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

[Meta] Support project-wide dart format page width configuration #56863

Open
4 of 5 tasks
munificent opened this issue Oct 7, 2024 · 5 comments
Open
4 of 5 tasks

[Meta] Support project-wide dart format page width configuration #56863

munificent opened this issue Oct 7, 2024 · 5 comments
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...).

Comments

@munificent
Copy link
Member

munificent commented Oct 7, 2024

After the new tall style issue, the two formatter issues with the most 👍 are both about configuring page width (1, 2). This has been the #1 feature request for many years with hundreds of users asking for it.

So we are adding support for project-wide page width configuration to the formatter. After weighing a number of ways to author this configuration, the best approach seems to be a combination of two options. Each helps address the problems of the other:

Look for a surrounding analysis_options.yaml file and let it configure the page width

When formatting a file on disk using dart format, we walk up the surrounding directories looking for an analysis_options.yaml file. If we find one and it has a section like:

formatter:
  page_width: 123

Then the file is formatted at that width.

This lets users author the width in one location in a parent directory (likely the root directory of a package), and then all files in that directory tree get formatted the same way. Since analysis_options.yaml files also support an include key to import another analysis_options.yaml file, organizations have a way to have a standardized formatting style shared across all of their projects.

The main problem with using analysis_options.yaml (or any other config file) is that code generators and other tools that format code that only exists in memory don't know what config file to use. If the code generator doesn't use the configured width, but the user then runs dart format on the whole directory, the generated code will be changed. Likewise, if CI tries to validate that every file is formatted using the configured width, that will fail because the generated stuff is formatted at a different width.

Allow a comment inside of a source file to indicate the page width for that file

If you put // dart format width=123 at the top of a file, then the file is formatted at that page width. This is simple to implement, and works with dart format, code generators, and any other tool that wants to format code. It works even without having access to file IO or any knowledge of where the formatted code will end up on disk.

But it's very tedious for users to have to remember to put this in every single file they author. If they forget, they get inconsistently formatted code. No user has asked for this. They want project-wide configuration.

Hybrid approach

We implement both of these. If there is both a project-wide page width set in analysis_options.yaml and the file has a page width comment, the latter takes precedence (just like a // @dart=1.2 language version comment takes precedence over the SDK constraint in a pubspec).

Mixing both approaches lets us avoid the problems with each:

  • Tools like dart format and IDEs look for a configured project-wide page width in analysis_options.yaml. This is implemented in dart format now (behind an experiment flag). The analysis_server package, which most Dart IDE integrations use, already has support for working with analysis_options.yaml files, so plumbing the page width from that to the files it formats should be straightforward.

    This gives users the project-wide configuration they want for the code they write and read.

  • Tools that generate Dart code and then immediately format in memory without having easy access to the surrounding analysis_options.yaml file can simply add a // dart format width=123 comment to the top of the generated code before they format it.

    Since that takes precedence over any project-wide configuration, then formatting that file in memory with no analysis_options.yaml file will produce the same results as when the file is later formatted with dart format or the IDE. That should mean that things like CI steps that check for formatted files won't end up breaking on generated code.

This approach does mean that generated files may have a different width than the user prefers. But these files are fairly rarely read, so that's a tolerable compromise to avoid having every library that works with generated code and the formatter become obligated to support searching for and reading analysis_options.yaml (which isn't possible in some contexts like build_runner).

Implementation tasks

The core formatter work is done now (but behind a flag). The work remaining is largely in other packages:

@munificent munificent added the area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). label Oct 7, 2024
@munificent munificent changed the title [meta] Support project-wide dart format page width configuration [Meta] Support project-wide dart format page width configuration Oct 7, 2024
@munificent
Copy link
Member Author

@dart-lang/analyzer-team, @dart-lang/language-team, @dart-lang/core-package-admins, any other important tools/workflows/packages that should handle this?

@parlough
Copy link
Member

parlough commented Oct 8, 2024

If you're going to use formatter as the top-level key (which makes sense), perhaps that name should be reserved on pub.dev. It seems analyzer plugins (and other plugin-like tools) often use top-level keys matching their package name for configuration.

@kevmoo
Copy link
Member

kevmoo commented Oct 8, 2024

@parlough 's comment reminds me about a similar discussion we had about namespacing "properties" in pubspec.yaml, too.

@munificent
Copy link
Member Author

Good idea, @parlough. Filed an issue and added it to the list. Thanks!

@sigurdm
Copy link
Contributor

sigurdm commented Oct 10, 2024

We should also ensure that pana respects that setting.

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, ...).
Projects
None yet
Development

No branches or pull requests

5 participants
@kevmoo @munificent @sigurdm @parlough and others