-
Notifications
You must be signed in to change notification settings - Fork 715
feat: add elaborators, completions, and hovers for named errors #8730
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
This reverts commit a3e4470.
|
Mathlib CI status (docs):
|
Co-authored-by: Joachim Breitner <[email protected]>
Co-authored-by: Marc Huisinga <[email protected]>
Co-authored-by: Marc Huisinga <[email protected]>
Co-authored-by: Marc Huisinga <[email protected]>
| logErrorAt id m!"There is no explanation associated with the name `{name}`. \ | ||
| Add an explanation of this error to the `Lean.ErrorExplanations` module." |
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.
Could this error also suggest how to add an error explanation, or would that be too fragile? :-)
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'll be adding a README with more detailed instructions to src/Lean/ErrorExplanations/ once I add that module (post-stage0 rebuild).
mhuisi
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.
All of my remaining comments are nitpicks, feel free to merge as you see fit! :-)
| /-- Example -/ | ||
| register_error_explanation TestDomain.Foo2 { | ||
| summary := "Error 2" | ||
| sinceVersion := "4.0.0" | ||
| } |
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.
One thing I noticed when playing around with this and writing some test error explanation myself was that forgetting the docstring on register_error_explanation results in the following unhelpful error:
unexpected token 'register_error_explanation'; expected command
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.
Oh, good catch! Improving this will require a syntax change and re-bootstrapping, though, so probably best left to a separate PR.
…prover#8730) This PR adds support for throwing named errors with associated error explanations. In particular, it adds elaborators for the syntax defined in leanprover#8649, which use the error-explanation infrastructure added in leanprover#8651. This includes completions, hovers, and jump-to-definition for error names. Note that another stage0 rebuild will be required to define explanations using `register_error_explanation`. --------- Co-authored-by: Joachim Breitner <[email protected]> Co-authored-by: Marc Huisinga <[email protected]>
…prover#8730) This PR adds support for throwing named errors with associated error explanations. In particular, it adds elaborators for the syntax defined in leanprover#8649, which use the error-explanation infrastructure added in leanprover#8651. This includes completions, hovers, and jump-to-definition for error names. Note that another stage0 rebuild will be required to define explanations using `register_error_explanation`. --------- Co-authored-by: Joachim Breitner <[email protected]> Co-authored-by: Marc Huisinga <[email protected]>
This PR adds support for throwing named errors with associated error explanations. In particular, it adds elaborators for the syntax defined in #8649, which use the error-explanation infrastructure added in #8651. This includes completions, hovers, and jump-to-definition for error names.
Note that another stage0 rebuild will be required to define explanations using
register_error_explanation.