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 UnwrapPretty trait #345

Open
esoterra opened this issue Feb 20, 2024 · 5 comments
Open

Add UnwrapPretty trait #345

esoterra opened this issue Feb 20, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@esoterra
Copy link
Contributor

esoterra commented Feb 20, 2024

Add a trait like the following that people can choose to import that lets them call unwrap_pretty() on results where E is a Diagnostic which is quite useful in things like unit tests.

use miette::{Diagnostic, Report};

pub trait UnwrapPretty {
    type Output;

    fn unwrap_pretty(self) -> Self::Output;
}

impl<T, E> UnwrapPretty for Result<T, E>
where
    E: Diagnostic + Sync + Send + 'static
{
    type Output = T;

    fn unwrap_pretty(self) -> Self::Output {
        match self {
            Ok(output) => output,
            Err(diagnostic) => {
                panic!("{:?}", Report::new(diagnostic));
            },
        }
    }
}
@zkat zkat added the enhancement New feature or request label Feb 20, 2024
@esoterra
Copy link
Contributor Author

esoterra commented Feb 20, 2024

Another possible idea that I'm less sure about is OkPretty as a replacement for calling ok() printing the diagnostic when it fails.

pub trait OkPretty {
    type Output;

    fn ok_pretty(self) -> Option<Self::Output>;
}

impl<T, E> OkPretty for Result<T, E>
where
    E: Diagnostic + Sync + Send + 'static
{
    type Output = T;

    fn ok_pretty(self) -> Option<Self::Output> {
        match self {
            Ok(output) => Some(output),
            Err(diagnostic) => {
                println!("{:?}", Report::new(diagnostic));
                None
            },
        }
    }
}

If I could pick one, it'd definitely be UnwrapPretty but I think both would be nice.

@zkat
Copy link
Owner

zkat commented Feb 21, 2024

I kinda like this idea, but I'm also debating whether it's good to include in miette.

Philosophically speaking, I think it's important to avoid unwraps as much as possible, and miette is designed with the intention of making it easier to avoid them.

But I can see how sometimes you just want to unwrap, such as tests! and how UnwrapPretty might be useful there.

That said: tests are not typically a place where I think fancy-looking diagnostics are really necessary. Fancy diagnostics are more intended for end-users of a tool.

I also definitely wouldn't want OkPretty because we should absolutely not be logging arbitrarily like that. I think doing that is a very application-specific decision to make, not something to encourage.

@esoterra
Copy link
Contributor Author

Fair, OkPretty is pretty out there.

As someone writing a compiler right now, being able to unwrap in my tests and actually see where the error happened is incredibly helpful.

@zkat
Copy link
Owner

zkat commented Feb 22, 2024

Yes, but the cost of just adding this as your own utility, since you find it useful to you, is very very low. It might be best to keep it that way.

@esoterra
Copy link
Contributor Author

esoterra commented Feb 22, 2024

Fair I did in fact implement this for my project.

I just meant that there are real use cases for this. Whether there are enough to justify inclusion 🤷‍♀️

https://github.com/esoterra/claw-lang/blob/main/crates/common/src/diagnostic.rs

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