From 6cd44a86c6e6f1d9c79006d4cfa89220dbd3a7b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kat=20March=C3=A1n?= Date: Mon, 20 Sep 2021 14:27:25 -0700 Subject: [PATCH] feat(handlers): Update graphical handler to use new label protocol (#66) --- src/eyreish/mod.rs | 6 +-- src/handler.rs | 6 +-- src/handlers/graphical.rs | 109 ++++++++++++++++++++++++-------------- src/named_source.rs | 2 + src/protocol.rs | 39 ++++++++++++-- src/source_impls.rs | 81 +++++++++++++++++++++------- 6 files changed, 172 insertions(+), 71 deletions(-) diff --git a/src/eyreish/mod.rs b/src/eyreish/mod.rs index 06be76e9..f051dd4d 100644 --- a/src/eyreish/mod.rs +++ b/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; diff --git a/src/handler.rs b/src/handler.rs index ea2551c1..b47a37f5 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -205,11 +205,7 @@ impl Default for MietteHandler { } impl ReportHandler for MietteHandler { - fn debug( - &self, - diagnostic: &(dyn Diagnostic + 'static), - f: &mut fmt::Formatter<'_>, - ) -> fmt::Result { + fn debug(&self, diagnostic: &(dyn Diagnostic), f: &mut fmt::Formatter<'_>) -> fmt::Result { if f.alternate() { return fmt::Debug::fmt(diagnostic, f); } diff --git a/src/handlers/graphical.rs b/src/handlers/graphical.rs index a960fc48..671ea9f8 100644 --- a/src/handlers/graphical.rs +++ b/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, + 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, ) -> fmt::Result { - // TODO: Actually do the rewrite against the new protocol. - let contexts: Vec<_> = labels + let contents = labels .iter() - .cloned() - .coalesce(|left, right| { - if left.offset() + left.len() >= right.offset() { - let left_end = left.offset() + left.len(); - let right_end = right.offset() + right.len(); - Ok(LabeledSpan::new( - left.label().map(String::from), - left.offset(), - right_end - left_end, + .map(|label| source.read_span(label.inner(), self.context_lines, self.context_lines)) + .collect::>>, MietteError>>() + .map_err(|_| fmt::Error)?; + let contexts = labels.iter().cloned().zip(contents.iter()).coalesce( + |(left, left_conts), (right, right_conts)| { + 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 + 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() { - // let source_name = source_name.style(self.theme.styles.link); - // writeln!( - // f, - // "[{}:{}:{}]", - // source_name, - // contents.line() + 1, - // contents.column() + 1 - // )?; - // } else { - // writeln!(f, "[{}:{}]", contents.line() + 1, contents.column() + 1)?; - // } + + if let Some(source_name) = contents.name() { + let source_name = source_name.style(self.theme.styles.link); + writeln!( + f, + "[{}:{}:{}]", + source_name, + contents.line() + 1, + contents.column() + 1 + )?; + } else if lines.len() == 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 + 'a>, Vec), 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, diff --git a/src/named_source.rs b/src/named_source.rs index 656cc3d9..6d5e943d 100644 --- a/src/named_source.rs +++ b/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(), ))) } } diff --git a/src/protocol.rs b/src/protocol.rs index 94c97082..872d525b 100644 --- a/src/protocol.rs +++ b/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, } 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 { diff --git a/src/source_impls.rs b/src/source_impls.rs index fccab2b4..c1733583 100644 --- a/src/source_impls.rs +++ b/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, 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 { - Ok(( - &input[before_lines_starts - .get(0) - .copied() - .unwrap_or_else(|| span.offset())..offset], + // These lines are commented as a hat-tip towards better failures. For the + // case of error reporting _specifically_, exposing a bug in your own + // error span calculation code by crashing with an opaque fmt::Error is + // not ideal. Instead, we go ahead and return what we've collected so far + // in out-of-range cases and just let it render without or with a missing + // 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 + 'a>, MietteError> { - let (data, start_line, start_column) = - context_info(self, span, context_lines_before, context_lines_after)?; - return Ok(Box::new(MietteSpanContents::new( - data, - start_line, - start_column, - ))); + let contents = context_info(self, span, context_lines_before, context_lines_after)?; + Ok(Box::new(contents)) } } @@ -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(()) + } }