Repository: zkat/miette
Pull request #418 (2025-01-08 14:57:34)

fix(graphical): prevent leading newline when no link/code
#358 introduced a new behavior: if a diagnostic has no `code`, the rendered report will contain a leading newline.

I'm not sure if this was an intentional change or an unintended side effect, but as a long-time miette user, I wouldn't expect my rendered report to include an extra leading newline - whether I'm outputting to the terminal, displaying in web page, or running snapshot tests. So I opened this PR.

Please let me know if I've misunderstood anything.

zanieb: I believe this is the cause of the regression blocking our upgrade in https://github.com/astral-sh/uv/pull/9568 — many snapshots changed :)
zkat: @zanieb do you think you could run your test suite against this PR and see if it does actually fix the issue? It does seem to be the same thing though but I want to avoid thrash if possible
zanieb: Yep this looks good to me https://github.com/astral-sh/uv/pull/10608
zkat: @zanieb thanks for taking a look!

Patch:
diff --git a/src/handlers/graphical.rs b/src/handlers/graphical.rs
index fcbec05..3e9c790 100644
--- a/src/handlers/graphical.rs
+++ b/src/handlers/graphical.rs
@@ -237,7 +237,7 @@ impl GraphicalReportHandler {
         parent_src: Option<&dyn SourceCode>,
     ) -> fmt::Result {
         let src = diagnostic.source_code().or(parent_src);
-        self.render_header(f, diagnostic)?;
+        self.render_header(f, diagnostic, false)?;
         self.render_causes(f, diagnostic, src)?;
         self.render_snippets(f, diagnostic, src)?;
         self.render_footer(f, diagnostic)?;
@@ -261,13 +261,19 @@ impl GraphicalReportHandler {
         Ok(())
     }
 
-    fn render_header(&self, f: &mut impl fmt::Write, diagnostic: &(dyn Diagnostic)) -> fmt::Result {
+    fn render_header(
+        &self,
+        f: &mut impl fmt::Write,
+        diagnostic: &(dyn Diagnostic),
+        is_nested: bool,
+    ) -> fmt::Result {
         let severity_style = match diagnostic.severity() {
             Some(Severity::Error) | None => self.theme.styles.error,
             Some(Severity::Warning) => self.theme.styles.warning,
             Some(Severity::Advice) => self.theme.styles.advice,
         };
         let mut header = String::new();
+        let mut need_newline = is_nested;
         if self.links == LinkStyle::Link && diagnostic.url().is_some() {
             let url = diagnostic.url().unwrap(); // safe
             let code = if let Some(code) = diagnostic.code() {
@@ -284,6 +290,7 @@ impl GraphicalReportHandler {
             );
             write!(header, "{}", link)?;
             writeln!(f, "{}", header)?;
+            need_newline = true;
         } else if let Some(code) = diagnostic.code() {
             write!(header, "{}", code.style(severity_style),)?;
             if self.links == LinkStyle::Text && diagnostic.url().is_some() {
@@ -291,8 +298,11 @@ impl GraphicalReportHandler {
                 write!(header, " ({})", url.style(self.theme.styles.link))?;
             }
             writeln!(f, "{}", header)?;
+            need_newline = true;
+        }
+        if need_newline {
+            writeln!(f)?;
         }
-        writeln!(f)?;
         Ok(())
     }
 
@@ -493,7 +503,7 @@ impl GraphicalReportHandler {
                         Some(Severity::Warning) => write!(f, "Warning: ")?,
                         Some(Severity::Advice) => write!(f, "Advice: ")?,
                     };
-                    inner_renderer.render_header(f, rel)?;
+                    inner_renderer.render_header(f, rel, true)?;
                     let src = rel.source_code().or(parent_src);
                     inner_renderer.render_causes(f, rel, src)?;
                     inner_renderer.render_snippets(f, rel, src)?;


Test patch:
diff --git a/tests/graphical.rs b/tests/graphical.rs
index 3176812..b80e16d 100644
--- a/tests/graphical.rs
+++ b/tests/graphical.rs
@@ -59,7 +59,7 @@ fn word_wrap_options() -> Result<(), MietteError> {
     let out =
         fmt_report_with_settings(Report::msg("abcdefghijklmnopqrstuvwxyz"), |handler| handler);
 
-    let expected = "\n  × abcdefghijklmnopqrstuvwxyz\n".to_string();
+    let expected = "  × abcdefghijklmnopqrstuvwxyz\n".to_string();
     assert_eq!(expected, out);
 
     // A long word can break with a smaller width
@@ -75,14 +75,14 @@ fn word_wrap_options() -> Result<(), MietteError> {
   │ uvwx
   │ yz
 "#
-    .to_string();
+    .trim_start_matches('\n');
     assert_eq!(expected, out);
 
     // Unless, word breaking is disabled
     let out = fmt_report_with_settings(Report::msg("abcdefghijklmnopqrstuvwxyz"), |handler| {
         handler.with_width(10).with_break_words(false)
     });
-    let expected = "\n  × abcdefghijklmnopqrstuvwxyz\n".to_string();
+    let expected = "  × abcdefghijklmnopqrstuvwxyz\n".to_string();
     assert_eq!(expected, out);
 
     // Breaks should start at the boundary of each word if possible
@@ -104,7 +104,7 @@ fn word_wrap_options() -> Result<(), MietteError> {
   │ 5678
   │ 90
 "#
-    .to_string();
+    .trim_start_matches('\n');
     assert_eq!(expected, out);
 
     // But long words should not break if word breaking is disabled
@@ -121,7 +121,7 @@ fn word_wrap_options() -> Result<(), MietteError> {
   │ 1234567
   │ 1234567890
 "#
-    .to_string();
+    .trim_start_matches('\n');
     assert_eq!(expected, out);
 
     // Unless, of course, there are hyphens
@@ -149,7 +149,7 @@ fn word_wrap_options() -> Result<(), MietteError> {
   │ f-g-
   │ h
 "#
-    .to_string();
+    .trim_start_matches('\n');
     assert_eq!(expected, out);
 
     // Which requires an additional opt-out
@@ -171,7 +171,7 @@ fn word_wrap_options() -> Result<(), MietteError> {
   │ a-b-c-d-e-f-g
   │ a-b-c-d-e-f-g-h
 "#
-    .to_string();
+    .trim_start_matches('\n');
     assert_eq!(expected, out);
 
     // Or if there are _other_ unicode word boundaries
@@ -199,7 +199,7 @@ fn word_wrap_options() -> Result<(), MietteError> {
   │ f/g/
   │ h
 "#
-    .to_string();
+    .trim_start_matches('\n');
     assert_eq!(expected, out);
 
     // Such things require you to opt-in to only breaking on ASCII whitespace
@@ -221,7 +221,7 @@ fn word_wrap_options() -> Result<(), MietteError> {
   │ a/b/c/d/e/f/g
   │ a/b/c/d/e/f/g/h
 "#
-    .to_string();
+    .trim_start_matches('\n');
     assert_eq!(expected, out);
 
     Ok(())
@@ -245,7 +245,7 @@ fn wrap_option() -> Result<(), MietteError> {
   │ pqr stu
   │ vwx yz
 "#
-    .to_string();
+    .trim_start_matches('\n');
     assert_eq!(expected, out);
 
     // Unless, wrapping is disabled
@@ -254,7 +254,7 @@ fn wrap_option() -> Result<(), MietteError> {
         |handler| handler.with_width(15).with_wrap_lines(false),
     );
     let expected =
-        "\n  × abc def ghi jkl mno pqr stu vwx yz abc def ghi jkl mno pqr stu vwx yz\n".to_string();
+        "  × abc def ghi jkl mno pqr stu vwx yz abc def ghi jkl mno pqr stu vwx yz\n".to_string();
     assert_eq!(expected, out);
 
     // Then, user-defined new lines should be preserved wrapping is disabled
@@ -267,7 +267,7 @@ fn wrap_option() -> Result<(), MietteError> {
   │ abc def ghi jkl mno pqr stu vwx yz
   │ abc def ghi jkl mno pqr stu vwx yz
 "#
-    .to_string();
+    .trim_start_matches('\n');
     assert_eq!(expected, out);
 
     Ok(())
@@ -485,8 +485,7 @@ if true {
    · ╰──── big
    ╰────
 "#
-    .to_string();
-
+    .trim_start_matches('\n');
     assert_eq!(expected, out);
 }
 
@@ -517,8 +516,7 @@ fn single_line_highlight_span_full_line() {
    ·   ╰── This bit here
    ╰────
 "#
-    .to_string();
-
+    .trim_start_matches('\n');
     assert_eq!(expected, out);
 }
 
@@ -1781,8 +1779,7 @@ fn zero_length_eol_span() {
    · ╰── This bit here
    ╰────
 "#
-    .to_string();
-
+    .trim_start_matches('\n');
     assert_eq!(expected, out);
 }
 
@@ -1817,8 +1814,7 @@ fn primary_label() {
    ·    ╰── nope
    ╰────
 "#
-    .to_string();
-
+    .trim_start_matches('\n');
     assert_eq!(expected, out);
 }
 
@@ -1968,7 +1964,8 @@ fn syntax_highlighter() {
    ·                  ╰── this is a label
  3 │ }
    ╰────
-"#;
+"#
+    .trim_start_matches('\n');
     assert!(out.contains("\u{1b}[38;2;180;142;173m"));
     assert_eq!(expected, strip_ansi_escapes::strip_str(out))
 }
@@ -2028,6 +2025,7 @@ fn syntax_highlighter_on_real_file() {
         l2 = line,
         l3 = line + 1
     );
+    let expected = expected.trim_start_matches('\n');
     assert!(out.contains("\u{1b}[38;2;180;142;173m"));
     assert_eq!(expected, strip_ansi_escapes::strip_str(out));
 }
diff --git a/tests/test_diagnostic_source_macro.rs b/tests/test_diagnostic_source_macro.rs
index 71429ef..e038830 100644
--- a/tests/test_diagnostic_source_macro.rs
+++ b/tests/test_diagnostic_source_macro.rs
@@ -118,7 +118,8 @@ fn test_diagnostic_source_pass_extra_info() {
 
   this is a footer
 "#
-    .to_string();
+    .trim_start_matches('\n');
+
     assert_eq!(expected, out);
 }
 
@@ -149,7 +150,8 @@ fn test_diagnostic_source_is_output() {
          ╰────
         help: That's where the error is!
       
-"#;
+"#
+    .trim_start_matches('\n');
 
     assert_eq!(expected, out);
 }
@@ -208,7 +210,8 @@ fn test_nested_diagnostic_source_is_output() {
    ╰────
 
   Yooo, a footer
-"#;
+"#
+    .trim_start_matches('\n');
 
     assert_eq!(expected, out);
 }
@@ -295,7 +298,8 @@ fn test_nested_cause_chains_for_related_errors_are_output() {
    ╰────
 
   Yooo, a footer
-"#;
+"#
+    .trim_start_matches('\n');
 
     assert_eq!(expected, out);
 }
@@ -354,7 +358,9 @@ fn test_display_related_errors_as_nested() {
          ·      ╰── here
          ╰────
         help: Get a grip...
-"#;
+"#
+    .trim_start_matches('\n');
+
     assert_eq!(expected, out);
 }
 
@@ -403,7 +409,8 @@ fn source_is_inherited_to_causes() {
       
 
   Yooo, a footer
-"#;
+"#
+    .trim_start_matches('\n');
 
     assert_eq!(expected, out);
 }