Repository: zkat/miette (rust)

Issue:
Rendering bug on small spans in large spans
While using miette, I found a small but significant rendering bug in rendering arrows. The picture below shows what it looks like:
![image](https://github.com/zkat/miette/assets/22172241/c5c9ef87-80f8-4537-b74c-dcd5e80c3d26)

Of course, I can't just give you just that image to go on, so I made a minimal example:

```rust
#[derive(Error, Debug, Diagnostic)]
#[error("oops!")]
#[diagnostic(severity(Error))]
struct MyBad {
    #[source_code]
    src: NamedSource,
    #[label("big")]
    big: SourceSpan,
    #[label("small")]
    small: SourceSpan,
}
let err = MyBad {
    src: NamedSource::new("issue", "\
if true {
    a
} else {
    b
}"),
    big: (0, 32).into(),
    small: (14, 1).into(),
};
let out = fmt_report(err.into());
println!("Error: {}", out);

```
This prints:
```rust
r#"  × oops!
   ╭─[issue:1:1]
 1 │ ╭─▶ if true {
 2 │ │╭▶     a
   · ││    ┬
   · ││    ╰── small
 3 │ │   } else {
 4 │ │       b
 5 │ ├─▶ }
   · ╰──── big
   ╰────
"#
```

while I expected it to print:

```rust
r#"  × oops!
   ╭─[issue:1:1]
 1 │ ╭─▶ if true {
 2 │ │       a
   · │       ┬
   · │       ╰── small
 3 │ │   } else {
 4 │ │       b
 5 │ ├─▶ }
   · ╰──── big
   ╰────
"#

```

As you can see, this looks a bit like a test, which is exactly what I wrote and contributed as a pull request so the bug is reproducible. See #316.


Pull request description:
rendering bug on small spans in large spans
See #317. This contributes a test for #317  and then fixes the bug :)

Closes #317 

The bad screenshot from #317 

![image](https://github.com/zkat/miette/assets/22172241/85aa0da8-4bf9-4305-8bd5-5a2bed65ec16)

now becomes this with the proposed changes:

![image](https://github.com/zkat/miette/assets/22172241/2d9984bd-73da-4299-8a20-9b8f515730d7)

Note that there were actually two subtle bugs. One was the double arrow in the gutter problem, which also occurred when rendering without ansi escape codes. However, this line
https://github.com/zkat/miette/blob/a8b4ae012aa0cf03b53a18f013c2b3f76c5040e7/src/handlers/graphical.rs#L611

Also did not work when the gutter contains ansi escape codes. It extends to the `width` variable, but when the string contains ansi codes it does not actually add any spaces since it thinks the escape codes have width as well. They of course do not, so the output doesn't look right. Unfortunately this is quite hard to make tests for.

Patch:
diff --git a/src/handlers/graphical.rs b/src/handlers/graphical.rs
index baadfb3..3193472 100644
--- a/src/handlers/graphical.rs
+++ b/src/handlers/graphical.rs
@@ -20,7 +20,7 @@ This printer can be customized by using [`new_themed()`](GraphicalReportHandler:
 
 See [`set_hook()`](crate::set_hook) for more details on customizing your global
 printer.
- */
+*/
 #[derive(Debug, Clone)]
 pub struct GraphicalReportHandler {
     pub(crate) links: LinkStyle,
@@ -468,7 +468,7 @@ impl GraphicalReportHandler {
         for line in &lines {
             let mut num_highlights = 0;
             for hl in &labels {
-                if !line.span_line_only(hl) && line.span_applies(hl) {
+                if !line.span_line_only(hl) && line.span_applies_gutter(hl) {
                     num_highlights += 1;
                 }
             }
@@ -674,7 +674,7 @@ impl GraphicalReportHandler {
         }
         let chars = &self.theme.characters;
         let mut gutter = String::new();
-        let applicable = highlights.iter().filter(|hl| line.span_applies(hl));
+        let applicable = highlights.iter().filter(|hl| line.span_applies_gutter(hl));
         let mut arrow = false;
         for (i, hl) in applicable.enumerate() {
             if line.span_starts(hl) {
@@ -735,18 +735,35 @@ impl GraphicalReportHandler {
         if max_gutter == 0 {
             return Ok(());
         }
+
+        // keeps track of how many colums wide the gutter is
+        // important for ansi since simply measuring the size of the final string
+        // gives the wrong result when the string contains ansi codes.
+        let mut gutter_cols = 0;
+
         let chars = &self.theme.characters;
         let mut gutter = String::new();
-        let applicable = highlights.iter().filter(|hl| line.span_applies(hl));
+        let applicable = highlights.iter().filter(|hl| line.span_applies_gutter(hl));
         for (i, hl) in applicable.enumerate() {
             if !line.span_line_only(hl) && line.span_ends(hl) {
                 if render_mode == LabelRenderMode::MultiLineRest {
                     // this is to make multiline labels work. We want to make the right amount
                     // of horizontal space for them, but not actually draw the lines
-                    for _ in 0..max_gutter.saturating_sub(i) + 2 {
+                    let horizontal_space = max_gutter.saturating_sub(i) + 2;
+                    for _ in 0..horizontal_space {
                         gutter.push(' ');
                     }
+                    // account for one more horizontal space, since in multiline mode
+                    // we also add in the vertical line before the label like this:
+                    // 2 │ ╭─▶   text
+                    // 3 │ ├─▶     here
+                    //   · ╰──┤ these two lines
+                    //   ·    │ are the problem
+                    //        ^this
+                    gutter_cols += horizontal_space + 1;
                 } else {
+                    let num_repeat = max_gutter.saturating_sub(i) + 2;
+
                     gutter.push_str(&chars.lbot.style(hl.style).to_string());
 
                     gutter.push_str(
@@ -754,25 +771,42 @@ impl GraphicalReportHandler {
                             .hbar
                             .to_string()
                             .repeat(
-                                max_gutter.saturating_sub(i)
+                                num_repeat
                                     // if we are rendering a multiline label, then leave a bit of space for the
                                     // rcross character
-                                    + if render_mode == LabelRenderMode::MultiLineFirst {
+                                    - if render_mode == LabelRenderMode::MultiLineFirst {
                                         1
                                     } else {
-                                        2
+                                        0
                                     },
                             )
                             .style(hl.style)
                             .to_string(),
                     );
+
+                    // we count 1 for the lbot char, and then a few more, the same number
+                    // as we just repeated for. For each repeat we only add 1, even though
+                    // due to ansi escape codes the number of bytes in the string could grow
+                    // a lot each time.
+                    gutter_cols += num_repeat + 1;
                 }
                 break;
             } else {
                 gutter.push_str(&chars.vbar.style(hl.style).to_string());
+
+                // we may push many bytes for the ansi escape codes style adds,
+                // but we still only add a single character-width to the string in a terminal
+                gutter_cols += 1;
             }
         }
-        write!(f, "{:width$}", gutter, width = max_gutter + 1)?;
+
+        // now calculate how many spaces to add based on how many columns we just created.
+        // it's the max width of the gutter, minus how many character-widths we just generated
+        // capped at 0 (though this should never go below in reality), and then we add 3 to
+        // account for arrowheads when a gutter line ends
+        let num_spaces = (max_gutter + 3).saturating_sub(gutter_cols);
+        // we then write the gutter and as many spaces as we need
+        write!(f, "{}{:width$}", gutter, "", width = num_spaces)?;
         Ok(())
     }
 
@@ -1133,9 +1167,12 @@ impl Line {
         span.offset() >= self.offset && span.offset() + span.len() <= self.offset + self.length
     }
 
+    /// Returns whether `span` should be visible on this line, either in the gutter or under the
+    /// text on this line
     fn span_applies(&self, span: &FancySpan) -> bool {
         let spanlen = if span.len() == 0 { 1 } else { span.len() };
         // Span starts in this line
+
         (span.offset() >= self.offset && span.offset() < self.offset + self.length)
             // Span passes through this line
             || (span.offset() < self.offset && span.offset() + spanlen > self.offset + self.length) //todo
@@ -1143,6 +1180,20 @@ impl Line {
             || (span.offset() + spanlen > self.offset && span.offset() + spanlen <= self.offset + self.length)
     }
 
+    /// Returns whether `span` should be visible on this line in the gutter (so this excludes spans
+    /// that are only visible on this line and do not span multiple lines)
+    fn span_applies_gutter(&self, span: &FancySpan) -> bool {
+        let spanlen = if span.len() == 0 { 1 } else { span.len() };
+        // Span starts in this line
+        self.span_applies(span)
+            && !(
+                // as long as it doesn't start *and* end on this line
+                (span.offset() >= self.offset && span.offset() < self.offset + self.length)
+                    && (span.offset() + spanlen > self.offset
+                        && span.offset() + spanlen <= self.offset + self.length)
+            )
+    }
+
     // A 'flyby' is a multi-line span that technically covers this line, but
     // does not begin or end within the line itself. This method is used to
     // calculate gutters.


Test patch:
diff --git a/tests/graphical.rs b/tests/graphical.rs
index f59f849..aabf167 100644
--- a/tests/graphical.rs
+++ b/tests/graphical.rs
@@ -283,9 +283,9 @@ if true {
     let expected = r#"  × oops!
    ╭─[issue:1:1]
  1 │ ╭─▶ if true {
- 2 │ │╭▶     a
-   · ││    ┬
-   · ││    ╰── small
+ 2 │ │       a
+   · │       ┬
+   · │       ╰── small
  3 │ │   } else {
  4 │ │       b
  5 │ ├─▶ }