zkat/miette

Add `UnwrapPretty` trait

Opened this issue · 5 comments

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));
            },
        }
    }
}

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 commented

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.

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 commented

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.

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