correctly wrap at text-width

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 @@ pub fn is_virtual(self) -> bool {
         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 @@ fn is_whitespace(&self) -> bool {
         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<str>,
     pub wrap_indicator_highlight: Option<Highlight>,
     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 @@ fn default() -> Self {
             wrap_indicator: Box::from(" "),
             viewport_width: 17,
             wrap_indicator_highlight: None,
+            soft_wrap_at_text_width: false,
         }
     }
 }
@@ -318,39 +334,68 @@ fn wrap_word(&mut self) -> usize {
                 .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<GraphemeWithSource<'t>> {
+        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 @@ fn advance_to_next_word(&mut self) {
     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

helix-core/src/doc_formatter/test.rs

@@ -12,6 +12,7 @@ fn new_test(softwrap: bool) -> Self {
             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 @@ fn collect_to_str(&mut self) -> String {
         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 @@ fn collect_to_str(&mut self) -> String {
                 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(),

helix-view/src/document.rs

@@ -1953,7 +1953,7 @@ pub fn text_format(&self, mut viewport_width: u16, theme: Option<&Theme>) -> Tex
             .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 @@ pub fn text_format(&self, mut viewport_width: u16, theme: Option<&Theme>) -> Tex
             .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 @@ pub fn text_format(&self, mut viewport_width: u16, theme: Option<&Theme>) -> Tex
             wrap_indicator_highlight: theme
                 .and_then(|theme| theme.find_scope_index("ui.virtual.wrap"))
                 .map(Highlight),
+            soft_wrap_at_text_width,
         }
     }