From 8ff027710352a9090e99a7da3eecc0df3f4fe054 Mon Sep 17 00:00:00 2001 From: Keith Simmons Date: Tue, 19 Apr 2022 11:36:44 -0700 Subject: [PATCH] Handle linewise motions correctly and fix panic when executing invalid actions --- crates/vim/src/motion.rs | 83 +++------ crates/vim/src/normal.rs | 383 ++++++++++++++++++++++++++++++++++----- 2 files changed, 368 insertions(+), 98 deletions(-) diff --git a/crates/vim/src/motion.rs b/crates/vim/src/motion.rs index beea48ef88..530ee1d657 100644 --- a/crates/vim/src/motion.rs +++ b/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 => ( - movement::line_beginning(map, point, false), - 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), + StartOfLine => (start_of_line(map, point), SelectionGoal::None), + EndOfLine => (end_of_line(map, point), SelectionGoal::None), + StartOfDocument => (start_of_document(map, point), SelectionGoal::None), + EndOfDocument => (end_of_document(map, point), SelectionGoal::None), } } - pub fn expand_selection(self, map: &DisplaySnapshot, selection: &mut Selection) { + pub fn line_wise(self) -> bool { use Motion::*; match self { - Up => { - let (start, _) = Up.move_point(map, selection.start, SelectionGoal::None); - // 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); - } + Down | Up | StartOfDocument | EndOfDocument => true, + _ => false, } } } @@ -287,10 +250,22 @@ fn previous_word_start( point } -fn start_of_document(map: &DisplaySnapshot) -> DisplayPoint { - 0usize.to_display_point(map) +fn start_of_line(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPoint { + map.prev_line_boundary(point.to_point(map)).1 } -fn end_of_document(map: &DisplaySnapshot) -> DisplayPoint { - map.clip_point(map.max_point(), Bias::Left) +fn end_of_line(map: &DisplaySnapshot, point: DisplayPoint) -> DisplayPoint { + 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) } diff --git a/crates/vim/src/normal.rs b/crates/vim/src/normal.rs index e8b17e9b89..d0ca7ae870 100644 --- a/crates/vim/src/normal.rs +++ b/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!( - "Normal mode recieved motion with namespaced operator. Likely this means an invalid keymap was used"), + Some(Operator::Namespace(_)) => { + // 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, _| { - (map.clip_point(head, Bias::Left), SelectionGoal::None) + editor.move_cursors(cx, |map, mut cursor, goal| { + 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! {" - The quick + let mut cx = VimTestContext::new(cx, true, "").await; + + cx.set_state( + indoc! {" + The |quick brown fox jumps - over the lazy dog"}; - let mut cx = VimTestContext::new(cx, true, initial_content).await; - + over the lazy dog"}, + 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! {" - The quick + let mut cx = VimTestContext::new(cx, true, "").await; + + cx.set_state( + indoc! {" + The q|uick brown fox jumps - over the lazy dog"}; - let mut cx = VimTestContext::new(cx, true, initial_content).await; + over the lazy dog"}, + 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, + ); + } }