From 9a93240d27ac2cdd589526293d6901b2342c8576 Mon Sep 17 00:00:00 2001 From: Pascal Kuthe Date: Sun, 19 Nov 2023 22:34:06 +0100 Subject: [PATCH] correctly wrap at text-width --- helix-core/src/doc_formatter.rs | 86 +++++++++++++++++++++------- helix-core/src/doc_formatter/test.rs | 20 +++++++ helix-view/src/document.rs | 16 +++--- 3 files changed, 94 insertions(+), 28 deletions(-) diff --git a/helix-core/src/doc_formatter.rs b/helix-core/src/doc_formatter.rs index 0f9a52b5d..65e1532f0 100644 --- a/helix-core/src/doc_formatter.rs +++ b/helix-core/src/doc_formatter.rs @@ -10,6 +10,7 @@ //! called a "block" and the caller must advance it as needed. use std::borrow::Cow; +use std::cmp::Ordering; use std::fmt::Debug; use std::mem::replace; @@ -43,6 +44,11 @@ impl GraphemeSource { matches!(self, GraphemeSource::VirtualText { .. }) } + pub fn is_eof(self) -> bool { + // all doc chars except the EOF char have non-zero codepoints + matches!(self, GraphemeSource::Document { codepoints: 0 }) + } + pub fn doc_chars(self) -> usize { match self { GraphemeSource::Document { codepoints } => codepoints as usize, @@ -117,6 +123,14 @@ impl<'a> GraphemeWithSource<'a> { self.grapheme.is_whitespace() } + fn is_newline(&self) -> bool { + matches!(self.grapheme, Grapheme::Newline) + } + + fn is_eof(&self) -> bool { + self.source.is_eof() + } + fn width(&self) -> usize { self.grapheme.width() } @@ -135,6 +149,7 @@ pub struct TextFormat { pub wrap_indicator: Box, pub wrap_indicator_highlight: Option, pub viewport_width: u16, + pub soft_wrap_at_text_width: bool, } // test implementation is basically only used for testing or when softwrap is always disabled @@ -148,6 +163,7 @@ impl Default for TextFormat { wrap_indicator: Box::from(" "), viewport_width: 17, wrap_indicator_highlight: None, + soft_wrap_at_text_width: false, } } } @@ -318,39 +334,68 @@ impl<'t> DocumentFormatter<'t> { .change_position(visual_x, self.text_fmt.tab_width); word_width += grapheme.width(); } + if let Some(grapheme) = &mut self.peeked_grapheme { + let visual_x = self.visual_pos.col + word_width; + grapheme + .grapheme + .change_position(visual_x, self.text_fmt.tab_width); + } word_width } + fn peek_grapheme(&mut self, col: usize, char_pos: usize) -> Option<&GraphemeWithSource<'t>> { + if self.peeked_grapheme.is_none() { + self.peeked_grapheme = self.advance_grapheme(col, char_pos); + } + self.peeked_grapheme.as_ref() + } + + fn next_grapheme(&mut self, col: usize, char_pos: usize) -> Option> { + self.peek_grapheme(col, char_pos); + self.peeked_grapheme.take() + } + fn advance_to_next_word(&mut self) { self.word_buf.clear(); let mut word_width = 0; let mut word_chars = 0; + if self.exhausted { + return; + } + loop { - // softwrap word if necessary - if word_width + self.visual_pos.col >= self.text_fmt.viewport_width as usize { - // wrapping this word would move too much text to the next line - // split the word at the line end instead - if word_width > self.text_fmt.max_wrap as usize { - // Usually we stop accomulating graphemes as soon as softwrapping becomes necessary. - // However if the last grapheme is multiple columns wide it might extend beyond the EOL. - // The condition below ensures that this grapheme is not cutoff and instead wrapped to the next line - if word_width + self.visual_pos.col > self.text_fmt.viewport_width as usize { - self.peeked_grapheme = self.word_buf.pop(); - } + let mut col = self.visual_pos.col + word_width; + let char_pos = self.char_pos + word_chars; + match col.cmp(&(self.text_fmt.viewport_width as usize)) { + // The EOF char and newline chars are always selectable in helix. That means + // that wrapping happens "too-early" if a word fits a line perfectly. This + // is intentional so that all selectable graphemes are always visisble (and + // therefore the cursor never dissapears). However if the user manually set a + // lower softwrap width then this is undesirable. Just increasing the viewport- + // width by one doesn't work because if a line is wrapped multiple times then + // some words may extend past the specified width. + // + // So we special case a word that ends exactly at line bounds and is followed + // by a newline/eof character here. + Ordering::Equal + if self.text_fmt.soft_wrap_at_text_width + && self.peek_grapheme(col, char_pos).map_or(false, |grapheme| { + grapheme.is_newline() || grapheme.is_eof() + }) => {} + Ordering::Equal if word_width > self.text_fmt.max_wrap as usize => return, + Ordering::Greater if word_width > self.text_fmt.max_wrap as usize => { + self.peeked_grapheme = self.word_buf.pop(); return; } - - word_width = self.wrap_word(); + Ordering::Equal | Ordering::Greater => { + word_width = self.wrap_word(); + col = self.visual_pos.col + word_width; + } + Ordering::Less => (), } - let grapheme = if let Some(grapheme) = self.peeked_grapheme.take() { - grapheme - } else if let Some(grapheme) = - self.advance_grapheme(self.visual_pos.col + word_width, self.char_pos + word_chars) - { - grapheme - } else { + let Some(grapheme) = self.next_grapheme(col, char_pos) else { return; }; word_chars += grapheme.doc_chars(); @@ -376,7 +421,6 @@ impl<'t> DocumentFormatter<'t> { pub fn next_char_pos(&self) -> usize { self.char_pos } - /// returns the visual position at the end of the last yielded grapheme pub fn next_visual_pos(&self) -> Position { self.visual_pos diff --git a/helix-core/src/doc_formatter/test.rs b/helix-core/src/doc_formatter/test.rs index b214ab944..415ce8f6a 100644 --- a/helix-core/src/doc_formatter/test.rs +++ b/helix-core/src/doc_formatter/test.rs @@ -12,6 +12,7 @@ impl TextFormat { wrap_indicator_highlight: None, // use a prime number to allow lining up too often with repeat viewport_width: 17, + soft_wrap_at_text_width: false, } } } @@ -21,6 +22,7 @@ impl<'t> DocumentFormatter<'t> { use std::fmt::Write; let mut res = String::new(); let viewport_width = self.text_fmt.viewport_width; + let soft_wrap_at_text_width = self.text_fmt.soft_wrap_at_text_width; let mut line = 0; for grapheme in self { @@ -28,6 +30,8 @@ impl<'t> DocumentFormatter<'t> { line += 1; assert_eq!(grapheme.visual_pos.row, line); write!(res, "\n{}", ".".repeat(grapheme.visual_pos.col)).unwrap(); + } + if !soft_wrap_at_text_width { assert!( grapheme.visual_pos.col <= viewport_width as usize, "softwrapped failed {}<={viewport_width}", @@ -98,6 +102,22 @@ fn long_word_softwrap() { ); } +fn softwrap_text_at_text_width(text: &str) -> String { + let mut text_fmt = TextFormat::new_test(true); + text_fmt.soft_wrap_at_text_width = true; + let annotations = TextAnnotations::default(); + let mut formatter = + DocumentFormatter::new_at_prev_checkpoint(text.into(), &text_fmt, &annotations, 0); + formatter.collect_to_str() +} +#[test] +fn long_word_softwrap_text_width() { + assert_eq!( + softwrap_text_at_text_width("xxxxxxxx1xxxx2xxx\nxxxxxxxx1xxxx2xxx"), + "xxxxxxxx1xxxx2xxx \nxxxxxxxx1xxxx2xxx " + ); +} + fn overlay_text(text: &str, char_pos: usize, softwrap: bool, overlays: &[Overlay]) -> String { DocumentFormatter::new_at_prev_checkpoint( text.into(), diff --git a/helix-view/src/document.rs b/helix-view/src/document.rs index 3314a243f..412f79fa2 100644 --- a/helix-view/src/document.rs +++ b/helix-view/src/document.rs @@ -1953,7 +1953,7 @@ impl Document { .language_config() .and_then(|config| config.text_width) .unwrap_or(config.text_width); - let soft_wrap_at_text_width = self + let mut soft_wrap_at_text_width = self .language_config() .and_then(|config| { config @@ -1964,12 +1964,13 @@ impl Document { .or(config.soft_wrap.wrap_at_text_width) .unwrap_or(false); if soft_wrap_at_text_width { - // We increase max_line_len by 1 because softwrap considers the newline character - // as part of the line length while the "typical" expectation is that this is not the case. - // In particular other commands like :reflow do not count the line terminator. - // This is technically inconsistent for the last line as that line never has a line terminator - // but having the last visual line exceed the width by 1 seems like a rare edge case. - viewport_width = viewport_width.min(text_width as u16 + 1) + // if the viewport is smaller than the specified + // width then this setting has no effcet + if text_width >= viewport_width as usize { + soft_wrap_at_text_width = false; + } else { + viewport_width = text_width as u16; + } } let config = self.config.load(); let editor_soft_wrap = &config.soft_wrap; @@ -2006,6 +2007,7 @@ impl Document { wrap_indicator_highlight: theme .and_then(|theme| theme.find_scope_index("ui.virtual.wrap")) .map(Highlight), + soft_wrap_at_text_width, } }