From 10ba35ed9874bed55f4850343283c79321d3abc9 Mon Sep 17 00:00:00 2001 From: Brendan Zabarauskas Date: Mon, 22 Jun 2020 19:30:49 +1000 Subject: [PATCH] Fix rendering of tabulation columns --- codespan-reporting/src/term/config.rs | 51 --------- codespan-reporting/src/term/renderer.rs | 100 ++++++++++++------ ...rm__tab_columns__tab_width_2_no_color.snap | 23 ++++ ...rm__tab_columns__tab_width_3_no_color.snap | 23 ++++ ...rm__tab_columns__tab_width_6_no_color.snap | 23 ++++ ...b_columns__tab_width_default_no_color.snap | 23 ++++ .../term__tabbed__tab_width_3_no_color.snap | 4 +- .../term__tabbed__tab_width_6_no_color.snap | 4 +- ...m__tabbed__tab_width_default_no_color.snap | 4 +- codespan-reporting/tests/term.rs | 81 +++++++++++++- 10 files changed, 243 insertions(+), 93 deletions(-) create mode 100644 codespan-reporting/tests/snapshots/term__tab_columns__tab_width_2_no_color.snap create mode 100644 codespan-reporting/tests/snapshots/term__tab_columns__tab_width_3_no_color.snap create mode 100644 codespan-reporting/tests/snapshots/term__tab_columns__tab_width_6_no_color.snap create mode 100644 codespan-reporting/tests/snapshots/term__tab_columns__tab_width_default_no_color.snap diff --git a/codespan-reporting/src/term/config.rs b/codespan-reporting/src/term/config.rs index 8c68ac7..7b834d7 100644 --- a/codespan-reporting/src/term/config.rs +++ b/codespan-reporting/src/term/config.rs @@ -1,4 +1,3 @@ -use std::io; use termcolor::{Color, ColorSpec}; use crate::diagnostic::{LabelStyle, Severity}; @@ -31,56 +30,6 @@ impl Default for Config { } } -impl Config { - /// Measure the unicode width of a character, taking into account the tab width. - pub fn width(&self, ch: char) -> usize { - use unicode_width::UnicodeWidthChar; - - match ch { - '\t' => self.tab_width, - _ => ch.width().unwrap_or(0), - } - } - - /// Construct a source writer using the current config. - pub fn source<'a, W: ?Sized>(&self, writer: &'a mut W) -> SourceWriter<&'a mut W> { - SourceWriter { - upstream: writer, - tab_width: self.tab_width, - } - } -} - -/// Writer that replaces tab characters with the configured number of spaces. -pub struct SourceWriter { - upstream: W, - tab_width: usize, -} - -impl io::Write for SourceWriter { - fn write(&mut self, buf: &[u8]) -> io::Result { - let mut last_term = 0usize; - for (i, ch) in buf.iter().enumerate() { - if *ch == b'\t' { - self.upstream.write_all(&buf[last_term..i])?; - last_term = i + 1; - write!( - self.upstream, - "{space: >width$}", - space = "", - width = self.tab_width, - )?; - } - } - self.upstream.write_all(&buf[last_term..])?; - Ok(buf.len()) - } - - fn flush(&mut self) -> io::Result<()> { - self.upstream.flush() - } -} - /// The display style to use when rendering diagnostics. #[derive(Clone, Debug)] pub enum DisplayStyle { diff --git a/codespan-reporting/src/term/renderer.rs b/codespan-reporting/src/term/renderer.rs index fb4029b..c288955 100644 --- a/codespan-reporting/src/term/renderer.rs +++ b/codespan-reporting/src/term/renderer.rs @@ -71,7 +71,6 @@ type Underline = (LabelStyle, VerticalBound); /// │ │ │ │ │ /// ┌──────────────────────────────────────────────────────────────────────────── /// header ── │ error[0001]: oh noes, a cupcake has occurred! -/// empty ── │ /// snippet start ── │ ┌─ test:9:0 /// snippet empty ── │ │ /// snippet line ── │ 9 │ ╭ Cupcake ipsum dolor. Sit amet marshmallow topping cheesecake @@ -265,8 +264,14 @@ impl<'writer, 'config> Renderer<'writer, 'config> { } } - // Write source - write!(self.config.source(self.writer), " {}", source)?; + // Write source text + write!(self, " ")?; + for (metrics, ch) in self.char_metrics(source.char_indices()) { + match ch { + '\t' => (0..metrics.unicode_width).try_for_each(|_| write!(self, " "))?, + _ => write!(self, "{}", ch)?, + } + } write!(self, "\n")?; } @@ -366,8 +371,12 @@ impl<'writer, 'config> Renderer<'writer, 'config> { write!(self, " ")?; let mut previous_label_style = None; - for (byte_index, ch) in source - .char_indices() + let placeholder_metrics = Metrics { + byte_index: source.len(), + unicode_width: 1, + }; + for (metrics, ch) in self + .char_metrics(source.char_indices()) // Add a placeholder source column at the end to allow for // printing carets at the end of lines, eg: // @@ -375,13 +384,10 @@ impl<'writer, 'config> Renderer<'writer, 'config> { // 1 │ Hello world! // │ ^ // ``` - // - // ' ' is used because its unicode width is equal to 1 according - // to `unicode_width::UnicodeWidthChar`. - .chain(std::iter::once((source.len(), ' '))) + .chain(std::iter::once((placeholder_metrics, '\0'))) { // Find the current label style at this column - let column_range = byte_index..(byte_index + ch.len_utf8()); + let column_range = metrics.byte_index..(metrics.byte_index + ch.len_utf8()); let current_label_style = single_labels .iter() .filter(|(_, range, _)| is_overlapping(range, &column_range)) @@ -402,12 +408,12 @@ impl<'writer, 'config> Renderer<'writer, 'config> { Some(LabelStyle::Primary) => Some(self.chars().single_primary_caret), Some(LabelStyle::Secondary) => Some(self.chars().single_secondary_caret), // Only print padding if we are before the end of the last single line caret - None if byte_index < max_label_end => Some(' '), + None if metrics.byte_index < max_label_end => Some(' '), None => None, }; if let Some(caret_ch) = caret_ch { // FIXME: improve rendering for carets that occurring within character boundaries - for _ in 0..self.config.width(ch) { + for _ in 0..metrics.unicode_width { write!(self, "{}", caret_ch)?; } } @@ -615,12 +621,39 @@ impl<'writer, 'config> Renderer<'writer, 'config> { Ok(()) } + /// Adds tab-stop aware unicode-width computations to an iterator over + /// character indices. Assumes that the character indices begin at the start + /// of the line. + fn char_metrics( + &self, + char_indices: impl Iterator, + ) -> impl Iterator { + use unicode_width::UnicodeWidthChar; + + let tab_width = self.config.tab_width; + let mut unicode_column = 0; + + char_indices.map(move |(byte_index, ch)| { + let metrics = Metrics { + byte_index, + unicode_width: match (ch, tab_width) { + ('\t', 0) => 0, // Guard divide-by-zero + ('\t', _) => tab_width - (unicode_column % tab_width), + (ch, _) => ch.width().unwrap_or(0), + }, + }; + unicode_column += metrics.unicode_width; + + (metrics, ch) + }) + } + /// Location focus. fn snippet_locus(&mut self, locus: &Locus) -> io::Result<()> { write!( self, - "{origin}:{line_number}:{column_number}", - origin = locus.name, + "{name}:{line_number}:{column_number}", + name = locus.name, line_number = locus.location.line_number, column_number = locus.location.column_number, ) @@ -628,7 +661,7 @@ impl<'writer, 'config> Renderer<'writer, 'config> { /// The outer gutter of a source line. fn outer_gutter(&mut self, outer_padding: usize) -> io::Result<()> { - write!(self, "{space: >width$}", space = "", width = outer_padding,)?; + write!(self, "{space: >width$}", space = "", width = outer_padding)?; write!(self, " ")?; Ok(()) } @@ -672,27 +705,25 @@ impl<'writer, 'config> Renderer<'writer, 'config> { trailing_label: Option<(usize, &SingleLabel<'_>)>, char_indices: impl Iterator, ) -> io::Result<()> { - for (byte_index, ch) in char_indices { - let column_range = byte_index..(byte_index + ch.len_utf8()); + for (metrics, ch) in self.char_metrics(char_indices) { + let column_range = metrics.byte_index..(metrics.byte_index + ch.len_utf8()); let label_style = hanging_labels(single_labels, trailing_label) .filter(|(_, range, _)| column_range.contains(&range.start)) .map(|(label_style, _, _)| *label_style) .max_by_key(label_priority_key); - let spaces = match label_style { - None => 0..self.config.width(ch), + let mut spaces = match label_style { + None => 0..metrics.unicode_width, Some(label_style) => { self.set_color(self.styles().label(severity, label_style))?; write!(self, "{}", self.chars().pointer_left)?; self.reset()?; - 1..self.config.width(ch) + 1..metrics.unicode_width } }; // Only print padding if we are before the end of the last single line caret - if byte_index <= max_label_start { - for _ in spaces { - write!(self, " ")?; - } + if metrics.byte_index <= max_label_start { + spaces.try_for_each(|_| write!(self, " "))?; } } @@ -773,12 +804,12 @@ impl<'writer, 'config> Renderer<'writer, 'config> { ) -> io::Result<()> { self.set_color(self.styles().label(severity, label_style))?; - for (_, ch) in source - .char_indices() - .take_while(|(byte_index, _)| *byte_index < range.end + 1) + for (metrics, _) in self + .char_metrics(source.char_indices()) + .take_while(|(metrics, _)| metrics.byte_index < range.end + 1) { // FIXME: improve rendering for carets that occurring within character boundaries - for _ in 0..self.config.width(ch) { + for _ in 0..metrics.unicode_width { write!(self, "{}", self.chars().multi_top)?; } } @@ -808,12 +839,12 @@ impl<'writer, 'config> Renderer<'writer, 'config> { ) -> io::Result<()> { self.set_color(self.styles().label(severity, label_style))?; - for (_, ch) in source - .char_indices() - .take_while(|(byte_index, _)| *byte_index < range.end) + for (metrics, _) in self + .char_metrics(source.char_indices()) + .take_while(|(metrics, _)| metrics.byte_index < range.end) { // FIXME: improve rendering for carets that occurring within character boundaries - for _ in 0..self.config.width(ch) { + for _ in 0..metrics.unicode_width { write!(self, "{}", self.chars().multi_bottom)?; } } @@ -915,6 +946,11 @@ impl<'writer, 'config> WriteColor for Renderer<'writer, 'config> { } } +struct Metrics { + byte_index: usize, + unicode_width: usize, +} + /// Check if two ranges overlap fn is_overlapping(range0: &Range, range1: &Range) -> bool { let start = std::cmp::max(range0.start, range1.start); diff --git a/codespan-reporting/tests/snapshots/term__tab_columns__tab_width_2_no_color.snap b/codespan-reporting/tests/snapshots/term__tab_columns__tab_width_2_no_color.snap new file mode 100644 index 0000000..0faa8d8 --- /dev/null +++ b/codespan-reporting/tests/snapshots/term__tab_columns__tab_width_2_no_color.snap @@ -0,0 +1,23 @@ +--- +source: codespan-reporting/tests/term.rs +expression: TEST_DATA.emit_no_color(&config) +--- +warning: tab test + ┌─ tab_columns:1:2 + │ +1 │ hello + │ ^^^^^ +2 │ ∙ hello + │ ^^^^^ +3 │ ∙∙ hello + │ ^^^^^ +4 │ ∙∙∙ hello + │ ^^^^^ +5 │ ∙∙∙∙ hello + │ ^^^^^ +6 │ ∙∙∙∙∙ hello + │ ^^^^^ +7 │ ∙∙∙∙∙∙ hello + │ ^^^^^ + + diff --git a/codespan-reporting/tests/snapshots/term__tab_columns__tab_width_3_no_color.snap b/codespan-reporting/tests/snapshots/term__tab_columns__tab_width_3_no_color.snap new file mode 100644 index 0000000..40f20a6 --- /dev/null +++ b/codespan-reporting/tests/snapshots/term__tab_columns__tab_width_3_no_color.snap @@ -0,0 +1,23 @@ +--- +source: codespan-reporting/tests/term.rs +expression: TEST_DATA.emit_no_color(&config) +--- +warning: tab test + ┌─ tab_columns:1:2 + │ +1 │ hello + │ ^^^^^ +2 │ ∙ hello + │ ^^^^^ +3 │ ∙∙ hello + │ ^^^^^ +4 │ ∙∙∙ hello + │ ^^^^^ +5 │ ∙∙∙∙ hello + │ ^^^^^ +6 │ ∙∙∙∙∙ hello + │ ^^^^^ +7 │ ∙∙∙∙∙∙ hello + │ ^^^^^ + + diff --git a/codespan-reporting/tests/snapshots/term__tab_columns__tab_width_6_no_color.snap b/codespan-reporting/tests/snapshots/term__tab_columns__tab_width_6_no_color.snap new file mode 100644 index 0000000..018a402 --- /dev/null +++ b/codespan-reporting/tests/snapshots/term__tab_columns__tab_width_6_no_color.snap @@ -0,0 +1,23 @@ +--- +source: codespan-reporting/tests/term.rs +expression: TEST_DATA.emit_no_color(&config) +--- +warning: tab test + ┌─ tab_columns:1:2 + │ +1 │ hello + │ ^^^^^ +2 │ ∙ hello + │ ^^^^^ +3 │ ∙∙ hello + │ ^^^^^ +4 │ ∙∙∙ hello + │ ^^^^^ +5 │ ∙∙∙∙ hello + │ ^^^^^ +6 │ ∙∙∙∙∙ hello + │ ^^^^^ +7 │ ∙∙∙∙∙∙ hello + │ ^^^^^ + + diff --git a/codespan-reporting/tests/snapshots/term__tab_columns__tab_width_default_no_color.snap b/codespan-reporting/tests/snapshots/term__tab_columns__tab_width_default_no_color.snap new file mode 100644 index 0000000..67cd187 --- /dev/null +++ b/codespan-reporting/tests/snapshots/term__tab_columns__tab_width_default_no_color.snap @@ -0,0 +1,23 @@ +--- +source: codespan-reporting/tests/term.rs +expression: TEST_DATA.emit_no_color(&config) +--- +warning: tab test + ┌─ tab_columns:1:2 + │ +1 │ hello + │ ^^^^^ +2 │ ∙ hello + │ ^^^^^ +3 │ ∙∙ hello + │ ^^^^^ +4 │ ∙∙∙ hello + │ ^^^^^ +5 │ ∙∙∙∙ hello + │ ^^^^^ +6 │ ∙∙∙∙∙ hello + │ ^^^^^ +7 │ ∙∙∙∙∙∙ hello + │ ^^^^^ + + diff --git a/codespan-reporting/tests/snapshots/term__tabbed__tab_width_3_no_color.snap b/codespan-reporting/tests/snapshots/term__tabbed__tab_width_3_no_color.snap index bb367ab..f7328c3 100644 --- a/codespan-reporting/tests/snapshots/term__tabbed__tab_width_3_no_color.snap +++ b/codespan-reporting/tests/snapshots/term__tabbed__tab_width_3_no_color.snap @@ -11,8 +11,8 @@ warning: unknown weapon `DogJaw` warning: unknown condition `attack-cooldown` ┌─ tabbed:4:23 │ -4 │ ReloadingCondition: attack-cooldown - │ ^^^^^^^^^^^^^^^ the condition +4 │ ReloadingCondition: attack-cooldown + │ ^^^^^^^^^^^^^^^ the condition warning: unknown field `Foo` ┌─ tabbed:5:2 diff --git a/codespan-reporting/tests/snapshots/term__tabbed__tab_width_6_no_color.snap b/codespan-reporting/tests/snapshots/term__tabbed__tab_width_6_no_color.snap index 14e97a2..bcd1b2b 100644 --- a/codespan-reporting/tests/snapshots/term__tabbed__tab_width_6_no_color.snap +++ b/codespan-reporting/tests/snapshots/term__tabbed__tab_width_6_no_color.snap @@ -11,8 +11,8 @@ warning: unknown weapon `DogJaw` warning: unknown condition `attack-cooldown` ┌─ tabbed:4:23 │ -4 │ ReloadingCondition: attack-cooldown - │ ^^^^^^^^^^^^^^^ the condition +4 │ ReloadingCondition: attack-cooldown + │ ^^^^^^^^^^^^^^^ the condition warning: unknown field `Foo` ┌─ tabbed:5:2 diff --git a/codespan-reporting/tests/snapshots/term__tabbed__tab_width_default_no_color.snap b/codespan-reporting/tests/snapshots/term__tabbed__tab_width_default_no_color.snap index 2ea4a27..d3a41a0 100644 --- a/codespan-reporting/tests/snapshots/term__tabbed__tab_width_default_no_color.snap +++ b/codespan-reporting/tests/snapshots/term__tabbed__tab_width_default_no_color.snap @@ -11,8 +11,8 @@ warning: unknown weapon `DogJaw` warning: unknown condition `attack-cooldown` ┌─ tabbed:4:23 │ -4 │ ReloadingCondition: attack-cooldown - │ ^^^^^^^^^^^^^^^ the condition +4 │ ReloadingCondition: attack-cooldown + │ ^^^^^^^^^^^^^^^ the condition warning: unknown field `Foo` ┌─ tabbed:5:2 diff --git a/codespan-reporting/tests/term.rs b/codespan-reporting/tests/term.rs index 600132a..ee02372 100644 --- a/codespan-reporting/tests/term.rs +++ b/codespan-reporting/tests/term.rs @@ -631,10 +631,83 @@ mod tabbed { fn tab_width_default_no_color() { let config = TEST_CONFIG.clone(); - insta::assert_snapshot!( - "tab_width_default_no_color", - TEST_DATA.emit_no_color(&config) - ); + insta::assert_snapshot!(TEST_DATA.emit_no_color(&config)); + } + + #[test] + fn tab_width_3_no_color() { + let config = Config { + tab_width: 3, + ..TEST_CONFIG.clone() + }; + + insta::assert_snapshot!(TEST_DATA.emit_no_color(&config)); + } + + #[test] + fn tab_width_6_no_color() { + let config = Config { + tab_width: 6, + ..TEST_CONFIG.clone() + }; + + insta::assert_snapshot!(TEST_DATA.emit_no_color(&config)); + } +} + +mod tab_columns { + use super::*; + + lazy_static::lazy_static! { + static ref TEST_DATA: TestData<'static, SimpleFiles<&'static str, String>> = { + let mut files = SimpleFiles::new(); + + let source = [ + "\thello", + "∙\thello", + "∙∙\thello", + "∙∙∙\thello", + "∙∙∙∙\thello", + "∙∙∙∙∙\thello", + "∙∙∙∙∙∙\thello", + ].join("\n"); + let hello_ranges = source + .match_indices("hello") + .map(|(start, hello)| start..(start+hello.len())) + .collect::>(); + + let file_id = files.add("tab_columns", source); + + let diagnostics = vec![ + Diagnostic::warning() + .with_message("tab test") + .with_labels( + hello_ranges + .into_iter() + .map(|range| Label::primary(file_id, range)) + .collect(), + ), + ]; + + TestData { files, diagnostics } + }; + } + + #[test] + fn tab_width_default_no_color() { + let config = TEST_CONFIG.clone(); + + insta::assert_snapshot!(TEST_DATA.emit_no_color(&config)); + } + + #[test] + fn tab_width_2_no_color() { + let config = Config { + tab_width: 2, + ..TEST_CONFIG.clone() + }; + + insta::assert_snapshot!(TEST_DATA.emit_no_color(&config)); } #[test]