Handle comments and metadata before variables more gracefully. #1612
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Comments and metadata before a declaration are always tricky because the formatter by default attaches comments to the innermost piece following the comment. So in: ```dart // Comment int field; ``` We want the comment attached to the Piece for the entire field declaration. But the formatter won't see the comment until it hits the `int` token and will end up attaching it to the piece for the type annotation which is embedded inside the field piece. That in turn means that the formatter will think there is a newline inside the type (because there is one between the comment and `int`) which then forces a split after the type annotation too: ```dart // Comment int field; ``` In most places, this is handled by having the surrounding SequenceBuilder grab the comment before visiting the subsequent declaration. But that doesn't work when you have a comment after metadata: ```dart @meta // Comment int field; ``` Now the comment isn't before the field declaration. It's stuck inside it. But we still want to hoist it out. This PR fixes that for every place I could find: fields, functions, variables, and for-loop variables. The latter is particularly hard because for-in loops have some weird formatting already and it's also just a weird place for splits to occur. I wish I had a cleaner more systematic way of handling these but despite trying for most of today, I haven't been able to come up with a cleaner approach. This at least gets the formatter producing better output. Fix #1604.
natebosch
approved these changes
Dec 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Comments and metadata before a declaration are always tricky because the formatter by default attaches comments to the innermost piece following the comment. So in:
We want the comment attached to the Piece for the entire field declaration. But the formatter won't see the comment until it hits the
inttoken and will end up attaching it to the piece for the type annotation which is embedded inside the field piece. That in turn means that the formatter will think there is a newline inside the type (because there is one between the comment andint) which then forces a split after the type annotation too:In most places, this is handled by having the surrounding SequenceBuilder grab the comment before visiting the subsequent declaration. But that doesn't work when you have a comment after metadata:
Now the comment isn't before the field declaration. It's stuck inside it. But we still want to hoist it out.
This PR fixes that for every place I could find: fields, functions, variables, and for-loop variables. The latter is particularly hard because for-in loops have some weird formatting already and it's also just a weird place for splits to occur.
I wish I had a cleaner more systematic way of handling these but despite trying for most of today, I haven't been able to come up with a cleaner approach. This at least gets the formatter producing better output.
Fix #1604.