feat(graphical): render disjoint snippets separately for cleaner output (#324)

Author: LHolten

src/handlers/graphical.rs

@@ -6,7 +6,7 @@ use unicode_width::UnicodeWidthChar;
 use crate::diagnostic_chain::{DiagnosticChain, ErrorKind};
 use crate::handlers::theme::*;
 use crate::protocol::{Diagnostic, Severity};
-use crate::{LabeledSpan, MietteError, ReportHandler, SourceCode, SourceSpan, SpanContents};
+use crate::{LabeledSpan, ReportHandler, SourceCode, SourceSpan, SpanContents};
 
 /**
 A [`ReportHandler`] that displays a given [`Report`](crate::Report) in a
@@ -386,66 +386,58 @@ impl GraphicalReportHandler {
         diagnostic: &(dyn Diagnostic),
         opt_source: Option<&dyn SourceCode>,
     ) -> fmt::Result {
-        if let Some(source) = opt_source {
-            if let Some(labels) = diagnostic.labels() {
-                let mut labels = labels.collect::<Vec<_>>();
-                labels.sort_unstable_by_key(|l| l.inner().offset());
-                if !labels.is_empty() {
-                    let contents = labels
-                        .iter()
-                        .map(|label| {
-                            source.read_span(label.inner(), self.context_lines, self.context_lines)
-                        })
-                        .collect::<Result<Vec<Box<dyn SpanContents<'_>>>, MietteError>>()
-                        .map_err(|_| fmt::Error)?;
-                    let mut contexts = Vec::with_capacity(contents.len());
-                    for (right, right_conts) in labels.iter().cloned().zip(contents.iter()) {
-                        if contexts.is_empty() {
-                            contexts.push((right, right_conts));
-                        } else {
-                            let (left, left_conts) = contexts.last().unwrap().clone();
-                            let left_end = left.offset() + left.len();
-                            let right_end = right.offset() + right.len();
-                            if left_conts.line() + left_conts.line_count() >= right_conts.line() {
-                                // The snippets will overlap, so we create one Big Chunky Boi
-                                let new_span = LabeledSpan::new(
-                                    left.label().map(String::from),
-                                    left.offset(),
-                                    if right_end >= left_end {
-                                        // Right end goes past left end
-                                        right_end - left.offset()
-                                    } else {
-                                        // right is contained inside left
-                                        left.len()
-                                    },
-                                );
-                                if source
-                                    .read_span(
-                                        new_span.inner(),
-                                        self.context_lines,
-                                        self.context_lines,
-                                    )
-                                    .is_ok()
-                                {
-                                    contexts.pop();
-                                    contexts.push((
-                                        // We'll throw this away later
-                                        new_span, left_conts,
-                                    ));
-                                } else {
-                                    contexts.push((right, right_conts));
-                                }
-                            } else {
-                                contexts.push((right, right_conts));
-                            }
-                        }
-                    }
-                    for (ctx, _) in contexts {
-                        self.render_context(f, source, &ctx, &labels[..])?;
-                    }
+        let source = match opt_source {
+            Some(source) => source,
+            None => return Ok(()),
+        };
+        let labels = match diagnostic.labels() {
+            Some(labels) => labels,
+            None => return Ok(()),
+        };
+
+        let mut labels = labels.collect::<Vec<_>>();
+        labels.sort_unstable_by_key(|l| l.inner().offset());
+
+        let mut contexts = Vec::with_capacity(labels.len());
+        for right in labels.iter().cloned() {
+            let right_conts = source
+                .read_span(right.inner(), self.context_lines, self.context_lines)
+                .map_err(|_| fmt::Error)?;
+
+            if contexts.is_empty() {
+                contexts.push((right, right_conts));
+                continue;
+            }
+
+            let (left, left_conts) = contexts.last().unwrap();
+            if left_conts.line() + left_conts.line_count() >= right_conts.line() {
+                // The snippets will overlap, so we create one Big Chunky Boi
+                let left_end = left.offset() + left.len();
+                let right_end = right.offset() + right.len();
+                let new_end = std::cmp::max(left_end, right_end);
+
+                let new_span = LabeledSpan::new(
+                    left.label().map(String::from),
+                    left.offset(),
+                    new_end - left.offset(),
+                );
+                // Check that the two contexts can be combined
+                if let Ok(new_conts) =
+                    source.read_span(new_span.inner(), self.context_lines, self.context_lines)
+                {
+                    contexts.pop();
+                    // We'll throw the contents away later
+                    contexts.push((new_span, new_conts));
+                    continue;
                 }
             }
+
+            contexts.push((right, right_conts));
+        }
+        for (ctx, _) in contexts {
+            self.render_context(f, source, &ctx, &labels[..])?;
         }
+
         Ok(())
     }
 
@@ -458,10 +450,16 @@ impl GraphicalReportHandler {
     ) -> fmt::Result {
         let (contents, lines) = self.get_lines(source, context.inner())?;
 
-        let primary_label = labels
-            .iter()
+        // only consider labels from the context as primary label
+        let ctx_labels = labels.iter().filter(|l| {
+            context.inner().offset() <= l.inner().offset()
+                && l.inner().offset() + l.inner().len()
+                    <= context.inner().offset() + context.inner().len()
+        });
+        let primary_label = ctx_labels
+            .clone()
             .find(|label| label.primary())
-            .or_else(|| labels.first());
+            .or_else(|| ctx_labels.clone().next());
 
         // sorting is your friend
         let labels = labels

tests/graphical.rs

@@ -1757,3 +1757,95 @@ fn single_line_with_wide_char_unaligned_span_empty() -> Result<(), MietteError>
     assert_eq!(expected, out);
     Ok(())
 }
+
+#[test]
+fn triple_adjacent_highlight() -> Result<(), MietteError> {
+    #[derive(Debug, Diagnostic, Error)]
+    #[error("oops!")]
+    #[diagnostic(code(oops::my::bad), help("try doing it better next time?"))]
+    struct MyBad {
+        #[source_code]
+        src: NamedSource,
+        #[label = "this bit here"]
+        highlight1: SourceSpan,
+        #[label = "also this bit"]
+        highlight2: SourceSpan,
+        #[label = "finally we got"]
+        highlight3: SourceSpan,
+    }
+
+    let src = "source\n\n\n  text\n\n\n    here".to_string();
+    let err = MyBad {
+        src: NamedSource::new("bad_file.rs", src),
+        highlight1: (0, 6).into(),
+        highlight2: (11, 4).into(),
+        highlight3: (22, 4).into(),
+    };
+    let out = fmt_report(err.into());
+    println!("Error: {}", out);
+    let expected = "oops::my::bad
+
+  × oops!
+   ╭─[bad_file.rs:1:1]
+ 1 │ source
+   · ───┬──
+   ·    ╰── this bit here
+ 2 │ 
+ 3 │ 
+ 4 │   text
+   ·   ──┬─
+   ·     ╰── also this bit
+ 5 │ 
+ 6 │ 
+ 7 │     here
+   ·     ──┬─
+   ·       ╰── finally we got
+   ╰────
+  help: try doing it better next time?
+";
+    assert_eq!(expected, &out);
+    Ok(())
+}
+
+#[test]
+fn non_adjacent_highlight() -> Result<(), MietteError> {
+    #[derive(Debug, Diagnostic, Error)]
+    #[error("oops!")]
+    #[diagnostic(code(oops::my::bad), help("try doing it better next time?"))]
+    struct MyBad {
+        #[source_code]
+        src: NamedSource,
+        #[label = "this bit here"]
+        highlight1: SourceSpan,
+        #[label = "also this bit"]
+        highlight2: SourceSpan,
+    }
+
+    let src = "source\n\n\n\n  text    here".to_string();
+    let err = MyBad {
+        src: NamedSource::new("bad_file.rs", src),
+        highlight1: (0, 6).into(),
+        highlight2: (12, 4).into(),
+    };
+    let out = fmt_report(err.into());
+    println!("Error: {}", out);
+    let expected = "oops::my::bad
+
+  × oops!
+   ╭─[bad_file.rs:1:1]
+ 1 │ source
+   · ───┬──
+   ·    ╰── this bit here
+ 2 │ 
+   ╰────
+   ╭─[bad_file.rs:5:3]
+ 4 │ 
+ 5 │   text    here
+   ·   ──┬─
+   ·     ╰── also this bit
+   ╰────
+  help: try doing it better next time?
+";
+    assert_eq!(expected, &out);
+    Ok(())
+}