Fix Vim 'e' Behavior When Boundary Is Last Point on Line (#7424)

Andrew Marek created 

This was originally just to fix
https://github.com/zed-industries/zed/issues/4354, which I did by just
returning the previous offset in `find_boundary`.. but `find_boundary`
is used in the "insert mode" / normal editor too, so returning the
previous boundary breaks existing functionality in that case.

I was considering a new `find_boundary` function just for some of the
vim motions like this, but I thought that this is straightforward enough
and future Vim functions might need similar logic too.

Release Notes:

- Fixed https://github.com/zed-industries/zed/issues/4354

Change summary

crates/editor/src/movement.rs                                  | 35 +++
crates/vim/src/motion.rs                                       | 30 +-
crates/vim/test_data/test_next_word_end_newline_last_char.json |  3 
3 files changed, 48 insertions(+), 20 deletions(-)

Detailed changes

crates/editor/src/movement.rs 🔗 

@@ -395,14 +395,17 @@ pub fn find_preceding_boundary(
 /// Scans for a boundary following the given start point until a boundary is found, indicated by the
 /// given predicate returning true. The predicate is called with the character to the left and right
 /// of the candidate boundary location, and will be called with `\n` characters indicating the start
-/// or end of a line.
-pub fn find_boundary(
+/// or end of a line. The function supports optionally returning the point just before the boundary
+/// is found via return_point_before_boundary.
+pub fn find_boundary_point(
     map: &DisplaySnapshot,
     from: DisplayPoint,
     find_range: FindRange,
     mut is_boundary: impl FnMut(char, char) -> bool,
+    return_point_before_boundary: bool,
 ) -> DisplayPoint {
     let mut offset = from.to_offset(&map, Bias::Right);
+    let mut prev_offset = offset;
     let mut prev_ch = None;
 
     for ch in map.buffer_snapshot.chars_at(offset) {
@@ -411,16 +414,38 @@ pub fn find_boundary(
         }
         if let Some(prev_ch) = prev_ch {
             if is_boundary(prev_ch, ch) {
-                break;
+                if return_point_before_boundary {
+                    return map.clip_point(prev_offset.to_display_point(map), Bias::Right);
+                } else {
+                    break;
+                }
             }
         }
-
+        prev_offset = offset;
         offset += ch.len_utf8();
         prev_ch = Some(ch);
     }
     map.clip_point(offset.to_display_point(map), Bias::Right)
 }
 
+pub fn find_boundary(
+    map: &DisplaySnapshot,
+    from: DisplayPoint,
+    find_range: FindRange,
+    is_boundary: impl FnMut(char, char) -> bool,
+) -> DisplayPoint {
+    return find_boundary_point(map, from, find_range, is_boundary, false);
+}
+
+pub fn find_boundary_exclusive(
+    map: &DisplaySnapshot,
+    from: DisplayPoint,
+    find_range: FindRange,
+    is_boundary: impl FnMut(char, char) -> bool,
+) -> DisplayPoint {
+    return find_boundary_point(map, from, find_range, is_boundary, true);
+}
+
 /// Returns an iterator over the characters following a given offset in the [`DisplaySnapshot`].
 /// The returned value also contains a range of the start/end of a returned character in
 /// the [`DisplaySnapshot`]. The offsets are relative to the start of a buffer.
@@ -763,7 +788,7 @@ mod tests {
                     &snapshot,
                     display_points[0],
                     FindRange::MultiLine,
-                    is_boundary
+                    is_boundary,
                 ),
                 display_points[1]
             );

crates/vim/src/motion.rs 🔗 

@@ -798,23 +798,14 @@ fn next_word_end(
             *point.row_mut() += 1;
             *point.column_mut() = 0;
         }
-        point = movement::find_boundary(map, point, FindRange::MultiLine, |left, right| {
-            let left_kind = coerce_punctuation(char_kind(&scope, left), ignore_punctuation);
-            let right_kind = coerce_punctuation(char_kind(&scope, right), ignore_punctuation);
 
-            left_kind != right_kind && left_kind != CharKind::Whitespace
-        });
+        point =
+            movement::find_boundary_exclusive(map, point, FindRange::MultiLine, |left, right| {
+                let left_kind = coerce_punctuation(char_kind(&scope, left), ignore_punctuation);
+                let right_kind = coerce_punctuation(char_kind(&scope, right), ignore_punctuation);
 
-        // find_boundary clips, so if the character after the next character is a newline or at the end of the document, we know
-        // we have backtracked already
-        if !map
-            .chars_at(point)
-            .nth(1)
-            .map(|(c, _)| c == '\n')
-            .unwrap_or(true)
-        {
-            *point.column_mut() = point.column().saturating_sub(1);
-        }
+                left_kind != right_kind && left_kind != CharKind::Whitespace
+            });
         point = map.clip_point(point, Bias::Left);
     }
     point
@@ -1285,6 +1276,15 @@ mod test {
         cx.assert_shared_state("one two thˇree four").await;
     }
 
+    #[gpui::test]
+    async fn test_next_word_end_newline_last_char(cx: &mut gpui::TestAppContext) {
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+        let initial_state = indoc! {r"something(ˇfoo)"};
+        cx.set_shared_state(initial_state).await;
+        cx.simulate_shared_keystrokes(["}"]).await;
+        cx.assert_shared_state(indoc! {r"something(fooˇ)"}).await;
+    }
+
     #[gpui::test]
     async fn test_next_line_start(cx: &mut gpui::TestAppContext) {
         let mut cx = NeovimBackedTestContext::new(cx).await;