vim: Better jump list support (#33495)

Conrad Irwin created

Closes #23527
Closes #30183
Closes some Discord chats

Release Notes:

- vim: Motions now push to the jump list using the same logic as vim
(i.e.
`G`/`g g`/`g d` always do, but `j`/`k` always don't). Most non-vim
actions
(including clicking with the mouse) continue to push to the jump list
only
  when they move the cursor by 10 or more lines.

Change summary

crates/editor/src/editor.rs              | 31 ++++++++---
crates/editor/src/items.rs               |  2 
crates/vim/src/motion.rs                 | 67 ++++++++++++++++++++++++++
crates/vim/src/normal.rs                 | 52 +++++++++++++++++---
crates/vim/test_data/test_jump_list.json | 14 +++++
5 files changed, 147 insertions(+), 19 deletions(-)

Detailed changes

crates/editor/src/editor.rs 🔗

@@ -1256,7 +1256,7 @@ impl Default for SelectionHistoryMode {
 
 #[derive(Debug)]
 pub struct SelectionEffects {
-    nav_history: bool,
+    nav_history: Option<bool>,
     completions: bool,
     scroll: Option<Autoscroll>,
 }
@@ -1264,7 +1264,7 @@ pub struct SelectionEffects {
 impl Default for SelectionEffects {
     fn default() -> Self {
         Self {
-            nav_history: true,
+            nav_history: None,
             completions: true,
             scroll: Some(Autoscroll::fit()),
         }
@@ -1294,7 +1294,7 @@ impl SelectionEffects {
 
     pub fn nav_history(self, nav_history: bool) -> Self {
         Self {
-            nav_history,
+            nav_history: Some(nav_history),
             ..self
         }
     }
@@ -2909,11 +2909,12 @@ impl Editor {
         let new_cursor_position = newest_selection.head();
         let selection_start = newest_selection.start;
 
-        if effects.nav_history {
+        if effects.nav_history.is_none() || effects.nav_history == Some(true) {
             self.push_to_nav_history(
                 *old_cursor_position,
                 Some(new_cursor_position.to_point(buffer)),
                 false,
+                effects.nav_history == Some(true),
                 cx,
             );
         }
@@ -3164,7 +3165,7 @@ impl Editor {
         if let Some(state) = &mut self.deferred_selection_effects_state {
             state.effects.scroll = effects.scroll.or(state.effects.scroll);
             state.effects.completions = effects.completions;
-            state.effects.nav_history |= effects.nav_history;
+            state.effects.nav_history = effects.nav_history.or(state.effects.nav_history);
             let (changed, result) = self.selections.change_with(cx, change);
             state.changed |= changed;
             return result;
@@ -13097,7 +13098,13 @@ impl Editor {
     }
 
     pub fn create_nav_history_entry(&mut self, cx: &mut Context<Self>) {
-        self.push_to_nav_history(self.selections.newest_anchor().head(), None, false, cx);
+        self.push_to_nav_history(
+            self.selections.newest_anchor().head(),
+            None,
+            false,
+            true,
+            cx,
+        );
     }
 
     fn push_to_nav_history(
@@ -13105,6 +13112,7 @@ impl Editor {
         cursor_anchor: Anchor,
         new_position: Option<Point>,
         is_deactivate: bool,
+        always: bool,
         cx: &mut Context<Self>,
     ) {
         if let Some(nav_history) = self.nav_history.as_mut() {
@@ -13116,7 +13124,7 @@ impl Editor {
 
             if let Some(new_position) = new_position {
                 let row_delta = (new_position.row as i64 - cursor_position.row as i64).abs();
-                if row_delta < MIN_NAVIGATION_HISTORY_ROW_DELTA {
+                if row_delta == 0 || (row_delta < MIN_NAVIGATION_HISTORY_ROW_DELTA && !always) {
                     return;
                 }
             }
@@ -14788,9 +14796,12 @@ impl Editor {
         let Some(end) = multibuffer.buffer_point_to_anchor(&buffer, range.end, cx) else {
             return;
         };
-        self.change_selections(Some(Autoscroll::center()), window, cx, |s| {
-            s.select_anchor_ranges([start..end])
-        });
+        self.change_selections(
+            SelectionEffects::default().nav_history(true),
+            window,
+            cx,
+            |s| s.select_anchor_ranges([start..end]),
+        );
     }
 
     pub fn go_to_diagnostic(

crates/editor/src/items.rs 🔗

@@ -778,7 +778,7 @@ impl Item for Editor {
 
     fn deactivated(&mut self, _: &mut Window, cx: &mut Context<Self>) {
         let selection = self.selections.newest_anchor();
-        self.push_to_nav_history(selection.head(), None, true, cx);
+        self.push_to_nav_history(selection.head(), None, true, false, cx);
     }
 
     fn workspace_deactivated(&mut self, _: &mut Window, cx: &mut Context<Self>) {

crates/vim/src/motion.rs 🔗

@@ -768,6 +768,73 @@ impl Motion {
         }
     }
 
+    pub(crate) fn push_to_jump_list(&self) -> bool {
+        use Motion::*;
+        match self {
+            CurrentLine
+            | Down { .. }
+            | EndOfLine { .. }
+            | EndOfLineDownward
+            | FindBackward { .. }
+            | FindForward { .. }
+            | FirstNonWhitespace { .. }
+            | GoToColumn
+            | Left
+            | MiddleOfLine { .. }
+            | NextLineStart
+            | NextSubwordEnd { .. }
+            | NextSubwordStart { .. }
+            | NextWordEnd { .. }
+            | NextWordStart { .. }
+            | PreviousLineStart
+            | PreviousSubwordEnd { .. }
+            | PreviousSubwordStart { .. }
+            | PreviousWordEnd { .. }
+            | PreviousWordStart { .. }
+            | RepeatFind { .. }
+            | RepeatFindReversed { .. }
+            | Right
+            | StartOfLine { .. }
+            | StartOfLineDownward
+            | Up { .. }
+            | WrappingLeft
+            | WrappingRight => false,
+            EndOfDocument
+            | EndOfParagraph
+            | GoToPercentage
+            | Jump { .. }
+            | Matching
+            | NextComment
+            | NextGreaterIndent
+            | NextLesserIndent
+            | NextMethodEnd
+            | NextMethodStart
+            | NextSameIndent
+            | NextSectionEnd
+            | NextSectionStart
+            | PreviousComment
+            | PreviousGreaterIndent
+            | PreviousLesserIndent
+            | PreviousMethodEnd
+            | PreviousMethodStart
+            | PreviousSameIndent
+            | PreviousSectionEnd
+            | PreviousSectionStart
+            | SentenceBackward
+            | SentenceForward
+            | Sneak { .. }
+            | SneakBackward { .. }
+            | StartOfDocument
+            | StartOfParagraph
+            | UnmatchedBackward { .. }
+            | UnmatchedForward { .. }
+            | WindowBottom
+            | WindowMiddle
+            | WindowTop
+            | ZedSearchResult { .. } => true,
+        }
+    }
+
     pub fn infallible(&self) -> bool {
         use Motion::*;
         match self {

crates/vim/src/normal.rs 🔗

@@ -24,10 +24,10 @@ use crate::{
 };
 use collections::BTreeSet;
 use convert::ConvertTarget;
-use editor::Anchor;
 use editor::Bias;
 use editor::Editor;
 use editor::scroll::Autoscroll;
+use editor::{Anchor, SelectionEffects};
 use editor::{display_map::ToDisplayPoint, movement};
 use gpui::{Context, Window, actions};
 use language::{Point, SelectionGoal, ToPoint};
@@ -358,13 +358,18 @@ impl Vim {
     ) {
         self.update_editor(window, cx, |_, editor, window, cx| {
             let text_layout_details = editor.text_layout_details(window);
-            editor.change_selections(Some(Autoscroll::fit()), window, cx, |s| {
-                s.move_cursors_with(|map, cursor, goal| {
-                    motion
-                        .move_point(map, cursor, goal, times, &text_layout_details)
-                        .unwrap_or((cursor, goal))
-                })
-            })
+            editor.change_selections(
+                SelectionEffects::default().nav_history(motion.push_to_jump_list()),
+                window,
+                cx,
+                |s| {
+                    s.move_cursors_with(|map, cursor, goal| {
+                        motion
+                            .move_point(map, cursor, goal, times, &text_layout_details)
+                            .unwrap_or((cursor, goal))
+                    })
+                },
+            )
         });
     }
 
@@ -1799,4 +1804,35 @@ mod test {
             fox jˇumps over
             the lazy dog"});
     }
+
+    #[gpui::test]
+    async fn test_jump_list(cx: &mut gpui::TestAppContext) {
+        let mut cx = NeovimBackedTestContext::new(cx).await;
+
+        cx.set_shared_state(indoc! {"
+            ˇfn a() { }
+
+
+
+
+
+            fn b() { }
+
+
+
+
+
+            fn b() { }"})
+            .await;
+        cx.simulate_shared_keystrokes("3 }").await;
+        cx.shared_state().await.assert_matches();
+        cx.simulate_shared_keystrokes("ctrl-o").await;
+        cx.shared_state().await.assert_matches();
+        cx.simulate_shared_keystrokes("ctrl-i").await;
+        cx.shared_state().await.assert_matches();
+        cx.simulate_shared_keystrokes("1 1 k").await;
+        cx.shared_state().await.assert_matches();
+        cx.simulate_shared_keystrokes("ctrl-o").await;
+        cx.shared_state().await.assert_matches();
+    }
 }

crates/vim/test_data/test_jump_list.json 🔗

@@ -0,0 +1,14 @@
+{"Put":{"state":"ˇfn a() { }\n\n\n\n\n\nfn b() { }\n\n\n\n\n\nfn b() { }"}}
+{"Key":"3"}
+{"Key":"}"}
+{"Get":{"state":"fn a() { }\n\n\n\n\n\nfn b() { }\n\n\n\n\n\nfn b() { ˇ}","mode":"Normal"}}
+{"Key":"ctrl-o"}
+{"Get":{"state":"ˇfn a() { }\n\n\n\n\n\nfn b() { }\n\n\n\n\n\nfn b() { }","mode":"Normal"}}
+{"Key":"ctrl-i"}
+{"Get":{"state":"fn a() { }\n\n\n\n\n\nfn b() { }\n\n\n\n\n\nfn b() { ˇ}","mode":"Normal"}}
+{"Key":"1"}
+{"Key":"1"}
+{"Key":"k"}
+{"Get":{"state":"fn a() { }\nˇ\n\n\n\n\nfn b() { }\n\n\n\n\n\nfn b() { }","mode":"Normal"}}
+{"Key":"ctrl-o"}
+{"Get":{"state":"ˇfn a() { }\n\n\n\n\n\nfn b() { }\n\n\n\n\n\nfn b() { }","mode":"Normal"}}