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 specs for compilers to return diagnostic info from run/1 #6280

Closed

Conversation

JakeBecker
Copy link
Contributor

This was motivated by ElixirLS and was influenced by the Language Server Protocol's specification for diagnostics.

I expect people might have strong opinions. I'm eager for feedback :)

@JakeBecker
Copy link
Contributor Author

@michalmuskala @josevalim, this PR has been open for more than a month. It's fairly small, and getting it in would unblock me from adding these diagnostics to Mix's compilers. I'd appreciate if someone could find the time to review it. Thanks!

@josevalim
Copy link
Member

Apologies @JakeBecker! This completely fell through the cracks.

This PR looks good to me but it is too abstract. I would like to see at least one compiler converted to it so we could understand how the new contract would be used. Do you think it is possible?

Thank you! ❤️

@JakeBecker
Copy link
Contributor Author

Sounds good to me, I'll add an implementation to the PR.

@JakeBecker
Copy link
Contributor Author

@josevalim I implemented the diagnostic return values for the Elixir compiler. I also changed the spec for the diagnostics a bit. Please take a look. Thanks!

I preserved the previous default behaviour of killing the process when encountering compile errors, since that seems important when launching a Mix task from the CLI. You can pass a CLI option `--return-errors` if you don't want to kill the process and instead return the diagnostics.
@JakeBecker
Copy link
Contributor Author

@josevalim I added commits that modify Mix.Tasks.Compile and Mix.Tasks.Compile.All to return the diagnostic information.

I preserved the previous default behaviour of killing the process when encountering compile errors, since that seems important when launching a Mix task from the CLI. You can pass a CLI option --return-errors if you don't want to kill the process and instead return the diagnostics.

Please review. Thanks!

@JakeBecker
Copy link
Contributor Author

I'm closing this to break it up into smaller PRs that will be easier to review.

@JakeBecker JakeBecker closed this Aug 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants