Handle linewise motions correctly and fix panic when executing invalid actions

crates/vim/src/motion.rs

@@ -4,7 +4,7 @@ use editor::{
     movement, Bias, DisplayPoint,
 };
 use gpui::{actions, impl_actions, MutableAppContext};
-use language::{Selection, SelectionGoal};
+use language::SelectionGoal;
 use serde::Deserialize;
 use workspace::Workspace;
 
@@ -122,7 +122,9 @@ fn motion(motion: Motion, cx: &mut MutableAppContext) {
     });
     match Vim::read(cx).state.mode {
         Mode::Normal => normal_motion(motion, cx),
-        Mode::Insert => panic!("motion bindings in insert mode interfere with normal typing"),
+        Mode::Insert => {
+            // Shouldn't execute a motion in insert mode. Ignoring
+        }
     }
 }
 
@@ -132,6 +134,7 @@ impl Motion {
         map: &DisplaySnapshot,
         point: DisplayPoint,
         goal: SelectionGoal,
+        block_cursor_positioning: bool,
     ) -> (DisplayPoint, SelectionGoal) {
         use Motion::*;
         match self {
@@ -147,65 +150,25 @@ impl Motion {
                 SelectionGoal::None,
             ),
             NextWordEnd { ignore_punctuation } => (
-                next_word_end(map, point, ignore_punctuation, true),
+                next_word_end(map, point, ignore_punctuation, block_cursor_positioning),
                 SelectionGoal::None,
             ),
             PreviousWordStart { ignore_punctuation } => (
                 previous_word_start(map, point, ignore_punctuation),
                 SelectionGoal::None,
             ),
-            StartOfLine => (
+            StartOfLine => (start_of_line(map, point), SelectionGoal::None),
-                movement::line_beginning(map, point, false),
+            EndOfLine => (end_of_line(map, point), SelectionGoal::None),
-                SelectionGoal::None,
+            StartOfDocument => (start_of_document(map, point), SelectionGoal::None),
-            ),
+            EndOfDocument => (end_of_document(map, point), SelectionGoal::None),
-            EndOfLine => (
-                map.clip_point(movement::line_end(map, point, false), Bias::Left),
-                SelectionGoal::None,
-            ),
-            StartOfDocument => (start_of_document(map), SelectionGoal::None),
-            EndOfDocument => (end_of_document(map), SelectionGoal::None),
         }
     }
 
-    pub fn expand_selection(self, map: &DisplaySnapshot, selection: &mut Selection<DisplayPoint>) {
+    pub fn line_wise(self) -> bool {
         use Motion::*;
         match self {
-            Up => {
+            Down | Up | StartOfDocument | EndOfDocument => true,
-                let (start, _) = Up.move_point(map, selection.start, SelectionGoal::None);
+            _ => false,
-                // Cursor at top of file. Return early rather
-                if start == selection.start {
-                    return;
-                }
-                let (start, _) = StartOfLine.move_point(map, start, SelectionGoal::None);
-                let (end, _) = EndOfLine.move_point(map, selection.end, SelectionGoal::None);
-                selection.start = start;
-                selection.end = end;
-                // TODO: Make sure selection goal is correct here
-                selection.goal = SelectionGoal::None;
-            }
-            Down => {
-                let (end, _) = Down.move_point(map, selection.end, SelectionGoal::None);
-                // Cursor at top of file. Return early rather
-                if end == selection.start {
-                    return;
-                }
-                let (start, _) = StartOfLine.move_point(map, selection.start, SelectionGoal::None);
-                let (end, _) = EndOfLine.move_point(map, end, SelectionGoal::None);
-                selection.start = start;
-                selection.end = end;
-                // TODO: Make sure selection goal is correct here
-                selection.goal = SelectionGoal::None;
-            }
-            NextWordEnd { ignore_punctuation } => {
-                selection.set_head(
-                    next_word_end(map, selection.head(), ignore_punctuation, false),
-                    SelectionGoal::None,
-                );
-            }
-            _ => {
-                let (head, goal) = self.move_point(map, selection.head(), selection.goal);
-                selection.set_head(head, goal);
-            }
         }
     }
 }
@@ -287,10 +250,22 @@ fn previous_word_start(
     point
 }
 
-fn start_of_document(map: &DisplaySnapshot) -> DisplayPoint {
+fn start_of_line(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPoint {
-    0usize.to_display_point(map)
+    map.prev_line_boundary(point.to_point(map)).1
 }
 
-fn end_of_document(map: &DisplaySnapshot) -> DisplayPoint {
+fn end_of_line(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPoint {
-    map.clip_point(map.max_point(), Bias::Left)
+    map.clip_point(map.next_line_boundary(point.to_point(map)).1, Bias::Left)
+}
+
+fn start_of_document(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPoint {
+    let mut new_point = 0usize.to_display_point(map);
+    *new_point.column_mut() = point.column();
+    map.clip_point(new_point, Bias::Left)
+}
+
+fn end_of_document(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPoint {
+    let mut new_point = map.max_point();
+    *new_point.column_mut() = point.column();
+    map.clip_point(new_point, Bias::Left)
 }

crates/vim/src/normal.rs

@@ -13,8 +13,9 @@ pub fn normal_motion(motion: Motion, cx: &mut MutableAppContext) {
             None => move_cursor(vim, motion, cx),
             Some(Operator::Change) => change_over(vim, motion, cx),
             Some(Operator::Delete) => delete_over(vim, motion, cx),
-            Some(Operator::Namespace(_)) => panic!(
+            Some(Operator::Namespace(_)) => {
-                "Normal mode recieved motion with namespaced operator. Likely this means an invalid keymap was used"),
+                // Can't do anything for a namespace operator. Ignoring
+            }
         }
         vim.clear_operator(cx);
     });
@@ -22,7 +23,9 @@ pub fn normal_motion(motion: Motion, cx: &mut MutableAppContext) {
 
 fn move_cursor(vim: &mut Vim, motion: Motion, cx: &mut MutableAppContext) {
     vim.update_active_editor(cx, |editor, cx| {
-        editor.move_cursors(cx, |map, cursor, goal| motion.move_point(map, cursor, goal))
+        editor.move_cursors(cx, |map, cursor, goal| {
+            motion.move_point(map, cursor, goal, true)
+        })
     });
 }
 
@@ -31,7 +34,15 @@ fn change_over(vim: &mut Vim, motion: Motion, cx: &mut MutableAppContext) {
         editor.transact(cx, |editor, cx| {
             // Don't clip at line ends during change operation
             editor.set_clip_at_line_ends(false, cx);
-            editor.move_selections(cx, |map, selection| motion.expand_selection(map, selection));
+            editor.move_selections(cx, |map, selection| {
+                let (head, goal) = motion.move_point(map, selection.head(), selection.goal, false);
+                selection.set_head(head, goal);
+
+                if motion.line_wise() {
+                    selection.start = map.prev_line_boundary(selection.start.to_point(map)).1;
+                    selection.end = map.next_line_boundary(selection.end.to_point(map)).1;
+                }
+            });
             editor.set_clip_at_line_ends(true, cx);
             editor.insert(&"", cx);
         });
@@ -42,15 +53,50 @@ fn change_over(vim: &mut Vim, motion: Motion, cx: &mut MutableAppContext) {
 fn delete_over(vim: &mut Vim, motion: Motion, cx: &mut MutableAppContext) {
     vim.update_active_editor(cx, |editor, cx| {
         editor.transact(cx, |editor, cx| {
-            // Don't clip at line ends during delete operation
+            // Use goal column to preserve previous position
             editor.set_clip_at_line_ends(false, cx);
-            editor.move_selections(cx, |map, selection| motion.expand_selection(map, selection));
+            editor.move_selections(cx, |map, selection| {
+                let original_head = selection.head();
+                let (head, _) = motion.move_point(map, selection.head(), selection.goal, false);
+                // Set the goal column to the original position in order to fix it up
+                // after the deletion
+                selection.set_head(head, SelectionGoal::Column(original_head.column()));
+
+                if motion.line_wise() {
+                    if selection.end.row() == map.max_point().row() {
+                        // Delete previous line break since we are at the end of the document
+                        if selection.start.row() > 0 {
+                            *selection.start.row_mut() = selection.start.row().saturating_sub(1);
+                            selection.start = map.clip_point(selection.start, Bias::Left);
+                            selection.start =
+                                map.next_line_boundary(selection.start.to_point(map)).1;
+                        } else {
+                            // Selection covers the whole document. Just delete to the start of the
+                            // line.
+                            selection.start =
+                                map.prev_line_boundary(selection.start.to_point(map)).1;
+                        }
+                        selection.end = map.next_line_boundary(selection.end.to_point(map)).1;
+                    } else {
+                        // Delete next line break so that we leave the previous line alone
+                        selection.start = map.prev_line_boundary(selection.start.to_point(map)).1;
+                        *selection.end.column_mut() = 0;
+                        *selection.end.row_mut() += 1;
+                        selection.end = map.clip_point(selection.end, Bias::Left);
+                    }
+                }
+            });
             editor.insert(&"", cx);
 
             // Fixup cursor position after the deletion
             editor.set_clip_at_line_ends(true, cx);
-            editor.move_selection_heads(cx, |map, head, _| {
+            editor.move_cursors(cx, |map, mut cursor, goal| {
-                (map.clip_point(head, Bias::Left), SelectionGoal::None)
+                if motion.line_wise() {
+                    if let SelectionGoal::Column(column) = goal {
+                        *cursor.column_mut() = column
+                    }
+                }
+                (map.clip_point(cursor, Bias::Left), SelectionGoal::None)
             });
         });
     });
@@ -173,19 +219,22 @@ mod test {
 
     #[gpui::test]
     async fn test_jump_to_end(cx: &mut gpui::TestAppContext) {
-        let initial_content = indoc! {"
+        let mut cx = VimTestContext::new(cx, true, "").await;
-            The quick
+
+        cx.set_state(
+            indoc! {"
+            The |quick
             
             brown fox jumps
-            over the lazy dog"};
+            over the lazy dog"},
-        let mut cx = VimTestContext::new(cx, true, initial_content).await;
+            Mode::Normal,
-
+        );
         cx.simulate_keystroke("shift-G");
         cx.assert_editor_state(indoc! {"
             The quick
             
             brown fox jumps
-            over the lazy do|g"});
+            over| the lazy dog"});
 
         // Repeat the action doesn't move
         cx.simulate_keystroke("shift-G");
@@ -193,7 +242,7 @@ mod test {
             The quick
             
             brown fox jumps
-            over the lazy do|g"});
+            over| the lazy dog"});
     }
 
     #[gpui::test]
@@ -212,7 +261,7 @@ mod test {
         }
 
         // Reset and test ignoring punctuation
-        cx.simulate_keystrokes(["g", "g"]);
+        cx.simulate_keystrokes(["g", "g", "0"]);
         let (_, cursor_offsets) = marked_text(indoc! {"
             The |quick-brown
             |
@@ -242,7 +291,7 @@ mod test {
         }
 
         // Reset and test ignoring punctuation
-        cx.simulate_keystrokes(["g", "g"]);
+        cx.simulate_keystrokes(["g", "g", "0"]);
         let (_, cursor_offsets) = marked_text(indoc! {"
             Th|e quick-brow|n
             
@@ -264,7 +313,7 @@ mod test {
             |fox_jumps |over
             |the"});
         let mut cx = VimTestContext::new(cx, true, &initial_content).await;
-        cx.simulate_keystroke("shift-G");
+        cx.simulate_keystrokes(["shift-G", "shift-$"]);
 
         for cursor_offset in cursor_offsets.into_iter().rev() {
             cx.simulate_keystroke("b");
@@ -272,7 +321,7 @@ mod test {
         }
 
         // Reset and test ignoring punctuation
-        cx.simulate_keystroke("shift-G");
+        cx.simulate_keystrokes(["shift-G", "shift-$"]);
         let (_, cursor_offsets) = marked_text(indoc! {"
             ||The |quick-brown
             |
@@ -303,12 +352,16 @@ mod test {
 
     #[gpui::test]
     async fn test_move_to_start(cx: &mut gpui::TestAppContext) {
-        let initial_content = indoc! {"
+        let mut cx = VimTestContext::new(cx, true, "").await;
-            The quick
+
+        cx.set_state(
+            indoc! {"
+            The q|uick
             
             brown fox jumps
-            over the lazy dog"};
+            over the lazy dog"},
-        let mut cx = VimTestContext::new(cx, true, initial_content).await;
+            Mode::Normal,
+        );
 
         // Jump to the end to
         cx.simulate_keystroke("shift-G");
@@ -316,12 +369,12 @@ mod test {
             The quick
             
             brown fox jumps
-            over the lazy do|g"});
+            over |the lazy dog"});
 
         // Jump to the start
         cx.simulate_keystrokes(["g", "g"]);
         cx.assert_editor_state(indoc! {"
-            |The quick
+            The q|uick
             
             brown fox jumps
             over the lazy dog"});
@@ -331,7 +384,7 @@ mod test {
         // Repeat action doesn't change
         cx.simulate_keystrokes(["g", "g"]);
         cx.assert_editor_state(indoc! {"
-            |The quick
+            The q|uick
             
             brown fox jumps
             over the lazy dog"});
@@ -469,24 +522,6 @@ mod test {
             jumps over"},
             cx,
         );
-        assert(
-            "k",
-            indoc! {"
-            The quick
-            brown |fox"},
-            indoc! {"
-            |"},
-            cx,
-        );
-        assert(
-            "j",
-            indoc! {"
-            The q|uick
-            brown fox"},
-            indoc! {"
-            |"},
-            cx,
-        );
         assert(
             "shift-$",
             indoc! {"
@@ -508,4 +543,264 @@ mod test {
             cx,
         );
     }
+
+    #[gpui::test]
+    async fn test_linewise_delete(cx: &mut gpui::TestAppContext) {
+        fn assert(motion: &str, initial_state: &str, state_after: &str, cx: &mut VimTestContext) {
+            cx.assert_binding(
+                ["d", motion],
+                initial_state,
+                Mode::Normal,
+                state_after,
+                Mode::Normal,
+            );
+        }
+        let cx = &mut VimTestContext::new(cx, true, "").await;
+        assert(
+            "k",
+            indoc! {"
+            The quick
+            brown |fox
+            jumps over"},
+            indoc! {"
+            jumps |over"},
+            cx,
+        );
+        assert(
+            "k",
+            indoc! {"
+            The quick
+            brown fox
+            jumps |over"},
+            indoc! {"
+            The qu|ick"},
+            cx,
+        );
+        assert(
+            "j",
+            indoc! {"
+            The q|uick
+            brown fox
+            jumps over"},
+            indoc! {"
+            jumps| over"},
+            cx,
+        );
+        assert(
+            "j",
+            indoc! {"
+            The quick
+            brown| fox
+            jumps over"},
+            indoc! {"
+            The q|uick"},
+            cx,
+        );
+        assert(
+            "j",
+            indoc! {"
+            The quick
+            brown| fox
+            jumps over"},
+            indoc! {"
+            The q|uick"},
+            cx,
+        );
+        cx.assert_binding(
+            ["d", "g", "g"],
+            indoc! {"
+            The quick
+            brown| fox
+            jumps over
+            the lazy"},
+            Mode::Normal,
+            indoc! {"
+            jumps| over
+            the lazy"},
+            Mode::Normal,
+        );
+        cx.assert_binding(
+            ["d", "g", "g"],
+            indoc! {"
+            The quick
+            brown fox
+            jumps over
+            the l|azy"},
+            Mode::Normal,
+            "|",
+            Mode::Normal,
+        );
+        assert(
+            "shift-G",
+            indoc! {"
+            The quick
+            brown| fox
+            jumps over
+            the lazy"},
+            indoc! {"
+            The q|uick"},
+            cx,
+        );
+        cx.assert_binding(
+            ["d", "g", "g"],
+            indoc! {"
+            The q|uick
+            brown fox
+            jumps over
+            the lazy"},
+            Mode::Normal,
+            indoc! {"
+            brown| fox
+            jumps over
+            the lazy"},
+            Mode::Normal,
+        );
+    }
+
+    #[gpui::test]
+    async fn test_linewise_change(cx: &mut gpui::TestAppContext) {
+        fn assert(motion: &str, initial_state: &str, state_after: &str, cx: &mut VimTestContext) {
+            cx.assert_binding(
+                ["c", motion],
+                initial_state,
+                Mode::Normal,
+                state_after,
+                Mode::Insert,
+            );
+        }
+        let cx = &mut VimTestContext::new(cx, true, "").await;
+        assert(
+            "k",
+            indoc! {"
+            The quick
+            brown |fox
+            jumps over"},
+            indoc! {"
+            |
+            jumps over"},
+            cx,
+        );
+        assert(
+            "k",
+            indoc! {"
+            The quick
+            brown fox
+            jumps |over"},
+            indoc! {"
+            The quick
+            |"},
+            cx,
+        );
+        assert(
+            "j",
+            indoc! {"
+            The q|uick
+            brown fox
+            jumps over"},
+            indoc! {"
+            |
+            jumps over"},
+            cx,
+        );
+        assert(
+            "j",
+            indoc! {"
+            The quick
+            brown| fox
+            jumps over"},
+            indoc! {"
+            The quick
+            |"},
+            cx,
+        );
+        assert(
+            "j",
+            indoc! {"
+            The quick
+            brown| fox
+            jumps over"},
+            indoc! {"
+            The quick
+            |"},
+            cx,
+        );
+        assert(
+            "shift-G",
+            indoc! {"
+            The quick
+            brown| fox
+            jumps over
+            the lazy"},
+            indoc! {"
+            The quick
+            |"},
+            cx,
+        );
+        assert(
+            "shift-G",
+            indoc! {"
+            The quick
+            brown| fox
+            jumps over
+            the lazy"},
+            indoc! {"
+            The quick
+            |"},
+            cx,
+        );
+        assert(
+            "shift-G",
+            indoc! {"
+            The quick
+            brown fox
+            jumps over
+            the l|azy"},
+            indoc! {"
+            The quick
+            brown fox
+            jumps over
+            |"},
+            cx,
+        );
+        cx.assert_binding(
+            ["c", "g", "g"],
+            indoc! {"
+            The quick
+            brown| fox
+            jumps over
+            the lazy"},
+            Mode::Normal,
+            indoc! {"
+            |
+            jumps over
+            the lazy"},
+            Mode::Insert,
+        );
+        cx.assert_binding(
+            ["c", "g", "g"],
+            indoc! {"
+            The quick
+            brown fox
+            jumps over
+            the l|azy"},
+            Mode::Normal,
+            "|",
+            Mode::Insert,
+        );
+        cx.assert_binding(
+            ["c", "g", "g"],
+            indoc! {"
+            The q|uick
+            brown fox
+            jumps over
+            the lazy"},
+            Mode::Normal,
+            indoc! {"
+            |
+            brown fox
+            jumps over
+            the lazy"},
+            Mode::Insert,
+        );
+    }
 }