-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Handle compiler diagnostics in Rust. #2445
Conversation
bd5acec
to
b59d403
Compare
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.
Very nice clean up!
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.
Looks good - just a few nits and loose ends - otherwise I'm ready to land.
@ry I think all the feedback has been addressed. |
Actually I need to upgrade prettier it looks like. It needs to be running on 3.4 TypeScript, because I had to use some new 3.4 syntax to get some of the code to work, and prettier doesn't like it. |
Ok, finally, should be all fixed now. |
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.
LGTM!
Resolves #1738
Replaces #2310
This PR is a more simplified approach to dealing with diagnostics. It only touches the cli in Rust.
It doesn't touch any of the
JSError
code, which means it doesn't align it to the TypeScript style of output. There is a trait though incli/diagnostic.ts
calledDisplayFormatter
which could be moved into core and simplify the logic of formatting the parts of aJSError
to align the TypeScript style, which could be tackled as a seperate PR.