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

Support miette::IntoDiagnostic for pest::error::Error<R> #582

Closed
cgranade opened this issue Feb 15, 2022 · 4 comments
Closed

Support miette::IntoDiagnostic for pest::error::Error<R> #582

cgranade opened this issue Feb 15, 2022 · 4 comments

Comments

@cgranade
Copy link

It would be nice to be able to more easily use miette to format diagnostics generated by pest errors. Currently, since pest::error::Error::message is private, this cannot easily be done by third-party libraries that depend on both pest and miette without re-implementing much of the message formatting logic in pest. It would be helpful if either message was made public, or if there was a feature-gated impl<R> IntoDiagnostic for pest::error::Error<R> to make it easier to include pest errors in miette diagnostics.

Thank you! ♥

@tomtau
Copy link
Contributor

tomtau commented Jul 14, 2022

@bobbbay would you like to tackle this? You mentioned miette integration in #606 (comment)

@bobbbay
Copy link
Contributor

bobbbay commented Jul 14, 2022

Yes, I already experimented with this locally. A feature-gated implementation is (in my opinion) the best way to go. The issue is moreso refactoring the internal Error representation so that we can implement miette's SpanContents on pest's built-in span type.

I would love to work on this, but I'm currently on vacation and can't always be next to my laptop. If anybody would like to work on this before I get back (end of August), please do.

@gavrilikhin-d
Copy link

So what’s the status on this?

@bobbbay
Copy link
Contributor

bobbbay commented Oct 28, 2022

The PR is closed and I don't plan on working on this anytime soon. From an implementation standpoint, it's doable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants