feat(handlers): Update graphical handler to use new label protocol (#66)

src/eyreish/mod.rs

@@ -26,11 +26,11 @@ pub use ReportHandler as EyreContext;
 #[allow(unreachable_pub)]
 pub use WrapErr as Context;
 
+#[cfg(not(feature = "fancy"))]
+use crate::DebugReportHandler;
 use crate::Diagnostic;
 #[cfg(feature = "fancy")]
 use crate::MietteHandler;
-#[cfg(not(feature = "fancy"))]
-use crate::DebugReportHandler;
 
 use error::ErrorImpl;
 
@@ -165,7 +165,7 @@ pub trait ReportHandler: core::any::Any + Send + Sync {
     /// ```
     fn debug(
         &self,
-        error: &(dyn Diagnostic + 'static),
+        error: &(dyn Diagnostic),
         f: &mut core::fmt::Formatter<'_>,
     ) -> core::fmt::Result;
 

src/handler.rs

@@ -205,11 +205,7 @@ impl Default for MietteHandler {
 }
 
 impl ReportHandler for MietteHandler {
-    fn debug(
+    fn debug(&self, diagnostic: &(dyn Diagnostic), f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        &self,
-        diagnostic: &(dyn Diagnostic + 'static),
-        f: &mut fmt::Formatter<'_>,
-    ) -> fmt::Result {
         if f.alternate() {
             return fmt::Debug::fmt(diagnostic, f);
         }

src/handlers/graphical.rs

@@ -7,7 +7,7 @@ use unicode_width::UnicodeWidthStr;
 use crate::chain::Chain;
 use crate::handlers::theme::*;
 use crate::protocol::{Diagnostic, Severity};
-use crate::{LabeledSpan, ReportHandler, SourceCode, SourceSpan, SpanContents};
+use crate::{LabeledSpan, MietteError, ReportHandler, SourceCode, SourceSpan, SpanContents};
 
 /**
 A [ReportHandler] that displays a given [crate::Report] in a quasi-graphical
@@ -26,6 +26,7 @@ pub struct GraphicalReportHandler {
     pub(crate) termwidth: usize,
     pub(crate) theme: GraphicalTheme,
     pub(crate) footer: Option<String>,
+    pub(crate) context_lines: usize,
 }
 
 impl GraphicalReportHandler {
@@ -37,6 +38,7 @@ impl GraphicalReportHandler {
             termwidth: 200,
             theme: GraphicalTheme::default(),
             footer: None,
+            context_lines: 1,
         }
     }
 
@@ -47,6 +49,7 @@ impl GraphicalReportHandler {
             termwidth: 200,
             theme,
             footer: None,
+            context_lines: 1,
         }
     }
 
@@ -73,6 +76,12 @@ impl GraphicalReportHandler {
         self.footer = Some(footer);
         self
     }
+
+    /// Sets the number of lines of context to show around each error.
+    pub fn with_context_lines(mut self, lines: usize) -> Self {
+        self.context_lines = lines;
+        self
+    }
 }
 
 impl Default for GraphicalReportHandler {
@@ -239,25 +248,51 @@ impl GraphicalReportHandler {
         source: &dyn SourceCode,
         labels: Vec<LabeledSpan>,
     ) -> fmt::Result {
-        // TODO: Actually do the rewrite against the new protocol.
+        let contents = labels
-        let contexts: Vec<_> = labels
             .iter()
-            .cloned()
+            .map(|label| source.read_span(label.inner(), self.context_lines, self.context_lines))
-            .coalesce(|left, right| {
+            .collect::<Result<Vec<Box<dyn SpanContents<'_>>>, MietteError>>()
-                if left.offset() + left.len() >= right.offset() {
+            .map_err(|_| fmt::Error)?;
-                    let left_end = left.offset() + left.len();
+        let contexts = labels.iter().cloned().zip(contents.iter()).coalesce(
-                    let right_end = right.offset() + right.len();
+            |(left, left_conts), (right, right_conts)| {
-                    Ok(LabeledSpan::new(
+                let left_end = left.offset() + left.len();
-                        left.label().map(String::from),
+                let right_end = right.offset() + right.len();
-                        left.offset(),
+                if left_conts.line() + left_conts.line_count() >= right_conts.line() {
-                        right_end - left_end,
+                    // The snippets will overlap, so we create one Big Chunky Boi
+                    Ok((
+                        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()
+                            },
+                        ),
+                        // We'll throw this away later
+                        left_conts,
                     ))
                 } else {
-                    Err((left, right))
+                    Err(((left, left_conts), (right, right_conts)))
                 }
-            })
+            },
-            .collect();
+        );
-        let (contents, lines) = self.get_lines(source, &labels)?;
+        for (ctx, _) in contexts {
+            self.render_context(f, source, &ctx, &labels[..])?;
+        }
+        Ok(())
+    }
+
+    fn render_context<'a>(
+        &self,
+        f: &mut impl fmt::Write,
+        source: &'a dyn SourceCode,
+        context: &LabeledSpan,
+        labels: &[LabeledSpan],
+    ) -> fmt::Result {
+        let (contents, lines) = self.get_lines(source, context.inner())?;
 
         // sorting is your friend
         let labels = labels
@@ -298,19 +333,21 @@ impl GraphicalReportHandler {
             self.theme.characters.ltop,
             self.theme.characters.hbar,
         )?;
-        // TODO: filenames
+
-        // if let Some(source_name) = source.name() {
+        if let Some(source_name) = contents.name() {
-        //     let source_name = source_name.style(self.theme.styles.link);
+            let source_name = source_name.style(self.theme.styles.link);
-        //     writeln!(
+            writeln!(
-        //         f,
+                f,
-        //         "[{}:{}:{}]",
+                "[{}:{}:{}]",
-        //         source_name,
+                source_name,
-        //         contents.line() + 1,
+                contents.line() + 1,
-        //         contents.column() + 1
+                contents.column() + 1
-        //     )?;
+            )?;
-        // } else {
+        } else if lines.len() == 1 {
-        //     writeln!(f, "[{}:{}]", contents.line() + 1, contents.column() + 1)?;
+            writeln!(f, "{}", self.theme.characters.hbar.to_string().repeat(3))?;
-        // }
+        } else {
+            writeln!(f, "[{}:{}]", contents.line() + 1, contents.column() + 1)?;
+        }
 
         // Blank line to improve readability
         writeln!(
@@ -574,22 +611,15 @@ impl GraphicalReportHandler {
     fn get_lines<'a>(
         &'a self,
         source: &'a dyn SourceCode,
-        labels: &'a [LabeledSpan],
+        context_span: &'a SourceSpan,
     ) -> Result<(Box<dyn SpanContents<'a> + 'a>, Vec<Line>), fmt::Error> {
-        let first = labels.first().expect("MIETTE BUG: This should be safe.");
-        let last = labels.last().expect("MIETTE BUG: This should be safe.");
-        let context_span = (
-            first.inner().offset(),
-            last.inner().offset() + last.inner().len(),
-        )
-            .into();
         let context_data = source
-            .read_span(&context_span, 1, 1)
+            .read_span(context_span, self.context_lines, self.context_lines)
             .map_err(|_| fmt::Error)?;
         let context = std::str::from_utf8(context_data.data()).expect("Bad utf8 detected");
         let mut line = context_data.line();
         let mut column = context_data.column();
-        let mut offset = context_span.offset();
+        let mut offset = context_data.span().offset();
         let mut line_offset = offset;
         let mut iter = context.chars().peekable();
         let mut line_str = String::new();
@@ -653,6 +683,7 @@ impl ReportHandler for GraphicalReportHandler {
 Support types
 */
 
+#[derive(Debug)]
 struct Line {
     line_number: usize,
     offset: usize,

src/named_source.rs

@@ -45,8 +45,10 @@ impl SourceCode for NamedSource {
         Ok(Box::new(MietteSpanContents::new_named(
             self.name.clone(),
             contents.data(),
+            contents.span().clone(),
             contents.line(),
             contents.column(),
+            contents.line_count(),
         )))
     }
 }

src/protocol.rs

@@ -243,8 +243,10 @@ Includes line and column information to optimize highlight calculations.
 pub trait SpanContents<'a> {
     /// Reference to the data inside the associated span, in bytes.
     fn data(&self) -> &'a [u8];
+    /// [SourceSpan] representing the span covered by this SpanContents.
+    fn span(&self) -> &SourceSpan;
     /// An optional (file?) name for the container of this SpanContents.
-    fn name(&self) -> Option<&'a str> {
+    fn name(&self) -> Option<&str> {
         None
     }
     /// The 0-indexed line in the associated [SourceCode] where the data begins.
@@ -252,6 +254,8 @@ pub trait SpanContents<'a> {
     /// The 0-indexed column in the associated [SourceCode] where the data begins,
     /// relative to `line`.
     fn column(&self) -> usize;
+    /// Total number of lines covered by this SpanContents.
+    fn line_count(&self) -> usize;
 }
 
 /**
@@ -259,23 +263,35 @@ Basic implementation of the [SpanContents] trait, for convenience.
 */
 #[derive(Clone, Debug)]
 pub struct MietteSpanContents<'a> {
-    /// Data from a [SourceCode], in bytes.
+    // Data from a [SourceCode], in bytes.
     data: &'a [u8],
+    // span actually covered by this SpanContents.
+    span: SourceSpan,
     // The 0-indexed line where the associated [SourceSpan] _starts_.
     line: usize,
     // The 0-indexed column where the associated [SourceSpan] _starts_.
     column: usize,
+    // Number of line in this snippet.
+    line_count: usize,
     // Optional filename
     name: Option<String>,
 }
 
 impl<'a> MietteSpanContents<'a> {
     /// Make a new [MietteSpanContents] object.
-    pub fn new(data: &'a [u8], line: usize, column: usize) -> MietteSpanContents<'a> {
+    pub fn new(
+        data: &'a [u8],
+        span: SourceSpan,
+        line: usize,
+        column: usize,
+        line_count: usize,
+    ) -> MietteSpanContents<'a> {
         MietteSpanContents {
             data,
+            span,
             line,
             column,
+            line_count,
             name: None,
         }
     }
@@ -284,13 +300,17 @@ impl<'a> MietteSpanContents<'a> {
     pub fn new_named(
         name: String,
         data: &'a [u8],
+        span: SourceSpan,
         line: usize,
         column: usize,
+        line_count: usize,
     ) -> MietteSpanContents<'a> {
         MietteSpanContents {
             data,
+            span,
             line,
             column,
+            line_count,
             name: Some(name),
         }
     }
@@ -300,18 +320,27 @@ impl<'a> SpanContents<'a> for MietteSpanContents<'a> {
     fn data(&self) -> &'a [u8] {
         self.data
     }
+    fn span(&self) -> &SourceSpan {
+        &self.span
+    }
     fn line(&self) -> usize {
         self.line
     }
     fn column(&self) -> usize {
         self.column
     }
+    fn line_count(&self) -> usize {
+        self.line_count
+    }
+    fn name(&self) -> Option<&str> {
+        self.name.as_deref()
+    }
 }
 
 /**
 Span within a [SourceCode] with an associated message.
 */
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub struct SourceSpan {
     /// The start of the span.
     offset: SourceOffset,
@@ -371,7 +400,7 @@ pub type ByteOffset = usize;
 /**
 Newtype that represents the [ByteOffset] from the beginning of a [SourceCode]
 */
-#[derive(Clone, Copy, Debug)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
 pub struct SourceOffset(ByteOffset);
 
 impl SourceOffset {

src/source_impls.rs

@@ -14,17 +14,20 @@ fn context_info<'a>(
     span: &SourceSpan,
     context_lines_before: usize,
     context_lines_after: usize,
-) -> Result<(&'a [u8], usize, usize), MietteError> {
+) -> Result<MietteSpanContents<'a>, MietteError> {
     let mut offset = 0usize;
+    let mut line_count = 0usize;
     let mut start_line = 0usize;
     let mut start_column = 0usize;
     let mut before_lines_starts = Vec::new();
     let mut current_line_start = 0usize;
     let mut end_lines = 0usize;
     let mut post_span = false;
+    let mut post_span_got_newline = false;
     let mut iter = input.iter().copied().peekable();
     while let Some(char) = iter.next() {
         if matches!(char, b'\r' | b'\n') {
+            line_count += 1;
             if char == b'\r' && iter.next_if_eq(&b'\n').is_some() {
                 offset += 1;
             }
@@ -41,9 +44,13 @@ fn context_info<'a>(
                 // started collecting end lines yet (we might still be
                 // collecting context lines).
                 if post_span {
-                    end_lines += 1;
                     start_column = 0;
-                    if end_lines > context_lines_after {
+                    if post_span_got_newline {
+                        end_lines += 1;
+                    } else {
+                        post_span_got_newline = true;
+                    }
+                    if end_lines >= context_lines_after {
                         offset += 1;
                         break;
                     }
@@ -54,7 +61,7 @@ fn context_info<'a>(
             start_column += 1;
         }
 
-        if offset >= span.offset() + span.len() - 1 {
+        if offset >= (span.offset() + span.len()).saturating_sub(1) {
             post_span = true;
             if end_lines >= context_lines_after {
                 offset += 1;
@@ -65,20 +72,36 @@ fn context_info<'a>(
         offset += 1;
     }
 
-    if offset >= span.offset() + span.len() - 1 {
+    // These lines are commented as a hat-tip towards better failures. For the
-        Ok((
+    // case of error reporting _specifically_, exposing a bug in your own
-            &input[before_lines_starts
+    // error span calculation code by crashing with an opaque fmt::Error is
-                .get(0)
+    // not ideal. Instead, we go ahead and return what we've collected so far
-                .copied()
+    // in out-of-range cases and just let it render without or with a missing
-                .unwrap_or_else(|| span.offset())..offset],
+    // label.
+    //
+    // Old code is left here in case we change our minds and we want to bring back more strict calculation.
+
+    if offset >= (span.offset() + span.len()).saturating_sub(1) {
+        let starting_offset = before_lines_starts.get(0).copied().unwrap_or_else(|| {
+            if line_count > 0 {
+                span.offset()
+            } else {
+                0
+            }
+        });
+        Ok(MietteSpanContents::new(
+            &input[starting_offset..offset],
+            (starting_offset, offset - starting_offset).into(),
             start_line,
             if context_lines_before == 0 {
                 start_column
             } else {
                 0
             },
+            line_count,
         ))
     } else {
+        // eprintln!("Out of bounds :(");
         Err(MietteError::OutOfBounds)
     }
 }
@@ -90,13 +113,8 @@ impl SourceCode for [u8] {
         context_lines_before: usize,
         context_lines_after: usize,
     ) -> Result<Box<dyn SpanContents<'a> + 'a>, MietteError> {
-        let (data, start_line, start_column) =
+        let contents = context_info(self, span, context_lines_before, context_lines_after)?;
-            context_info(self, span, context_lines_before, context_lines_after)?;
+        Ok(Box::new(contents))
-        return Ok(Box::new(MietteSpanContents::new(
-            data,
-            start_line,
-            start_column,
-        )));
     }
 }
 
@@ -193,6 +211,16 @@ mod tests {
         Ok(())
     }
 
+    #[test]
+    fn shifted() -> Result<(), MietteError> {
+        let src = String::from("foobar");
+        let contents = src.read_span(&(3, 3).into(), 1, 1)?;
+        assert_eq!("foobar", std::str::from_utf8(contents.data()).unwrap());
+        assert_eq!(0, contents.line());
+        assert_eq!(0, contents.column());
+        Ok(())
+    }
+
     #[test]
     fn middle() -> Result<(), MietteError> {
         let src = String::from("foo\nbar\nbaz\n");
@@ -226,13 +254,28 @@ mod tests {
     #[test]
     fn with_context() -> Result<(), MietteError> {
         let src = String::from("xxx\nfoo\nbar\nbaz\n\nyyy\n");
-        let contents = src.read_span(&(8, 4).into(), 1, 2)?;
+        let contents = src.read_span(&(8, 3).into(), 1, 1)?;
         assert_eq!(
-            "foo\nbar\nbaz\n\n",
+            "foo\nbar\nbaz\n",
             std::str::from_utf8(contents.data()).unwrap()
         );
         assert_eq!(1, contents.line());
         assert_eq!(0, contents.column());
         Ok(())
     }
+
+    #[test]
+    fn multiline_with_context() -> Result<(), MietteError> {
+        let src = String::from("aaa\nxxx\n\nfoo\nbar\nbaz\n\nyyy\nbbb\n");
+        let contents = src.read_span(&(9, 11).into(), 1, 1)?;
+        assert_eq!(
+            "\nfoo\nbar\nbaz\n\n",
+            std::str::from_utf8(contents.data()).unwrap()
+        );
+        assert_eq!(2, contents.line());
+        assert_eq!(0, contents.column());
+        let span: SourceSpan = (8, 14).into();
+        assert_eq!(&span, contents.span());
+        Ok(())
+    }
 }