vim: Fix issues with r/R (#13623)

Conrad Irwin created

Release Notes:

- vim: Fix undo after repeated insert/replace mode (#13573)
- vim: Fix 'r' repeating too much (#13566)

Change summary

crates/vim/src/normal.rs                            | 59 +++++------
crates/vim/src/normal/repeat.rs                     | 11 ++
crates/vim/src/vim.rs                               | 73 +++++++++-----
crates/vim/test_data/test_r.json                    |  7 +
crates/vim/test_data/test_undo_repeated_insert.json |  8 +
5 files changed, 97 insertions(+), 61 deletions(-)

Detailed changes

crates/vim/src/normal.rs 🔗

@@ -23,11 +23,11 @@ use crate::{
 };
 use case::{change_case_motion, change_case_object, CaseTarget};
 use collections::BTreeSet;
-use editor::display_map::ToDisplayPoint;
 use editor::scroll::Autoscroll;
 use editor::Anchor;
 use editor::Bias;
 use editor::Editor;
+use editor::{display_map::ToDisplayPoint, movement};
 use gpui::{actions, ViewContext, WindowContext};
 use language::{Point, SelectionGoal};
 use log::error;
@@ -490,45 +490,34 @@ pub(crate) fn normal_replace(text: Arc<str>, cx: &mut WindowContext) {
             editor.transact(cx, |editor, cx| {
                 editor.set_clip_at_line_ends(false, cx);
                 let (map, display_selections) = editor.selections.all_display(cx);
-                // Selections are biased right at the start. So we need to store
-                // anchors that are biased left so that we can restore the selections
-                // after the change
-                let stable_anchors = editor
-                    .selections
-                    .disjoint_anchors()
-                    .into_iter()
-                    .map(|selection| {
-                        let start = selection.start.bias_left(&map.buffer_snapshot);
-                        start..start
-                    })
-                    .collect::<Vec<_>>();
 
-                let edits = display_selections
-                    .into_iter()
-                    .map(|selection| {
-                        let mut range = selection.range();
-                        range.end = right(&map, range.end, count);
-                        let repeated_text = text.repeat(count);
-
-                        (
-                            range.start.to_offset(&map, Bias::Left)
-                                ..range.end.to_offset(&map, Bias::Left),
-                            repeated_text,
-                        )
-                    })
-                    .collect::<Vec<_>>();
+                let mut edits = Vec::new();
+                for selection in display_selections {
+                    let mut range = selection.range();
+                    for _ in 0..count {
+                        let new_point = movement::saturating_right(&map, range.end);
+                        if range.end == new_point {
+                            return;
+                        }
+                        range.end = new_point;
+                    }
+
+                    edits.push((
+                        range.start.to_offset(&map, Bias::Left)
+                            ..range.end.to_offset(&map, Bias::Left),
+                        text.repeat(count),
+                    ))
+                }
 
                 editor.buffer().update(cx, |buffer, cx| {
                     buffer.edit(edits, None, cx);
                 });
                 editor.set_clip_at_line_ends(true, cx);
                 editor.change_selections(None, cx, |s| {
-                    s.select_anchor_ranges(stable_anchors);
-                    if count > 1 {
-                        s.move_cursors_with(|map, point, _| {
-                            (right(map, point, count - 1), SelectionGoal::None)
-                        });
-                    }
+                    s.move_with(|map, selection| {
+                        let point = movement::saturating_left(map, selection.head());
+                        selection.collapse_to(point, SelectionGoal::None)
+                    });
                 });
             });
         });
@@ -1441,5 +1430,9 @@ mod test {
         cx.set_shared_state("ˇhello world\n").await;
         cx.simulate_shared_keystrokes("2 r - f w .").await;
         cx.shared_state().await.assert_eq("--llo -ˇ-rld\n");
+
+        cx.set_shared_state("ˇhello world\n").await;
+        cx.simulate_shared_keystrokes("2 0 r - ").await;
+        cx.shared_state().await.assert_eq("ˇhello world\n");
     }
 }

crates/vim/src/normal/repeat.rs 🔗

@@ -499,4 +499,15 @@ mod test {
         cx.simulate_shared_keystrokes(".").await;
         cx.shared_state().await.assert_eq("ˇx hello\n");
     }
+
+    #[gpui::test]
+    async fn test_undo_repeated_insert(cx: &mut gpui::TestAppContext) {
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+
+        cx.set_shared_state("hellˇo").await;
+        cx.simulate_shared_keystrokes("3 a . escape").await;
+        cx.shared_state().await.assert_eq("hello..ˇ.");
+        cx.simulate_shared_keystrokes("u").await;
+        cx.shared_state().await.assert_eq("hellˇo");
+    }
 }

crates/vim/src/vim.rs 🔗

@@ -418,8 +418,10 @@ impl Vim {
             state.last_mode = last_mode;
             state.mode = mode;
             state.operator_stack.clear();
-            state.current_tx.take();
-            state.current_anchor.take();
+            if mode == Mode::Normal || mode != last_mode {
+                state.current_tx.take();
+                state.current_anchor.take();
+            }
         });
         if mode != Mode::Insert && mode != Mode::Replace {
             self.take_count(cx);
@@ -744,33 +746,48 @@ impl Vim {
     }
 
     fn transaction_undone(&mut self, transaction_id: &TransactionId, cx: &mut WindowContext) {
-        if !self.state().mode.is_visual() {
-            return;
-        };
-        self.update_active_editor(cx, |vim, editor, cx| {
-            let original_mode = vim.state().undo_modes.get(transaction_id);
-            editor.change_selections(None, cx, |s| match original_mode {
-                Some(Mode::VisualLine) => {
-                    s.move_with(|map, selection| {
-                        selection.collapse_to(
-                            map.prev_line_boundary(selection.start.to_point(map)).1,
-                            SelectionGoal::None,
-                        )
-                    });
-                }
-                Some(Mode::VisualBlock) => {
-                    let mut first = s.first_anchor();
-                    first.collapse_to(first.start, first.goal);
-                    s.select_anchors(vec![first]);
-                }
-                _ => {
-                    s.move_with(|_, selection| {
-                        selection.collapse_to(selection.start, selection.goal);
+        match self.state().mode {
+            Mode::VisualLine | Mode::VisualBlock | Mode::Visual => {
+                self.update_active_editor(cx, |vim, editor, cx| {
+                    let original_mode = vim.state().undo_modes.get(transaction_id);
+                    editor.change_selections(None, cx, |s| match original_mode {
+                        Some(Mode::VisualLine) => {
+                            s.move_with(|map, selection| {
+                                selection.collapse_to(
+                                    map.prev_line_boundary(selection.start.to_point(map)).1,
+                                    SelectionGoal::None,
+                                )
+                            });
+                        }
+                        Some(Mode::VisualBlock) => {
+                            let mut first = s.first_anchor();
+                            first.collapse_to(first.start, first.goal);
+                            s.select_anchors(vec![first]);
+                        }
+                        _ => {
+                            s.move_with(|map, selection| {
+                                selection.collapse_to(
+                                    map.clip_at_line_end(selection.start),
+                                    selection.goal,
+                                );
+                            });
+                        }
                     });
-                }
-            });
-        });
-        self.switch_mode(Mode::Normal, true, cx)
+                });
+                self.switch_mode(Mode::Normal, true, cx)
+            }
+            Mode::Normal => {
+                self.update_active_editor(cx, |_, editor, cx| {
+                    editor.change_selections(None, cx, |s| {
+                        s.move_with(|map, selection| {
+                            selection
+                                .collapse_to(map.clip_at_line_end(selection.end), selection.goal)
+                        })
+                    })
+                });
+            }
+            Mode::Insert | Mode::Replace => {}
+        }
     }
 
     fn transaction_ended(&mut self, editor: View<Editor>, cx: &mut WindowContext) {

crates/vim/test_data/test_r.json 🔗

@@ -22,3 +22,10 @@
 {"Key":"w"}
 {"Key":"."}
 {"Get":{"state":"--llo -ˇ-rld\n","mode":"Normal"}}
+{"Put":{"state":"ˇhello world\n"}}
+{"Key":"2"}
+{"Key":"0"}
+{"Key":"r"}
+{"Key":"-"}
+{"Key":""}
+{"Get":{"state":"ˇhello world\n","mode":"Normal"}}