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

Add "// dart format width=80" comment in generated code #724

Closed
munificent opened this issue Oct 7, 2024 · 3 comments · Fixed by #725 or #733
Closed

Add "// dart format width=80" comment in generated code #724

munificent opened this issue Oct 7, 2024 · 3 comments · Fixed by #725 or #733
Assignees

Comments

@munificent
Copy link
Member

After many years of users asking for it, we are adding support to the formatter for project-wide page width configuration using a surrounding analysis_options.yaml file (dart-lang/sdk#56863). However, tools like source_gen don't necessarily have access to (or want to access) IO or know where generated code will end up on disk.

To avoid breaking CI by having the code get generated and formatted at one width and then get formatted to a different width when the user runs dart format, we also added support for putting a marker comment in a file to specify an explicit width, like:

// dart format width=80

When this comment is present at the top of the file (before any code, but can be after other comments), then that comment will take precedence and the file will reliably be formatted at that width regardless of any surrounding configuration. So if code generators insert that comment at the top of the code they generate, they don't have to worry about reading the user's configuration.

This issue is requesting that source_gen be updated to add that comment to the code it generates. Since it's just a comment, it's safe to add this at any time. Let me know if you have any questions/feedback and I'll do my best.

@jakemac53
Copy link
Contributor

cc @natebosch or @kevmoo could you take this on?

@natebosch natebosch self-assigned this Oct 8, 2024
natebosch added a commit that referenced this issue Oct 9, 2024
Closes #724

The 3 places where a Dart unit can be output are as a `LibraryBuildr`,
as a `PartBuilder`, or as a combined part. `SharedPartBuilder` output
does not need the width comment. Add a constant for the comment and use
it while writing each of the 3 types of output.
natebosch added a commit that referenced this issue Oct 9, 2024
Closes #724

The 3 places where a Dart unit can be output are as a `LibraryBuildr`,
as a `PartBuilder`, or as a combined part. `SharedPartBuilder` output
does not need the width comment. Add a constant for the comment and use
it while writing each of the 3 types of output.
@natebosch natebosch reopened this Dec 9, 2024
@natebosch
Copy link
Member

natebosch commented Dec 9, 2024

It turns out we have at least 1 project which relies on being able to specify an arbitrarily large width when they format the code in order to have predictable formatting for some particular parts of the generated code where a comment is expected in a certain position

I think it may be feasible to move this so that it inserts the comment during the formatDartCode callback, and this would let the use cases that override formatting handle it however they want.

The user visible impact is that the dart format width comment would always be the first line of the file, and it would precede any user defined headers.

@kevmoo @jakemac53 - any concern about that?

@kevmoo
Copy link
Member

kevmoo commented Dec 9, 2024

No concerns here, other than general fear about unknown unknowns

natebosch added a commit that referenced this issue Dec 10, 2024
Closes #724

A use case with a custom `formatCode` callback can reasonably use a
different line width than the default, so only add the width comment
when this package is responsible for performing a format.

Uses with a custom header will no longer slot the width comment in
following the header.

Some outputs will no longer have the comment. This is not breaking since
there has been no published since the comment was introduced.
mosuem pushed a commit to dart-lang/build that referenced this issue Dec 10, 2024
Closes dart-lang/source_gen#724

The 3 places where a Dart unit can be output are as a `LibraryBuildr`,
as a `PartBuilder`, or as a combined part. `SharedPartBuilder` output
does not need the width comment. Add a constant for the comment and use
it while writing each of the 3 types of output.
mosuem pushed a commit to dart-lang/build that referenced this issue Dec 10, 2024
Closes dart-lang/source_gen#724

The 3 places where a Dart unit can be output are as a `LibraryBuildr`,
as a `PartBuilder`, or as a combined part. `SharedPartBuilder` output
does not need the width comment. Add a constant for the comment and use
it while writing each of the 3 types of output.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants