Repository: zkat/miette
Pull request #330 (2024-01-27 20:47:23)

fix(graphical): render cause chains for inner errors
Hi @zkat!

The default `GraphicalReportHandler` disables the printing of cause chains for any inner errors (errors `related()` to a source diagnostic) when it disables nested footer printing. This results in lost cause chain information when printing with the default report handler.

Before this PR:
![image](https://github.com/zkat/miette/assets/6251883/9cea3276-e371-4723-84f3-d6c825bda953)

After this PR:
![image](https://github.com/zkat/miette/assets/6251883/0f0e9fd6-7de5-4c04-8a99-296766ed7bea)


This PR simply re-enables that cause-chain printing by removing the line where it was disabled. It looks like that line was added as part of the initial feature implementation in #170, but the current behavior of not printing the cause chains of inner errors (I found) surprising and it led me to scour my codebase trying to find where the source error was being lost.

If, however, retaining the current default behavior is preferred, I can implement a couple of methods on `GraphicalReportHandler` like `.with_nested_cause_chains()` and `.without_nested_cause_chains()` — just let me know!

Also more than happy to write some tests after I know if you're happy with changing this default behavior or if you'd prefer a configuration option was added!

Hope you are well!
Brooks

TheLostLambda: Whoops! Those tests didn't fail when I ran them locally, but I'll give then another look today! I'll get them passing!

TheLostLambda: Turns out the line of code I deleted did have a reason to exist — go figure! I've added that back and turned back on the cause chains specifically for nested related errors. I've also added a test to capture the new behavior!

All tests are passing for me now (with `cargo test --all --features fancy` like is used in the CI)!

Patch:
diff --git a/src/handlers/graphical.rs b/src/handlers/graphical.rs
index 021ba34..7812925 100644
--- a/src/handlers/graphical.rs
+++ b/src/handlers/graphical.rs
@@ -317,9 +317,10 @@ impl GraphicalReportHandler {
                     ErrorKind::Diagnostic(diag) => {
                         let mut inner = String::new();
 
-                        // Don't print footer for inner errors
                         let mut inner_renderer = self.clone();
+                        // Don't print footer for inner errors
                         inner_renderer.footer = None;
+                        // Cause chains are already flattened, so don't double-print the nested error
                         inner_renderer.with_cause_chain = false;
                         inner_renderer.render_report(&mut inner, diag)?;
 
@@ -362,6 +363,9 @@ impl GraphicalReportHandler {
         parent_src: Option<&dyn SourceCode>,
     ) -> fmt::Result {
         if let Some(related) = diagnostic.related() {
+            let mut inner_renderer = self.clone();
+            // Re-enable the printing of nested cause chains for related errors
+            inner_renderer.with_cause_chain = true;
             writeln!(f)?;
             for rel in related {
                 match rel.severity() {
@@ -369,12 +373,12 @@ impl GraphicalReportHandler {
                     Some(Severity::Warning) => write!(f, "Warning: ")?,
                     Some(Severity::Advice) => write!(f, "Advice: ")?,
                 };
-                self.render_header(f, rel)?;
-                self.render_causes(f, rel)?;
+                inner_renderer.render_header(f, rel)?;
+                inner_renderer.render_causes(f, rel)?;
                 let src = rel.source_code().or(parent_src);
-                self.render_snippets(f, rel, src)?;
-                self.render_footer(f, rel)?;
-                self.render_related(f, rel, src)?;
+                inner_renderer.render_snippets(f, rel, src)?;
+                inner_renderer.render_footer(f, rel)?;
+                inner_renderer.render_related(f, rel, src)?;
             }
         }
         Ok(())


Test patch:
diff --git a/tests/test_diagnostic_source_macro.rs b/tests/test_diagnostic_source_macro.rs
index 334ca34..e5305ac 100644
--- a/tests/test_diagnostic_source_macro.rs
+++ b/tests/test_diagnostic_source_macro.rs
@@ -194,3 +194,85 @@ fn test_nested_diagnostic_source_is_output() {
 
     assert_eq!(expected, out);
 }
+
+#[derive(Debug, miette::Diagnostic, thiserror::Error)]
+#[error("A multi-error happened")]
+struct MultiError {
+    #[related]
+    related_errs: Vec<Box<dyn Diagnostic>>,
+}
+
+#[cfg(feature = "fancy-no-backtrace")]
+#[test]
+fn test_nested_cause_chains_for_related_errors_are_output() {
+    let inner_error = TestStructError {
+        asdf_inner_foo: SourceError {
+            code: String::from("This is another error"),
+            help: String::from("You should fix this"),
+            label: (3, 4),
+        },
+    };
+    let first_error = NestedError {
+        code: String::from("right here"),
+        label: (6, 4),
+        the_other_err: Box::new(inner_error),
+    };
+    let second_error = SourceError {
+        code: String::from("You're actually a mess"),
+        help: String::from("Get a grip..."),
+        label: (3, 4),
+    };
+    let multi_error = MultiError {
+        related_errs: vec![Box::new(first_error), Box::new(second_error)],
+    };
+    let diag = NestedError {
+        code: String::from("the outside world"),
+        label: (6, 4),
+        the_other_err: Box::new(multi_error),
+    };
+    let mut out = String::new();
+    miette::GraphicalReportHandler::new_themed(miette::GraphicalTheme::unicode_nocolor())
+        .with_width(80)
+        .with_footer("Yooo, a footer".to_string())
+        .render_report(&mut out, &diag)
+        .unwrap();
+    println!("{}", out);
+
+    let expected = r#"  × A nested error happened
+  ╰─▶   × A multi-error happened
+      
+      Error:   × A nested error happened
+        ├─▶   × TestError
+        │
+        ╰─▶   × A complex error happened
+               ╭────
+             1 │ This is another error
+               ·    ──┬─
+               ·      ╰── here
+               ╰────
+              help: You should fix this
+      
+         ╭────
+       1 │ right here
+         ·       ──┬─
+         ·         ╰── here
+         ╰────
+      Error:   × A complex error happened
+         ╭────
+       1 │ You're actually a mess
+         ·    ──┬─
+         ·      ╰── here
+         ╰────
+        help: Get a grip...
+      
+   ╭────
+ 1 │ the outside world
+   ·       ──┬─
+   ·         ╰── here
+   ╰────
+
+  Yooo, a footer
+"#;
+
+    assert_eq!(expected, out);
+}