-
Notifications
You must be signed in to change notification settings - Fork 110
Add more context about dartfmt failures #132
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
Conversation
Closes #130. I validated internally this produces a nicer error message, such as: ``` [SEVERE]: Error formatting the generated source code for <REDACTED>|lib/focus_trap.dart which was output to lib/focus_trap.template.dart. This may indicate an issue in the generated code or in the formatter. Please check the generated code and file an issue on source_gen if appropriate. Could not format because the source could not be parsed: ```
lib/src/builder.dart
Outdated
| This may indicate an issue in the generated code or in the formatter. | ||
| Please check the generated code and file an issue on source_gen | ||
| if approppriate.""", | ||
| '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nit] remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually put this so the entire rest of the message is not formatted strangely - it's sort of a cludge with dartfmt. I can remove if you like, but then the error message doesn't stick into one paragraph.
lib/src/builder.dart
Outdated
| This may indicate an issue in the generated code or in the formatter. | ||
| Please check the generated code and file an issue on source_gen | ||
| if approppriate.""", | ||
| '' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blank string huh?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
lib/src/builder.dart
Outdated
| 'Error formatting generated source code for ${library.identifier} ' | ||
| 'which was output to ${_generatedFile(buildStep.inputId).path}.\n' | ||
| 'This may indicate an issue in the generated code or in the formatter.' | ||
| '\n' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider using '''...''' for multi-line things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've been moving to this format in a lot of places. ''' works best for things like top-level variables where the string doesn't have any long lines
- let's us break up the String arbitrarily and and separate from how it's broken in the output to fit 80 chars
- Doesn't introduce a weird jump to the left of the screen from an indented block when the output shouldn't be indented
kevmoo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also – want this internally ASAP?
Do we want to do a +1 in the pubspec + changelog and publish?
|
@kevmoo I sent a cooresponding CL to patch this internally. When you eventually sync, this will safely override it. If that's OK, I'll let you release normally. |
|
@matanlurey – hrm...not a good practice to start IMHO – very easy to publish this and do a normal sync...and avoid confusing folks maintaining 3rd-party |
|
Sure, but we're time sensitive right now. Given that @natebosch supports the package internally too I don't think it's a huge issue. Normally I would just wait for the sync. |
| Please check the generated code and file an issue on source_gen | ||
| if approppriate.""", | ||
| '''Error formatting generated source code for ${library.identifier} | ||
| which was output to ${_generatedFile(buildStep.inputId).path}.\n |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We ended up with a bad mix here - we don't need both ''' and \n...
* Add more context about dartfmt failures Closes dart-lang/source_gen#130. I validated internally this produces a nicer error message, such as: ``` [SEVERE]: Error formatting the generated source code for <REDACTED>|lib/focus_trap.dart which was output to lib/focus_trap.template.dart. This may indicate an issue in the generated code or in the formatter. Please check the generated code and file an issue on source_gen if appropriate. Could not format because the source could not be parsed: ``` * Update builder.dart
* Add more context about dartfmt failures Closes dart-lang/source_gen#130. I validated internally this produces a nicer error message, such as: ``` [SEVERE]: Error formatting the generated source code for <REDACTED>|lib/focus_trap.dart which was output to lib/focus_trap.template.dart. This may indicate an issue in the generated code or in the formatter. Please check the generated code and file an issue on source_gen if appropriate. Could not format because the source could not be parsed: ``` * Update builder.dart
Closes #130.
I validated internally this produces a nicer error message, such as: