vim: Fix 1G (#2640)

Max Brunsfeld created 2 years ago

Before this change code could not distinguish between a user providing a
count of 1 and no count at all.

Fixes: zed-industries/community#710

Release Notes:

- 1G now correctly goes to the first line in vim mode
([710](zed-industries/community#710))

Change summary

crates/vim/src/motion.rs        | 27 ++++++++++++++++++---------
crates/vim/src/normal.rs        | 12 ++++++------
crates/vim/src/normal/change.rs |  9 +++++----
crates/vim/src/normal/delete.rs |  2 +-
crates/vim/src/normal/yank.rs   |  2 +-
crates/vim/src/test.rs          | 14 ++++++++++++++
crates/vim/src/vim.rs           |  7 +++----
crates/vim/src/visual.rs        |  2 +-
8 files changed, 49 insertions(+), 26 deletions(-)

Detailed changes

crates/vim/src/motion.rs

@@ -209,8 +209,9 @@ impl Motion {
         map: &DisplaySnapshot,
         point: DisplayPoint,
         goal: SelectionGoal,
-        times: usize,
+        maybe_times: Option<usize>,
     ) -> Option<(DisplayPoint, SelectionGoal)> {
+        let times = maybe_times.unwrap_or(1);
         use Motion::*;
         let infallible = self.infallible();
         let (new_point, goal) = match self {
@@ -236,7 +237,10 @@ impl Motion {
             EndOfLine => (end_of_line(map, point), SelectionGoal::None),
             CurrentLine => (end_of_line(map, point), SelectionGoal::None),
             StartOfDocument => (start_of_document(map, point, times), SelectionGoal::None),
-            EndOfDocument => (end_of_document(map, point, times), SelectionGoal::None),
+            EndOfDocument => (
+                end_of_document(map, point, maybe_times),
+                SelectionGoal::None,
+            ),
             Matching => (matching(map, point), SelectionGoal::None),
             FindForward { before, text } => (
                 find_forward(map, point, *before, text.clone(), times),
@@ -257,7 +261,7 @@ impl Motion {
         &self,
         map: &DisplaySnapshot,
         selection: &mut Selection<DisplayPoint>,
-        times: usize,
+        times: Option<usize>,
         expand_to_surrounding_newline: bool,
     ) -> bool {
         if let Some((new_head, goal)) =
@@ -473,14 +477,19 @@ fn start_of_document(map: &DisplaySnapshot, point: DisplayPoint, line: usize) ->
     map.clip_point(new_point, Bias::Left)
 }
 
-fn end_of_document(map: &DisplaySnapshot, point: DisplayPoint, line: usize) -> DisplayPoint {
-    let mut new_point = if line == 1 {
-        map.max_point()
+fn end_of_document(
+    map: &DisplaySnapshot,
+    point: DisplayPoint,
+    line: Option<usize>,
+) -> DisplayPoint {
+    let new_row = if let Some(line) = line {
+        (line - 1) as u32
     } else {
-        Point::new((line - 1) as u32, 0).to_display_point(map)
+        map.max_buffer_row()
     };
-    *new_point.column_mut() = point.column();
-    map.clip_point(new_point, Bias::Left)
+
+    let new_point = Point::new(new_row, point.column());
+    map.clip_point(new_point.to_display_point(map), Bias::Left)
 }
 
 fn matching(map: &DisplaySnapshot, display_point: DisplayPoint) -> DisplayPoint {

crates/vim/src/normal.rs

@@ -93,7 +93,7 @@ pub fn init(cx: &mut AppContext) {
 pub fn normal_motion(
     motion: Motion,
     operator: Option<Operator>,
-    times: usize,
+    times: Option<usize>,
     cx: &mut WindowContext,
 ) {
     Vim::update(cx, |vim, cx| {
@@ -129,7 +129,7 @@ pub fn normal_object(object: Object, cx: &mut WindowContext) {
     })
 }
 
-fn move_cursor(vim: &mut Vim, motion: Motion, times: usize, cx: &mut WindowContext) {
+fn move_cursor(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &mut WindowContext) {
     vim.update_active_editor(cx, |editor, cx| {
         editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
             s.move_cursors_with(|map, cursor, goal| {
@@ -147,7 +147,7 @@ fn insert_after(_: &mut Workspace, _: &InsertAfter, cx: &mut ViewContext<Workspa
         vim.update_active_editor(cx, |editor, cx| {
             editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                 s.maybe_move_cursors_with(|map, cursor, goal| {
-                    Motion::Right.move_point(map, cursor, goal, 1)
+                    Motion::Right.move_point(map, cursor, goal, None)
                 });
             });
         });
@@ -164,7 +164,7 @@ fn insert_first_non_whitespace(
         vim.update_active_editor(cx, |editor, cx| {
             editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                 s.maybe_move_cursors_with(|map, cursor, goal| {
-                    Motion::FirstNonWhitespace.move_point(map, cursor, goal, 1)
+                    Motion::FirstNonWhitespace.move_point(map, cursor, goal, None)
                 });
             });
         });
@@ -177,7 +177,7 @@ fn insert_end_of_line(_: &mut Workspace, _: &InsertEndOfLine, cx: &mut ViewConte
         vim.update_active_editor(cx, |editor, cx| {
             editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                 s.maybe_move_cursors_with(|map, cursor, goal| {
-                    Motion::EndOfLine.move_point(map, cursor, goal, 1)
+                    Motion::EndOfLine.move_point(map, cursor, goal, None)
                 });
             });
         });
@@ -237,7 +237,7 @@ fn insert_line_below(_: &mut Workspace, _: &InsertLineBelow, cx: &mut ViewContex
                 });
                 editor.change_selections(Some(Autoscroll::fit()), cx, |s| {
                     s.maybe_move_cursors_with(|map, cursor, goal| {
-                        Motion::EndOfLine.move_point(map, cursor, goal, 1)
+                        Motion::EndOfLine.move_point(map, cursor, goal, None)
                     });
                 });
                 editor.edit_with_autoindent(edits, cx);

crates/vim/src/normal/change.rs

@@ -6,7 +6,7 @@ use editor::{
 use gpui::WindowContext;
 use language::Selection;
 
-pub fn change_motion(vim: &mut Vim, motion: Motion, times: usize, cx: &mut WindowContext) {
+pub fn change_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &mut WindowContext) {
     // Some motions ignore failure when switching to normal mode
     let mut motion_succeeded = matches!(
         motion,
@@ -78,10 +78,10 @@ pub fn change_object(vim: &mut Vim, object: Object, around: bool, cx: &mut Windo
 fn expand_changed_word_selection(
     map: &DisplaySnapshot,
     selection: &mut Selection<DisplayPoint>,
-    times: usize,
+    times: Option<usize>,
     ignore_punctuation: bool,
 ) -> bool {
-    if times == 1 {
+    if times.is_none() || times.unwrap() == 1 {
         let in_word = map
             .chars_at(selection.head())
             .next()
@@ -97,7 +97,8 @@ fn expand_changed_word_selection(
             });
             true
         } else {
-            Motion::NextWordStart { ignore_punctuation }.expand_selection(map, selection, 1, false)
+            Motion::NextWordStart { ignore_punctuation }
+                .expand_selection(map, selection, None, false)
         }
     } else {
         Motion::NextWordStart { ignore_punctuation }.expand_selection(map, selection, times, false)

crates/vim/src/normal/delete.rs

@@ -3,7 +3,7 @@ use collections::{HashMap, HashSet};
 use editor::{display_map::ToDisplayPoint, scroll::autoscroll::Autoscroll, Bias};
 use gpui::WindowContext;
 
-pub fn delete_motion(vim: &mut Vim, motion: Motion, times: usize, cx: &mut WindowContext) {
+pub fn delete_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &mut WindowContext) {
     vim.update_active_editor(cx, |editor, cx| {
         editor.transact(cx, |editor, cx| {
             editor.set_clip_at_line_ends(false, cx);

crates/vim/src/normal/yank.rs

@@ -2,7 +2,7 @@ use crate::{motion::Motion, object::Object, utils::copy_selections_content, Vim}
 use collections::HashMap;
 use gpui::WindowContext;
 
-pub fn yank_motion(vim: &mut Vim, motion: Motion, times: usize, cx: &mut WindowContext) {
+pub fn yank_motion(vim: &mut Vim, motion: Motion, times: Option<usize>, cx: &mut WindowContext) {
     vim.update_active_editor(cx, |editor, cx| {
         editor.transact(cx, |editor, cx| {
             editor.set_clip_at_line_ends(false, cx);

crates/vim/src/test.rs

@@ -109,3 +109,17 @@ async fn test_count_down(cx: &mut gpui::TestAppContext) {
     cx.simulate_keystrokes(["9", "down"]);
     cx.assert_editor_state("aa\nbb\ncc\ndd\neˇe");
 }
+
+#[gpui::test]
+async fn test_end_of_document_710(cx: &mut gpui::TestAppContext) {
+    let mut cx = VimTestContext::new(cx, true).await;
+
+    // goes to end by default
+    cx.set_state(indoc! {"aˇa\nbb\ncc"}, Mode::Normal);
+    cx.simulate_keystrokes(["shift-g"]);
+    cx.assert_editor_state("aa\nbb\ncˇc");
+
+    // can go to line 1 (https://github.com/zed-industries/community/issues/710)
+    cx.simulate_keystrokes(["1", "shift-g"]);
+    cx.assert_editor_state("aˇa\nbb\ncc");
+}

crates/vim/src/vim.rs

@@ -238,13 +238,12 @@ impl Vim {
         popped_operator
     }
 
-    fn pop_number_operator(&mut self, cx: &mut WindowContext) -> usize {
-        let mut times = 1;
+    fn pop_number_operator(&mut self, cx: &mut WindowContext) -> Option<usize> {
         if let Some(Operator::Number(number)) = self.active_operator() {
-            times = number;
             self.pop_operator(cx);
+            return Some(number);
         }
-        times
+        None
     }
 
     fn clear_operator(&mut self, cx: &mut WindowContext) {

crates/vim/src/visual.rs

@@ -25,7 +25,7 @@ pub fn init(cx: &mut AppContext) {
     cx.add_action(paste);
 }
 
-pub fn visual_motion(motion: Motion, times: usize, cx: &mut WindowContext) {
+pub fn visual_motion(motion: Motion, times: Option<usize>, cx: &mut WindowContext) {
     Vim::update(cx, |vim, cx| {
         vim.update_active_editor(cx, |editor, cx| {
             editor.change_selections(Some(Autoscroll::fit()), cx, |s| {