vim: Fix selection motions when trailing newline is selected

crates/editor/src/element.rs

@@ -202,7 +202,7 @@ impl SelectionLayout {
         if cursor_offset && !range.is_empty() && !selection.reversed {
             if head.column() > 0 {
                 head = map.clip_point(DisplayPoint::new(head.row(), head.column() - 1), Bias::Left);
-            } else if head.row().0 > 0 && head != map.max_point() {
+            } else if head.row().0 > 0 {
                 head = map.clip_point(
                     DisplayPoint::new(
                         head.row().previous_row(),
@@ -11528,20 +11528,20 @@ mod tests {
             DisplayPoint::new(DisplayRow(3), 2)
         );
 
-        // leaves cursor on the max point
+        // displays the trailing newline cursor on the preceding row
         assert_eq!(
             local_selections[2].range,
             DisplayPoint::new(DisplayRow(5), 6)..DisplayPoint::new(DisplayRow(6), 0)
         );
         assert_eq!(
             local_selections[2].head,
-            DisplayPoint::new(DisplayRow(6), 0)
+            DisplayPoint::new(DisplayRow(5), 6)
         );
 
         // active lines does not include 1 (even though the range of the selection does)
         assert_eq!(
             state.active_rows.keys().cloned().collect::<Vec<_>>(),
-            vec![DisplayRow(0), DisplayRow(3), DisplayRow(5), DisplayRow(6)]
+            vec![DisplayRow(0), DisplayRow(3), DisplayRow(5)]
         );
     }
 

crates/vim/src/helix.rs

@@ -169,14 +169,7 @@ impl Vim {
                     // our motions assume the current character is after the cursor,
                     // but in (forward) visual mode the current character is just
                     // before the end of the selection.
-
-                    // If the file ends with a newline (which is common) we don't do this.
-                    // so that if you go to the end of such a file you can use "up" to go
-                    // to the previous line and have it work somewhat as expected.
-                    if !selection.reversed
-                        && !selection.is_empty()
-                        && !(selection.end.column() == 0 && selection.end == map.max_point())
-                    {
+                    if !selection.reversed && !selection.is_empty() {
                         current_head = movement::left(map, selection.end)
                     }
 
@@ -241,9 +234,7 @@ impl Vim {
                     if !selection.reversed {
                         let next_point = movement::right(map, selection.end);
 
-                        if !(next_point.column() == 0 && next_point == map.max_point()) {
-                            selection.end = next_point;
-                        }
+                        selection.end = next_point;
                     }
 
                     // vim always ensures the anchor character stays selected.
@@ -2669,6 +2660,30 @@ mod test {
         cx.assert_state("abˇcd", Mode::Insert);
     }
 
+    #[gpui::test]
+    async fn test_helix_select_move_to_trailing_newline(cx: &mut gpui::TestAppContext) {
+        let mut cx = VimTestContext::new(cx, true).await;
+        cx.enable_helix();
+
+        cx.set_state("line one\nline two\nline threˇe\n", Mode::HelixNormal);
+        cx.simulate_keystrokes("v");
+        cx.assert_state("line one\nline two\nline thre«eˇ»\n", Mode::HelixSelect);
+        cx.simulate_keystrokes("l");
+        cx.assert_state("line one\nline two\nline thre«e\nˇ»", Mode::HelixSelect);
+    }
+
+    #[gpui::test]
+    async fn test_helix_select_trailing_newline(cx: &mut gpui::TestAppContext) {
+        let mut cx = VimTestContext::new(cx, true).await;
+        cx.enable_helix();
+
+        cx.set_state("line one\nline two\nline threeˇ\n", Mode::HelixNormal);
+        cx.simulate_keystrokes("v");
+        cx.assert_state("line one\nline two\nline three«\nˇ»", Mode::HelixSelect);
+        cx.simulate_keystrokes("g h");
+        cx.assert_state("line one\nline two\n«ˇline three\n»", Mode::HelixSelect);
+    }
+
     #[gpui::test]
     async fn test_goto_last_modification(cx: &mut gpui::TestAppContext) {
         let mut cx = VimTestContext::new(cx, true).await;

crates/vim/src/visual.rs

@@ -247,14 +247,7 @@ impl Vim {
                         // our motions assume the current character is after the cursor,
                         // but in (forward) visual mode the current character is just
                         // before the end of the selection.
-
-                        // If the file ends with a newline (which is common) we don't do this.
-                        // so that if you go to the end of such a file you can use "up" to go
-                        // to the previous line and have it work somewhat as expected.
-                        if !selection.reversed
-                            && !selection.is_empty()
-                            && !(selection.end.column() == 0 && selection.end == map.max_point())
-                        {
+                        if !selection.reversed && !selection.is_empty() {
                             current_head = movement::left(map, selection.end)
                         }
 
@@ -278,9 +271,7 @@ impl Vim {
                                 movement::right(map, selection.end)
                             };
 
-                            if !(next_point.column() == 0 && next_point == map.max_point()) {
-                                selection.end = next_point;
-                            }
+                            selection.end = next_point;
                         }
 
                         // vim always ensures the anchor character stays selected.
@@ -1748,6 +1739,17 @@ mod test {
         // });
     }
 
+    #[gpui::test]
+    async fn test_visual_move_trailing_newline(cx: &mut gpui::TestAppContext) {
+        let mut cx = VimTestContext::new(cx, true).await;
+
+        cx.set_state("This is a newlinˇe\n", Mode::Normal);
+        cx.simulate_keystrokes("v");
+        cx.assert_state("This is a newlin«eˇ»\n", Mode::Visual);
+        cx.simulate_keystrokes("l");
+        cx.assert_state("This is a newlin«e\nˇ»", Mode::Visual);
+    }
+
     #[gpui::test]
     async fn test_mode_across_command(cx: &mut gpui::TestAppContext) {
         let mut cx = VimTestContext::new(cx, true).await;