Repository: zkat/miette
Pull request #324 (2023-12-13 12:21:02)

Clean up render snippets
Hey, here are some local changes I made that might be useful.

In the first commit I tried to make the `render_snippets` function more readable and fixed what I think was a bug.
(Looks like it would not merge more than two contexts if the last context didn't overlap the first)

In the second commit I restricted finding the primary label to those in the current context.
This makes it so that the header above each snippet points to code in the snippet.
(It is especially useful for `SourceCode` implementations that wrap multiple files)

Example showing different headers for different snippets:
![image](https://github.com/zkat/miette/assets/24637999/20017e9b-0970-40f4-8e0e-3342af743932)



Patch:
diff --git a/src/handler.rs b/src/handler.rs
index e32f3ef..38c9fc8 100644
--- a/src/handler.rs
+++ b/src/handler.rs
@@ -56,6 +56,7 @@ pub struct MietteHandlerOpts {
     pub(crate) tab_width: Option<usize>,
     pub(crate) with_cause_chain: Option<bool>,
     pub(crate) break_words: Option<bool>,
+    pub(crate) wrap_lines: Option<bool>,
     pub(crate) word_separator: Option<textwrap::WordSeparator>,
     pub(crate) word_splitter: Option<textwrap::WordSplitter>,
 }
@@ -89,6 +90,16 @@ impl MietteHandlerOpts {
         self
     }
 
+    /// If true, long lines can be wrapped.
+    ///
+    /// If false, long lines will not be broken when they exceed the width.
+    ///
+    /// Defaults to true.
+    pub fn wrap_lines(mut self, wrap_lines: bool) -> Self {
+        self.wrap_lines = Some(wrap_lines);
+        self
+    }
+
     /// If true, long words can be broken when wrapping.
     ///
     /// If false, long words will not be broken when they exceed the width.
@@ -98,7 +109,6 @@ impl MietteHandlerOpts {
         self.break_words = Some(break_words);
         self
     }
-
     /// Sets the `textwrap::WordSeparator` to use when determining wrap points.
     pub fn word_separator(mut self, word_separator: textwrap::WordSeparator) -> Self {
         self.word_separator = Some(word_separator);
@@ -260,6 +270,9 @@ impl MietteHandlerOpts {
             if let Some(b) = self.break_words {
                 handler = handler.with_break_words(b)
             }
+            if let Some(b) = self.wrap_lines {
+                handler = handler.with_wrap_lines(b)
+            }
             if let Some(s) = self.word_separator {
                 handler = handler.with_word_separator(s)
             }
diff --git a/src/handlers/graphical.rs b/src/handlers/graphical.rs
index 3193472..021ba34 100644
--- a/src/handlers/graphical.rs
+++ b/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
@@ -30,6 +30,7 @@ pub struct GraphicalReportHandler {
     pub(crate) context_lines: usize,
     pub(crate) tab_width: usize,
     pub(crate) with_cause_chain: bool,
+    pub(crate) wrap_lines: bool,
     pub(crate) break_words: bool,
     pub(crate) word_separator: Option<textwrap::WordSeparator>,
     pub(crate) word_splitter: Option<textwrap::WordSplitter>,
@@ -54,6 +55,7 @@ impl GraphicalReportHandler {
             context_lines: 1,
             tab_width: 4,
             with_cause_chain: true,
+            wrap_lines: true,
             break_words: true,
             word_separator: None,
             word_splitter: None,
@@ -69,6 +71,7 @@ impl GraphicalReportHandler {
             footer: None,
             context_lines: 1,
             tab_width: 4,
+            wrap_lines: true,
             with_cause_chain: true,
             break_words: true,
             word_separator: None,
@@ -131,6 +134,12 @@ impl GraphicalReportHandler {
         self
     }
 
+    /// Enables or disables wrapping of lines to fit the width.
+    pub fn with_wrap_lines(mut self, wrap_lines: bool) -> Self {
+        self.wrap_lines = wrap_lines;
+        self
+    }
+
     /// Enables or disables breaking of words during wrapping.
     pub fn with_break_words(mut self, break_words: bool) -> Self {
         self.break_words = break_words;
@@ -197,7 +206,7 @@ impl GraphicalReportHandler {
                 opts = opts.word_splitter(word_splitter);
             }
 
-            writeln!(f, "{}", textwrap::fill(footer, opts))?;
+            writeln!(f, "{}", self.wrap(footer, opts))?;
         }
         Ok(())
     }
@@ -258,7 +267,7 @@ impl GraphicalReportHandler {
             opts = opts.word_splitter(word_splitter);
         }
 
-        writeln!(f, "{}", textwrap::fill(&diagnostic.to_string(), opts))?;
+        writeln!(f, "{}", self.wrap(&diagnostic.to_string(), opts))?;
 
         if !self.with_cause_chain {
             return Ok(());
@@ -314,10 +323,10 @@ impl GraphicalReportHandler {
                         inner_renderer.with_cause_chain = false;
                         inner_renderer.render_report(&mut inner, diag)?;
 
-                        writeln!(f, "{}", textwrap::fill(&inner, opts))?;
+                        writeln!(f, "{}", self.wrap(&inner, opts))?;
                     }
                     ErrorKind::StdError(err) => {
-                        writeln!(f, "{}", textwrap::fill(&err.to_string(), opts))?;
+                        writeln!(f, "{}", self.wrap(&err.to_string(), opts))?;
                     }
                 }
             }
@@ -341,7 +350,7 @@ impl GraphicalReportHandler {
                 opts = opts.word_splitter(word_splitter);
             }
 
-            writeln!(f, "{}", textwrap::fill(&help.to_string(), opts))?;
+            writeln!(f, "{}", self.wrap(&help.to_string(), opts))?;
         }
         Ok(())
     }
@@ -377,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(())
     }
 
@@ -449,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
@@ -810,6 +817,41 @@ impl GraphicalReportHandler {
         Ok(())
     }
 
+    fn wrap(&self, text: &str, opts: textwrap::Options<'_>) -> String {
+        if self.wrap_lines {
+            textwrap::fill(text, opts)
+        } else {
+            // Format without wrapping, but retain the indentation options
+            // Implementation based on `textwrap::indent`
+            let mut result = String::with_capacity(2 * text.len());
+            let trimmed_indent = opts.subsequent_indent.trim_end();
+            for (idx, line) in text.split_terminator('\n').enumerate() {
+                if idx > 0 {
+                    result.push('\n');
+                }
+                if idx == 0 {
+                    if line.trim().is_empty() {
+                        result.push_str(opts.initial_indent.trim_end());
+                    } else {
+                        result.push_str(opts.initial_indent);
+                    }
+                } else {
+                    if line.trim().is_empty() {
+                        result.push_str(trimmed_indent);
+                    } else {
+                        result.push_str(opts.subsequent_indent);
+                    }
+                }
+                result.push_str(line);
+            }
+            if text.ends_with('\n') {
+                // split_terminator will have eaten the final '\n'.
+                result.push('\n');
+            }
+            result
+        }
+    }
+
     fn write_linum(&self, f: &mut impl fmt::Write, width: usize, linum: usize) -> fmt::Result {
         write!(
             f,


Test patch:
diff --git a/tests/graphical.rs b/tests/graphical.rs
index aabf167..4e8b737 100644
--- a/tests/graphical.rs
+++ b/tests/graphical.rs
@@ -218,6 +218,50 @@ fn word_wrap_options() -> Result<(), MietteError> {
     Ok(())
 }
 
+#[test]
+fn wrap_option() -> Result<(), MietteError> {
+    // A line should break on the width
+    let out = fmt_report_with_settings(
+        Report::msg("abc def ghi jkl mno pqr stu vwx yz abc def ghi jkl mno pqr stu vwx yz"),
+        |handler| handler.with_width(15),
+    );
+    let expected = r#"  × abc def
+  │ ghi jkl
+  │ mno pqr
+  │ stu vwx
+  │ yz abc
+  │ def ghi
+  │ jkl mno
+  │ pqr stu
+  │ vwx yz
+"#
+    .to_string();
+    assert_eq!(expected, out);
+
+    // Unless, wrapping is disabled
+    let out = fmt_report_with_settings(
+        Report::msg("abc def ghi jkl mno pqr stu vwx yz abc def ghi jkl mno pqr stu vwx yz"),
+        |handler| handler.with_width(15).with_wrap_lines(false),
+    );
+    let expected =
+        "  × 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
+    let out = fmt_report_with_settings(
+      Report::msg("abc def ghi jkl mno pqr stu vwx yz\nabc def ghi jkl mno pqr stu vwx yz\nabc def ghi jkl mno pqr stu vwx yz"),
+      |handler| handler.with_width(15).with_wrap_lines(false),
+  );
+    let expected = r#"  × abc def ghi jkl mno pqr stu vwx yz
+  │ abc def ghi jkl mno pqr stu vwx yz
+  │ abc def ghi jkl mno pqr stu vwx yz
+"#
+    .to_string();
+    assert_eq!(expected, out);
+
+    Ok(())
+}
+
 #[test]
 fn empty_source() -> Result<(), MietteError> {
     #[derive(Debug, Diagnostic, Error)]
@@ -1713,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(())
+}