Skip to content

Conversation

@TheLostLambda
Copy link
Contributor

Partially addresses #366

Brings things in line with best practices as described in "Rust for Rustaceans" when it comes to "Ergonomic Trait Implementations" in Chapter 3. Practically means that people can pass more types to any functions taking either dyn Diagnostic or impl Diagnostic.

BREAKING CHANGE: Added blanket impls may overlap with manual user impls. If these impls are just hacks to achieve the same as the blanket ones do, then they can simply be removed. If the impls for T and &T differ semantically, then the user would need to employ the newtype pattern to avoid overlap.

Brings things in line with best practices as described in "Rust for
Rustaceans" when it comes to "Ergonomic Trait Implementations" in Chapter
3. Practically means that people can pass more types to any functions
taking either `dyn Diagnostic` or `impl Diagnostic`.

BREAKING CHANGE: Added blanket impls may overlap with manual user impls.
If these impls are just hacks to achieve the same as the blanket ones do,
then they can simply be removed. If the impls for `T` and `&T` differ
semantically, then the user would need to employ the newtype pattern to
avoid overlap.
@TheLostLambda
Copy link
Contributor Author

TheLostLambda commented Apr 24, 2024

Uh oh, this make have broken more than I thought... Let's hold off for the moment 😅

EDIT: I think it's fine... Turns out typoing unwrap() for unwrap_err() really does make a difference...

@TheLostLambda TheLostLambda marked this pull request as draft April 24, 2024 00:12
@TheLostLambda TheLostLambda marked this pull request as ready for review April 24, 2024 00:17
@TheLostLambda
Copy link
Contributor Author

Confirmed to be working well for me!

This let me go from:

use miette::Diagnostic;
use std::fmt::Debug;

pub trait UnwrapDiagnostic<E> {
    fn unwrap_diagnostic(self) -> E;
}

impl<T: Debug, E: Diagnostic> UnwrapDiagnostic<E> for Result<T, E> {
    fn unwrap_diagnostic(self) -> E {
        self.unwrap_err()
    }
}

impl<T: Debug, E: Diagnostic> UnwrapDiagnostic<E> for Result<T, Box<E>> {
    fn unwrap_diagnostic(self) -> E {
        *self.unwrap_err()
    }
}

macro_rules! assert_miette_snapshot {
    ($diag:expr) => {{
        use insta::{with_settings, assert_snapshot};
        use miette::{GraphicalReportHandler, GraphicalTheme};
        use crate::testing_tools::UnwrapDiagnostic;

        let mut out = String::new();
        GraphicalReportHandler::new_themed(GraphicalTheme::unicode_nocolor())
            .with_width(80)
            .render_report(&mut out, &$diag.unwrap_diagnostic())
            .unwrap();
        with_settings!({
            description => stringify!($diag)
        }, {
            assert_snapshot!(out);
        });
    }};
}

pub(crate) use assert_miette_snapshot;

To just:

macro_rules! assert_miette_snapshot {
    ($diag:expr) => {{
        use insta::{with_settings, assert_snapshot};
        use miette::{GraphicalReportHandler, GraphicalTheme};

        let mut out = String::new();
        GraphicalReportHandler::new_themed(GraphicalTheme::unicode_nocolor())
            .with_width(80)
            .render_report(&mut out, &$diag.unwrap_err())
            .unwrap();
        with_settings!({
            description => stringify!($diag)
        }, {
            assert_snapshot!(out);
        });
    }};
}

pub(crate) use assert_miette_snapshot;

@azzamsa
Copy link

azzamsa commented Jun 10, 2025

I'm encountering a similar issue to #431. What's the recommended workaround while waiting for this PR to be merged?

azzamsa added a commit to azzamsa/gelatyx that referenced this pull request Jun 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking A semver-major breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants