From ce72025f03e4e87fe5fa462d602286c118ceafea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 4 Sep 2023 08:32:43 +0200 Subject: [PATCH 1/5] Don't panic if tab_width is 0 --- src/rendering/cursor.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/rendering/cursor.rs b/src/rendering/cursor.rs index 12f29d35..85506d69 100644 --- a/src/rendering/cursor.rs +++ b/src/rendering/cursor.rs @@ -29,7 +29,11 @@ impl LineCursor { /// Returns the distance to the next tab position. pub const fn next_tab_width(&self) -> u32 { - let next_tab_pos = (self.position / self.tab_width + 1) * self.tab_width; + let next_tab_pos = if self.tab_width == 0 { + self.position + } else { + (self.position / self.tab_width + 1) * self.tab_width + }; next_tab_pos - self.position } From 117d7c8e139bbf299e000a4b553d914eaa4cc25e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 4 Sep 2023 08:38:22 +0200 Subject: [PATCH 2/5] Ignore unexpected tokens instead of panicing --- src/plugin/mod.rs | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs index 0b2af4c2..3ee6dec7 100644 --- a/src/plugin/mod.rs +++ b/src/plugin/mod.rs @@ -177,15 +177,17 @@ where chars.as_str() }; - let token = match this.peeked_token.take().unwrap() { - Token::Whitespace(count, seq) => { - Token::Whitespace(count - len as u32, skip_chars(seq, len)) - } - Token::Word(w) => Token::Word(skip_chars(w, len)), - _ => unreachable!(), - }; - - this.peeked_token.replace(token); + if let Some(token) = this.peeked_token.take() { + let token = match token { + Token::Whitespace(count, seq) => { + Token::Whitespace(count - len as u32, skip_chars(seq, len)) + } + Token::Word(w) => Token::Word(skip_chars(w, len)), + _ => return, + }; + + this.peeked_token.replace(token); + } }) } From 01a9684836202095d462ae0cd9d65afde36cb335 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 4 Sep 2023 09:00:41 +0200 Subject: [PATCH 3/5] Avoid potentially double-measuring strings --- src/rendering/line.rs | 7 +++++-- src/rendering/line_iter.rs | 18 +++++++++--------- src/style/mod.rs | 4 ++-- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/rendering/line.rs b/src/rendering/line.rs index a4f158aa..382802f2 100644 --- a/src/rendering/line.rs +++ b/src/rendering/line.rs @@ -121,11 +121,14 @@ where Ok(()) } - fn printed_characters(&mut self, st: &str, width: u32) -> Result<(), Self::Error> { + fn printed_characters(&mut self, st: &str, width: Option) -> Result<(), Self::Error> { let top_left = self.pos; - self.style + let render_width = self + .style .draw_string(st, self.pos, Baseline::Top, self.display)?; + let width = width.unwrap_or((render_width - top_left).x as u32); + self.pos += Point::new(width.saturating_as(), 0); let size = Size::new(width, self.style.line_height().saturating_as()); diff --git a/src/rendering/line_iter.rs b/src/rendering/line_iter.rs index bb4d1d69..0edb0e7a 100644 --- a/src/rendering/line_iter.rs +++ b/src/rendering/line_iter.rs @@ -51,7 +51,7 @@ pub trait ElementHandler { } /// A string of printable characters. - fn printed_characters(&mut self, _st: &str, _width: u32) -> Result<(), Self::Error> { + fn printed_characters(&mut self, _st: &str, _width: Option) -> Result<(), Self::Error> { Ok(()) } @@ -319,7 +319,7 @@ where let width = handler.measure(c); if self.move_cursor(width.saturating_as()).is_ok() { if let Some(Token::Break(c, _)) = self.plugin.render_token(token) { - handler.printed_characters(c, width)?; + handler.printed_characters(c, Some(width))?; } self.consume_token(); } @@ -439,7 +439,7 @@ where // Safety: space_pos must be a character boundary w.get_unchecked(0..space_pos) }; - handler.printed_characters(word, handler.measure(word))?; + handler.printed_characters(word, None)?; } handler.whitespace("\u{a0}", 1, self.spaces.consume(1))?; @@ -450,9 +450,7 @@ where } } - None => { - handler.printed_characters(w, handler.measure(w))?; - } + None => handler.printed_characters(w, None)?, } Ok(()) @@ -526,9 +524,11 @@ pub(crate) mod test { Ok(()) } - fn printed_characters(&mut self, st: &str, width: u32) -> Result<(), Self::Error> { - self.elements - .push(RenderElement::String(st.to_owned(), width)); + fn printed_characters(&mut self, str: &str, width: Option) -> Result<(), Self::Error> { + self.elements.push(RenderElement::String( + str.to_owned(), + width.unwrap_or_else(|| self.measure(str)), + )); Ok(()) } diff --git a/src/style/mod.rs b/src/style/mod.rs index 9d3d11d2..4f67af69 100644 --- a/src/style/mod.rs +++ b/src/style/mod.rs @@ -398,8 +398,8 @@ impl<'a, S: TextRenderer> ElementHandler for MeasureLineElementHandler<'a, S> { Ok(()) } - fn printed_characters(&mut self, _: &str, width: u32) -> Result<(), Self::Error> { - self.cursor += width; + fn printed_characters(&mut self, str: &str, width: Option) -> Result<(), Self::Error> { + self.cursor += width.unwrap_or_else(|| self.measure(str)); self.pos = self.pos.max(self.cursor); self.right = self.pos; self.space_count = self.partial_space_count; From ee630abecd22e8c122b090ba6be2bf7e7a7ff496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 4 Sep 2023 09:13:32 +0200 Subject: [PATCH 4/5] line_iter: avoid unwrap --- src/parser/mod.rs | 1 - src/rendering/line_iter.rs | 14 +++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/parser/mod.rs b/src/parser/mod.rs index 24320ba3..622242c6 100644 --- a/src/parser/mod.rs +++ b/src/parser/mod.rs @@ -104,7 +104,6 @@ where /// Create a new parser object to process the given piece of text. #[inline] #[must_use] - pub fn parse(text: &'a str) -> Self { Self { inner: text.chars(), diff --git a/src/rendering/line_iter.rs b/src/rendering/line_iter.rs index 0edb0e7a..74d7bcc4 100644 --- a/src/rendering/line_iter.rs +++ b/src/rendering/line_iter.rs @@ -237,11 +237,15 @@ where let single = space_width / space_count; let consumed = moved as u32 / single; if consumed > 0 { - let (pos, _) = string.char_indices().nth(consumed as usize).unwrap(); - let consumed_str = unsafe { - // SAFETY: Pos is a valid index, we just got it - string.get_unchecked(0..pos) - }; + let consumed_str = string + .char_indices() + .nth(consumed as usize) + .map(|(pos, _)| unsafe { + // SAFETY: Pos is a valid index, we just got it + string.get_unchecked(0..pos) + }) + .unwrap_or(string); + let consumed_width = consumed * single; let _ = self.move_cursor(consumed_width.saturating_as()); From aacd6c40dd5e6301143f4692df9c15d5e8d9d729 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Mon, 4 Sep 2023 10:04:29 +0200 Subject: [PATCH 5/5] longest_fitting_substr: avoid char boundary codegen in release --- src/rendering/line_iter.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/rendering/line_iter.rs b/src/rendering/line_iter.rs index 74d7bcc4..eeecb8e1 100644 --- a/src/rendering/line_iter.rs +++ b/src/rendering/line_iter.rs @@ -3,6 +3,7 @@ //! Turns a token stream into a number of events. A single `LineElementParser` object operates on //! a single line and is responsible for handling word wrapping, eating leading/trailing whitespace, //! handling tab characters, soft wrapping characters, non-breaking spaces, etc. + use crate::{ parser::{ChangeTextStyle, Parser, Token, SPEC_CHAR_NBSP}, plugin::{PluginMarker as Plugin, PluginWrapper}, @@ -133,7 +134,7 @@ where &mut self, handler: &E, w: &'a str, - ) -> (&'a str, Option<&'a str>) { + ) -> (&'a str, &'a str) { let mut width = 0; for (idx, c) in w.char_indices() { let char_width = handler.measure(unsafe { @@ -141,19 +142,18 @@ where w.get_unchecked(idx..idx + c.len_utf8()) }); if !self.cursor.fits_in_line(width + char_width) { - debug_assert!(w.is_char_boundary(idx)); - return ( - unsafe { - // SAFETY: we are working on character boundaries - w.get_unchecked(0..idx) - }, - w.get(idx..), - ); + unsafe { + if w.is_char_boundary(idx) { + return w.split_at(idx); + } else { + core::hint::unreachable_unchecked(); + } + } } width += char_width; } - (w, None) + (w, "") } fn next_word_fits(&self, space_width: i32, handler: &E) -> bool { @@ -342,7 +342,7 @@ where let (word, remainder) = if self.move_cursor(width.saturating_as()).is_ok() { // We can move the cursor here since `process_word()` // doesn't depend on it. - (w, None) + (w, "") } else if self.empty { // This word does not fit into an empty line. Find longest part // that fits and push the rest to the next line. @@ -366,7 +366,7 @@ where self.process_word(handler, word)?; } - if remainder.is_some() { + if !remainder.is_empty() { // Consume what was printed. self.plugin.consume_partial(word.len()); return Ok(LineEndType::LineBreak);