-
-
Notifications
You must be signed in to change notification settings - Fork 279
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
fix: map errors from formatting generated files back to their locations in the templ file #737
Conversation
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.
Thanks for looking at this!
return fmt.Errorf("%d:%d%s", pos.Line+1, pos.Col, suffix), true | ||
} | ||
|
||
func parseFormatterError(str string) (errLine, errCol uint32, suffix string, err error) { |
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 think it's better to inspect the error type.
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.
That is way better, thank you. I updated the code. Let me know what you think about the error formatting I'm using now.
@@ -221,7 +222,12 @@ func (h *FSEventHandler) generate(ctx context.Context, fileName string) (goUpdat | |||
|
|||
formattedGoCode, err := format.Source(b.Bytes()) | |||
if err != nil { | |||
return false, false, nil, fmt.Errorf("%s source formatting error: %w", fileName, err) | |||
mapped, ok := mapFormatterError(err, sourceMap) |
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.
As per the other comment, I think it would be better to look at the type of the error, and maybe even to rewrite the error that comes back.
A small test around that behaviour would be useful too, so we don't accidentally lose the behaviour in future work.
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.
Agreed, I'm stepping away for a bit and wanted to commit what I've got so far but I will add tests soon.
}{ | ||
{ | ||
name: "single error outputs location in srcFile", | ||
fileName: "single_error.templ.error", |
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.
The .error
suffix was necessary to prevent the test files from breaking the build because they would get pulled into any templ generate
running from the root.
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.
Looking good! Thanks for this. 😁
This is not a complete fix for the original issue #726. The generator does not run the resulting generated files through the go compiler so it is still possible to write a templ where the resulting go file doesn't compile but no error is output when running
templ generate
. I'd be happy to look into that as well but I wanted to put out a more minimal PR and get feedback first.This PR makes 2 main changes