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

💅 Implement a handler based on grace #107

Open
favonia opened this issue Oct 18, 2023 · 8 comments
Open

💅 Implement a handler based on grace #107

favonia opened this issue Oct 18, 2023 · 8 comments
Labels
enhancement New feature or request
Milestone

Comments

@favonia
Copy link
Contributor

favonia commented Oct 18, 2023

@johnyob has implemented the Rust-style diagnostic rendering at https://gitlab.com/alistair.obrien/grace https://github.com/johnyob/grace.

On the surface, it looks straightforward to adapt it into an asai diagnostic handler. Of course, the renderer will not satisfy our Level-2 display stability requirements, but I think we can implement a renderer with lower display stability requirements, which has the benefits of not relying solely on colors to highlight spans.

@favonia favonia added documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed labels Oct 18, 2023
@johnyob
Copy link

johnyob commented Oct 18, 2023

Hey 👋

Grace's current repository can be found here: https://github.com/johnyob/grace.

I'd recommend that asai doesn't use grace until early 2024 as I'm still (albeit slowly) working on grace's renderer.

@favonia
Copy link
Contributor Author

favonia commented Oct 18, 2023

@johnyob Thanks for the link---I just updated it.

If you are worried about the unstable API, asai is also very experimental. 😅 Also the core asai package will not depend on grace---we have a built-in terminal renderer and I planned to make the grace-backed handler a standalone package. Does that sound "safe" enough? OTOH, we can certainly wait until you feel the interface is more stable. 😁

@favonia favonia removed the help wanted Extra attention is needed label Oct 18, 2023
@favonia favonia added this to the far away milestone Oct 20, 2023
@favonia
Copy link
Contributor Author

favonia commented Oct 21, 2023

@johnyob Hi, I realized that you are using the package text but that package hasn't been updated for a while. More importantly, it currently does not support OCaml 5, but asai needs OCaml 5. Therefore, it's currently impossible to build a handler based on grace 😢

@favonia favonia removed the documentation Improvements or additions to documentation label Oct 21, 2023
@johnyob
Copy link

johnyob commented Oct 23, 2023

Yes, in the initial release of grace, this dependency will be removed and the library will be suitable for OCaml 5. I intend to push this work at the beginning of January 2024

@favonia
Copy link
Contributor Author

favonia commented Jan 17, 2024

@johnyob I am glad that it's now ready! This issue is no longer blocked.

@favonia favonia removed the blocked label Jan 17, 2024
@favonia
Copy link
Contributor Author

favonia commented Jan 17, 2024

@johnyob I believe the only things to do are

  1. Write a function to convert an asai-diagnostic to this type: https://ocaml.org/p/grace/0.1.0/doc/Grace/Diagnostic/index.html#type-t
  2. Write a new section in the tutorial to show how to use the bridge.

Here are some details:

  1. Do "Help" and "Note" correspond to "Hint" and "Info". This is Rust v.s. LSP and we stole the LSP terminology.
  2. The explanation will be the message.
  3. The main location will be "primary", and its content is the library. The additional remarks with locations will be "secondary". The additional remarks without location will be notes.

I feel the other direction is also straightforward, except that Grace allows multiple primary labels. I wonder if I missed anything...

@favonia
Copy link
Contributor Author

favonia commented Jan 17, 2024

@johnyob Question: an asai diagnostic comes with a stack backtrace. Should them be "secondary" labels, or standalone diagnostics by themselves?

@favonia
Copy link
Contributor Author

favonia commented Aug 12, 2024

@johnyob Hi, I wonder if you think Grace is stable enough now, and whether you have any comment about the questions I asked above? (Sorry for the bothering but I am very interested in implementing the handler.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants