Fix visual selection of trailing newline

Conrad Irwin created

Change summary

crates/editor/src/display_map.rs                      | 35 +++++++-----
crates/editor/src/element.rs                          | 19 ++++++
crates/vim/src/visual.rs                              | 35 ++++++++++++
crates/vim/test_data/test_enter_visual_line_mode.json |  5 +
4 files changed, 74 insertions(+), 20 deletions(-)

Detailed changes

crates/editor/src/display_map.rs 🔗

@@ -353,27 +353,30 @@ impl DisplaySnapshot {
         }
     }
 
+    // used by line_mode selections and tries to match vim behaviour
     pub fn expand_to_line(&self, range: Range<Point>) -> Range<Point> {
-        let mut new_start = self.prev_line_boundary(range.start).0;
-        if range.end.column == 0 {
-            return new_start..range.end;
-        }
-        let mut new_end = self.next_line_boundary(range.end).0;
-
-        if new_start.row == range.start.row && new_end.row == range.end.row {
-            if new_end.row < self.buffer_snapshot.max_point().row {
-                new_end.row += 1;
-                new_end.column = 0;
-            } else if new_start.row > 0 {
-                new_start.row -= 1;
-                new_start.column = self.buffer_snapshot.line_len(new_start.row);
-            }
-        }
+        let new_start = if range.start.row == 0 {
+            Point::new(0, 0)
+        } else if range.start.row == self.max_buffer_row()
+            || (range.end.column > 0 && range.end.row == self.max_buffer_row())
+        {
+            Point::new(range.start.row - 1, self.line_len(range.start.row - 1))
+        } else {
+            self.prev_line_boundary(range.start).0
+        };
+
+        let new_end = if range.end.column == 0 {
+            range.end
+        } else if range.end.row < self.max_buffer_row() {
+            Point::new(range.end.row + 1, 0)
+        } else {
+            self.buffer_snapshot.max_point()
+        };
 
         new_start..new_end
     }
 
-    fn point_to_display_point(&self, point: Point, bias: Bias) -> DisplayPoint {
+    pub fn point_to_display_point(&self, point: Point, bias: Bias) -> DisplayPoint {
         let inlay_point = self.inlay_snapshot.to_inlay_point(point);
         let fold_point = self.fold_snapshot.to_fold_point(inlay_point, bias);
         let tab_point = self.tab_snapshot.to_tab_point(fold_point);

crates/editor/src/element.rs 🔗

@@ -850,13 +850,16 @@ impl EditorElement {
                     let mut cursor_column = cursor_position.column() as usize;
                     let mut cursor_row = cursor_position.row();
 
+                    // highlight the last character in a selection
                     if CursorShape::Block == selection.cursor_shape
                         && !selection.range.is_empty()
                         && !selection.reversed
                     {
                         if cursor_column > 0 {
                             cursor_column -= 1;
-                        } else if cursor_row > 0 {
+                        } else if cursor_row > 0
+                            && cursor_position != layout.position_map.snapshot.max_point()
+                        {
                             cursor_row -= 1;
                             cursor_column =
                                 layout.position_map.snapshot.line_len(cursor_row) as usize;
@@ -2186,7 +2189,19 @@ impl Element<Editor> for EditorElement {
             for selection in &local_selections {
                 let is_empty = selection.start == selection.end;
                 let selection_start = snapshot.prev_line_boundary(selection.start).1;
-                let selection_end = snapshot.next_line_boundary(selection.end).1;
+                let mut selection_end = snapshot.next_line_boundary(selection.end).1;
+
+                // in vim visual mode the newline is considered at the end of the previous line
+                // instead of at the start of the current line
+                if editor.cursor_shape == CursorShape::Block
+                    && !is_empty
+                    && !selection.reversed
+                    && selection.end.column == 0
+                    && selection_end.row() > 0
+                    && selection_end.row() < snapshot.max_buffer_row()
+                {
+                    selection_end = DisplayPoint::new(selection_end.row() - 1, 0);
+                }
                 for row in cmp::max(selection_start.row(), start_row)
                     ..=cmp::min(selection_end.row(), end_row)
                 {

crates/vim/src/visual.rs 🔗

@@ -49,7 +49,14 @@ pub fn visual_motion(motion: Motion, times: Option<usize>, cx: &mut WindowContex
                     // 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 !selection.reversed {
+
+                    // 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())
+                    {
                         current_head = movement::left(map, selection.end)
                     }
 
@@ -60,7 +67,10 @@ pub fn visual_motion(motion: Motion, times: Option<usize>, cx: &mut WindowContex
 
                     // ensure the current character is included in the selection.
                     if !selection.reversed {
-                        selection.end = movement::right(map, selection.end)
+                        let next_point = movement::right(map, selection.end);
+                        if !(next_point.column() == 0 && next_point == map.max_point()) {
+                            selection.end = movement::right(map, selection.end)
+                        }
                     }
 
                     // vim always ensures the anchor character stays selected.
@@ -494,6 +504,27 @@ mod test {
             a
             ˇb"})
             .await;
+
+        // it should work at the end of the document
+        cx.set_shared_state(indoc! {"
+            a
+            b
+            ˇ"})
+            .await;
+        let cursor = cx.update_editor(|editor, _| editor.pixel_position_of_cursor());
+        cx.simulate_shared_keystrokes(["shift-v"]).await;
+        cx.assert_shared_state(indoc! {"
+            a
+            b
+            ˇ"})
+            .await;
+        assert_eq!(cx.mode(), cx.neovim_mode().await);
+        cx.update_editor(|editor, _| assert_eq!(cursor, editor.pixel_position_of_cursor()));
+        cx.simulate_shared_keystrokes(["x"]).await;
+        cx.assert_shared_state(indoc! {"
+            a
+            ˇb"})
+            .await;
     }
 
     #[gpui::test]

crates/vim/test_data/test_enter_visual_line_mode.json 🔗

@@ -8,3 +8,8 @@
 {"Get":{"state":"a\n«\nˇ»b","mode":{"Visual":{"line":true}}}}
 {"Key":"x"}
 {"Get":{"state":"a\nˇb","mode":"Normal"}}
+{"Put":{"state":"a\nb\nˇ"}}
+{"Key":"shift-v"}
+{"Get":{"state":"a\nb\nˇ","mode":{"Visual":{"line":true}}}}
+{"Key":"x"}
+{"Get":{"state":"a\nˇb","mode":"Normal"}}