From d370aa75af99da3e0c41ceb28e2d02ee81cd2538 Mon Sep 17 00:00:00 2001 From: Josh McKinney Date: Mon, 10 Jun 2024 11:51:54 -0700 Subject: [PATCH] fix(span): ensure that zero-width characters are rendered correctly (#1165) --- src/buffer/cell.rs | 130 +++++++++++++++++++++++++++++++++++++++++++-- src/text/span.rs | 118 +++++++++++++++++++++++++++++++++------- 2 files changed, 224 insertions(+), 24 deletions(-) diff --git a/src/buffer/cell.rs b/src/buffer/cell.rs index 858331f0..b034349f 100644 --- a/src/buffer/cell.rs +++ b/src/buffer/cell.rs @@ -67,6 +67,14 @@ impl Cell { self } + /// Appends a symbol to the cell. + /// + /// This is particularly useful for adding zero-width characters to the cell. + pub(crate) fn append_symbol(&mut self, symbol: &str) -> &mut Self { + self.symbol.push_str(symbol); + self + } + /// Sets the symbol of the cell to a single character. pub fn set_char(&mut self, ch: char) -> &mut Self { let mut buf = [0; 4]; @@ -154,16 +162,128 @@ mod tests { use super::*; #[test] - fn symbol_field() { - let mut cell = Cell::EMPTY; + fn new() { + let cell = Cell::new("ใ‚"); + assert_eq!( + cell, + Cell { + symbol: CompactString::new_inline("ใ‚"), + fg: Color::Reset, + bg: Color::Reset, + #[cfg(feature = "underline-color")] + underline_color: Color::Reset, + modifier: Modifier::empty(), + skip: false, + } + ); + } + + #[test] + fn empty() { + let cell = Cell::EMPTY; assert_eq!(cell.symbol(), " "); + } + + #[test] + fn set_symbol() { + let mut cell = Cell::EMPTY; cell.set_symbol("ใ‚"); // Multi-byte character assert_eq!(cell.symbol(), "ใ‚"); cell.set_symbol("๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ"); // Multiple code units combined with ZWJ assert_eq!(cell.symbol(), "๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ"); + } - // above Cell::EMPTY is put into a mutable variable and is changed then. - // While this looks like it might change the constant, it actually doesnt: - assert_eq!(Cell::EMPTY.symbol(), " "); + #[test] + fn append_symbol() { + let mut cell = Cell::EMPTY; + cell.set_symbol("ใ‚"); // Multi-byte character + cell.append_symbol("\u{200B}"); // zero-width space + assert_eq!(cell.symbol(), "ใ‚\u{200B}"); + } + + #[test] + fn set_char() { + let mut cell = Cell::EMPTY; + cell.set_char('ใ‚'); // Multi-byte character + assert_eq!(cell.symbol(), "ใ‚"); + } + + #[test] + fn set_fg() { + let mut cell = Cell::EMPTY; + cell.set_fg(Color::Red); + assert_eq!(cell.fg, Color::Red); + } + + #[test] + fn set_bg() { + let mut cell = Cell::EMPTY; + cell.set_bg(Color::Red); + assert_eq!(cell.bg, Color::Red); + } + + #[test] + fn set_style() { + let mut cell = Cell::EMPTY; + cell.set_style(Style::new().fg(Color::Red).bg(Color::Blue)); + assert_eq!(cell.fg, Color::Red); + assert_eq!(cell.bg, Color::Blue); + } + + #[test] + fn set_skip() { + let mut cell = Cell::EMPTY; + cell.set_skip(true); + assert!(cell.skip); + } + + #[test] + fn reset() { + let mut cell = Cell::EMPTY; + cell.set_symbol("ใ‚"); + cell.set_fg(Color::Red); + cell.set_bg(Color::Blue); + cell.set_skip(true); + cell.reset(); + assert_eq!(cell.symbol(), " "); + assert_eq!(cell.fg, Color::Reset); + assert_eq!(cell.bg, Color::Reset); + assert!(!cell.skip); + } + + #[test] + fn style() { + let cell = Cell::EMPTY; + assert_eq!( + cell.style(), + Style { + fg: Some(Color::Reset), + bg: Some(Color::Reset), + #[cfg(feature = "underline-color")] + underline_color: Some(Color::Reset), + add_modifier: Modifier::empty(), + sub_modifier: Modifier::empty(), + } + ); + } + + #[test] + fn default() { + let cell = Cell::default(); + assert_eq!(cell.symbol(), " "); + } + + #[test] + fn cell_eq() { + let cell1 = Cell::new("ใ‚"); + let cell2 = Cell::new("ใ‚"); + assert_eq!(cell1, cell2); + } + + #[test] + fn cell_ne() { + let cell1 = Cell::new("ใ‚"); + let cell2 = Cell::new("ใ„"); + assert_ne!(cell1, cell2); } } diff --git a/src/text/span.rs b/src/text/span.rs index 990c276f..af134cfd 100644 --- a/src/text/span.rs +++ b/src/text/span.rs @@ -362,34 +362,46 @@ impl Widget for Span<'_> { impl WidgetRef for Span<'_> { fn render_ref(&self, area: Rect, buf: &mut Buffer) { - let area = area.intersection(buf.area); - let Rect { - x: mut current_x, - y, - width, - .. - } = area; - let max_x = Ord::min(current_x.saturating_add(width), buf.area.right()); - for g in self.styled_graphemes(Style::default()) { - let symbol_width = g.symbol.width(); - let next_x = current_x.saturating_add(symbol_width as u16); - if next_x > max_x { + let Rect { mut x, y, .. } = area.intersection(buf.area); + for (i, grapheme) in self.styled_graphemes(Style::default()).enumerate() { + let symbol_width = grapheme.symbol.width(); + let next_x = x.saturating_add(symbol_width as u16); + if next_x > area.intersection(buf.area).right() { break; } - buf.get_mut(current_x, y) - .set_symbol(g.symbol) - .set_style(g.style); + + if i == 0 { + // the first grapheme is always set on the cell + buf.get_mut(x, y) + .set_symbol(grapheme.symbol) + .set_style(grapheme.style); + } else if x == area.x { + // there is one or more zero-width graphemes in the first cell, so the first cell + // must be appended to. + buf.get_mut(x, y) + .append_symbol(grapheme.symbol) + .set_style(grapheme.style); + } else if symbol_width == 0 { + // append zero-width graphemes to the previous cell + buf.get_mut(x - 1, y) + .append_symbol(grapheme.symbol) + .set_style(grapheme.style); + } else { + // just a normal grapheme (not first, not zero-width, not overflowing the area) + buf.get_mut(x, y) + .set_symbol(grapheme.symbol) + .set_style(grapheme.style); + } // multi-width graphemes must clear the cells of characters that are hidden by the // grapheme, otherwise the hidden characters will be re-rendered if the grapheme is // overwritten. - for i in (current_x + 1)..next_x { - buf.get_mut(i, y).reset(); + for x_hidden in (x + 1)..next_x { // it may seem odd that the style of the hidden cells are not set to the style of // the grapheme, but this is how the existing buffer.set_span() method works. - // buf.get_mut(i, y).set_style(g.style); + buf.get_mut(x_hidden, y).reset(); } - current_x = next_x; + x = next_x; } } } @@ -402,6 +414,7 @@ impl fmt::Display for Span<'_> { #[cfg(test)] mod tests { + use buffer::Cell; use rstest::fixture; use super::*; @@ -663,5 +676,72 @@ mod tests { ])]); assert_eq!(buf, expected); } + + #[test] + fn render_first_zero_width() { + let span = Span::raw("\u{200B}abc"); + let mut buf = Buffer::empty(Rect::new(0, 0, 3, 1)); + span.render(buf.area, &mut buf); + assert_eq!( + buf.content(), + [Cell::new("\u{200B}a"), Cell::new("b"), Cell::new("c"),] + ); + } + + #[test] + fn render_second_zero_width() { + let span = Span::raw("a\u{200B}bc"); + let mut buf = Buffer::empty(Rect::new(0, 0, 3, 1)); + span.render(buf.area, &mut buf); + assert_eq!( + buf.content(), + [Cell::new("a\u{200B}"), Cell::new("b"), Cell::new("c")] + ); + } + + #[test] + fn render_middle_zero_width() { + let span = Span::raw("ab\u{200B}c"); + let mut buf = Buffer::empty(Rect::new(0, 0, 3, 1)); + span.render(buf.area, &mut buf); + assert_eq!( + buf.content(), + [Cell::new("a"), Cell::new("b\u{200B}"), Cell::new("c")] + ); + } + + #[test] + fn render_last_zero_width() { + let span = Span::raw("abc\u{200B}"); + let mut buf = Buffer::empty(Rect::new(0, 0, 3, 1)); + span.render(buf.area, &mut buf); + assert_eq!( + buf.content(), + [Cell::new("a"), Cell::new("b"), Cell::new("c\u{200B}")] + ); + } + } + + /// Regression test for One line contains + /// some Unicode Left-Right-Marks (U+200E) + /// + /// The issue was that a zero-width character at the end of the buffer causes the buffer bounds + /// to be exceeded (due to a position + 1 calculation that fails to account for the possibility + /// that the next position might not be available). + #[test] + fn issue_1160() { + let span = Span::raw("Hello\u{200E}"); + let mut buf = Buffer::empty(Rect::new(0, 0, 5, 1)); + span.render(buf.area, &mut buf); + assert_eq!( + buf.content(), + [ + Cell::new("H"), + Cell::new("e"), + Cell::new("l"), + Cell::new("l"), + Cell::new("o\u{200E}"), + ] + ); } }