Add a way to navigate between changes (#28891)

Kirill Bulatov created

Closes https://github.com/zed-industries/zed/issues/19731

Adds `editor::GoToPreviousChange` and `editor::GoToNextChange` that work
the same as `vim::ChangeListOlder` and `vim::ChangeListNewer` as the
common logic was extracted and reused.

Release Notes:

- Added a way to navigate between changes with
`editor::GoToPreviousChange` and `editor::GoToNextChange`

Change summary

assets/keymaps/default-linux.json |   4 
assets/keymaps/default-macos.json |   4 
crates/editor/src/actions.rs      |   2 
crates/editor/src/editor.rs       | 131 +++++++++++++++++++++++++++++++-
crates/editor/src/element.rs      |   2 
crates/vim/src/change_list.rs     |  98 +++++++++++-------------
crates/vim/src/vim.rs             |   4 -
7 files changed, 178 insertions(+), 67 deletions(-)

Detailed changes

assets/keymaps/default-linux.json 🔗

@@ -134,7 +134,9 @@
       "shift-f10": "editor::OpenContextMenu",
       "ctrl-shift-e": "editor::ToggleEditPrediction",
       "f9": "editor::ToggleBreakpoint",
-      "shift-f9": "editor::EditLogBreakpoint"
+      "shift-f9": "editor::EditLogBreakpoint",
+      "ctrl-shift-backspace": "editor::GoToPreviousChange",
+      "ctrl-shift-alt-backspace": "editor::GoToNextChange"
     }
   },
   {

assets/keymaps/default-macos.json 🔗

@@ -542,7 +542,9 @@
       "cmd-\\": "pane::SplitRight",
       "cmd-k v": "markdown::OpenPreviewToTheSide",
       "cmd-shift-v": "markdown::OpenPreview",
-      "ctrl-cmd-c": "editor::DisplayCursorNames"
+      "ctrl-cmd-c": "editor::DisplayCursorNames",
+      "cmd-shift-backspace": "editor::GoToPreviousChange",
+      "cmd-shift-alt-backspace": "editor::GoToNextChange"
     }
   },
   {

crates/editor/src/actions.rs 🔗

@@ -306,6 +306,8 @@ actions!(
         GoToPreviousHunk,
         GoToImplementation,
         GoToImplementationSplit,
+        GoToNextChange,
+        GoToPreviousChange,
         GoToPreviousDiagnostic,
         GoToTypeDefinition,
         GoToTypeDefinitionSplit,

crates/editor/src/editor.rs 🔗

@@ -693,6 +693,52 @@ pub trait Addon: 'static {
     fn to_any(&self) -> &dyn std::any::Any;
 }
 
+/// A set of caret positions, registered when the editor was edited.
+pub struct ChangeList {
+    changes: Vec<Vec<Anchor>>,
+    /// Currently "selected" change.
+    position: Option<usize>,
+}
+
+impl ChangeList {
+    pub fn new() -> Self {
+        Self {
+            changes: Vec::new(),
+            position: None,
+        }
+    }
+
+    /// Moves to the next change in the list (based on the direction given) and returns the caret positions for the next change.
+    /// If reaches the end of the list in the direction, returns the corresponding change until called for a different direction.
+    pub fn next_change(&mut self, count: usize, direction: Direction) -> Option<&[Anchor]> {
+        if self.changes.is_empty() {
+            return None;
+        }
+
+        let prev = self.position.unwrap_or(self.changes.len());
+        let next = if direction == Direction::Prev {
+            prev.saturating_sub(count)
+        } else {
+            (prev + count).min(self.changes.len() - 1)
+        };
+        self.position = Some(next);
+        self.changes.get(next).map(|anchors| anchors.as_slice())
+    }
+
+    /// Adds a new change to the list, resetting the change list position.
+    pub fn push_to_change_list(&mut self, pop_state: bool, new_positions: Vec<Anchor>) {
+        self.position.take();
+        if pop_state {
+            self.changes.pop();
+        }
+        self.changes.push(new_positions.clone());
+    }
+
+    pub fn last(&self) -> Option<&[Anchor]> {
+        self.changes.last().map(|anchors| anchors.as_slice())
+    }
+}
+
 /// Zed's primary implementation of text input, allowing users to edit a [`MultiBuffer`].
 ///
 /// See the [module level documentation](self) for more information.
@@ -857,6 +903,7 @@ pub struct Editor {
     serialize_folds: Task<()>,
     mouse_cursor_hidden: bool,
     hide_mouse_mode: HideMouseMode,
+    pub change_list: ChangeList,
 }
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Default)]
@@ -1648,6 +1695,7 @@ impl Editor {
             hide_mouse_mode: EditorSettings::get_global(cx)
                 .hide_mouse
                 .unwrap_or_default(),
+            change_list: ChangeList::new(),
         };
         if let Some(breakpoints) = this.breakpoint_store.as_ref() {
             this._subscriptions
@@ -1661,8 +1709,8 @@ impl Editor {
         this._subscriptions.push(cx.subscribe_in(
             &cx.entity(),
             window,
-            |editor, _, e: &EditorEvent, window, cx| {
-                if let EditorEvent::SelectionsChanged { local } = e {
+            |editor, _, e: &EditorEvent, window, cx| match e {
+                EditorEvent::ScrollPositionChanged { local, .. } => {
                     if *local {
                         let new_anchor = editor.scroll_manager.anchor();
                         let snapshot = editor.snapshot(window, cx);
@@ -1674,6 +1722,30 @@ impl Editor {
                         });
                     }
                 }
+                EditorEvent::Edited { .. } => {
+                    if !vim_enabled(cx) {
+                        let (map, selections) = editor.selections.all_adjusted_display(cx);
+                        let pop_state = editor
+                            .change_list
+                            .last()
+                            .map(|previous| {
+                                previous.len() == selections.len()
+                                    && previous.iter().enumerate().all(|(ix, p)| {
+                                        p.to_display_point(&map).row()
+                                            == selections[ix].head().row()
+                                    })
+                            })
+                            .unwrap_or(false);
+                        let new_positions = selections
+                            .into_iter()
+                            .map(|s| map.display_point_to_anchor(s.head(), Bias::Left))
+                            .collect();
+                        editor
+                            .change_list
+                            .push_to_change_list(pop_state, new_positions);
+                    }
+                }
+                _ => (),
             },
         ));
 
@@ -13303,6 +13375,48 @@ impl Editor {
             .or_else(|| snapshot.buffer_snapshot.diff_hunk_before(Point::MAX))
     }
 
+    fn go_to_next_change(
+        &mut self,
+        _: &GoToNextChange,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
+        if let Some(selections) = self
+            .change_list
+            .next_change(1, Direction::Next)
+            .map(|s| s.to_vec())
+        {
+            self.change_selections(Some(Autoscroll::fit()), window, cx, |s| {
+                let map = s.display_map();
+                s.select_display_ranges(selections.iter().map(|a| {
+                    let point = a.to_display_point(&map);
+                    point..point
+                }))
+            })
+        }
+    }
+
+    fn go_to_previous_change(
+        &mut self,
+        _: &GoToPreviousChange,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
+        if let Some(selections) = self
+            .change_list
+            .next_change(1, Direction::Prev)
+            .map(|s| s.to_vec())
+        {
+            self.change_selections(Some(Autoscroll::fit()), window, cx, |s| {
+                let map = s.display_map();
+                s.select_display_ranges(selections.iter().map(|a| {
+                    let point = a.to_display_point(&map);
+                    point..point
+                }))
+            })
+        }
+    }
+
     fn go_to_line<T: 'static>(
         &mut self,
         position: Anchor,
@@ -17711,11 +17825,7 @@ impl Editor {
             .and_then(|e| e.to_str())
             .map(|a| a.to_string()));
 
-        let vim_mode = cx
-            .global::<SettingsStore>()
-            .raw_user_settings()
-            .get("vim_mode")
-            == Some(&serde_json::Value::Bool(true));
+        let vim_mode = vim_enabled(cx);
 
         let edit_predictions_provider = all_language_settings(file, cx).edit_predictions.provider;
         let copilot_enabled = edit_predictions_provider
@@ -18161,6 +18271,13 @@ impl Editor {
     }
 }
 
+fn vim_enabled(cx: &App) -> bool {
+    cx.global::<SettingsStore>()
+        .raw_user_settings()
+        .get("vim_mode")
+        == Some(&serde_json::Value::Bool(true))
+}
+
 // Consider user intent and default settings
 fn choose_completion_range(
     completion: &Completion,

crates/editor/src/element.rs 🔗

@@ -435,6 +435,8 @@ impl EditorElement {
         register_action(editor, window, Editor::stage_and_next);
         register_action(editor, window, Editor::unstage_and_next);
         register_action(editor, window, Editor::expand_all_diff_hunks);
+        register_action(editor, window, Editor::go_to_previous_change);
+        register_action(editor, window, Editor::go_to_next_change);
 
         register_action(editor, window, |editor, action, window, cx| {
             if let Some(task) = editor.format(action, window, cx) {

crates/vim/src/change_list.rs 🔗

@@ -1,6 +1,4 @@
-use editor::{
-    Anchor, Bias, Direction, Editor, display_map::ToDisplayPoint, movement, scroll::Autoscroll,
-};
+use editor::{Bias, Direction, Editor, display_map::ToDisplayPoint, movement, scroll::Autoscroll};
 use gpui::{Context, Window, actions};
 
 use crate::{Vim, state::Mode};
@@ -25,68 +23,60 @@ impl Vim {
     ) {
         let count = Vim::take_count(cx).unwrap_or(1);
         Vim::take_forced_motion(cx);
-        if self.change_list.is_empty() {
-            return;
-        }
-
-        let prev = self.change_list_position.unwrap_or(self.change_list.len());
-        let next = if direction == Direction::Prev {
-            prev.saturating_sub(count)
-        } else {
-            (prev + count).min(self.change_list.len() - 1)
-        };
-        self.change_list_position = Some(next);
-        let Some(selections) = self.change_list.get(next).cloned() else {
-            return;
-        };
         self.update_editor(window, cx, |_, editor, window, cx| {
-            editor.change_selections(Some(Autoscroll::fit()), window, cx, |s| {
-                let map = s.display_map();
-                s.select_display_ranges(selections.into_iter().map(|a| {
-                    let point = a.to_display_point(&map);
-                    point..point
-                }))
-            })
+            if let Some(selections) = editor
+                .change_list
+                .next_change(count, direction)
+                .map(|s| s.to_vec())
+            {
+                editor.change_selections(Some(Autoscroll::fit()), window, cx, |s| {
+                    let map = s.display_map();
+                    s.select_display_ranges(selections.iter().map(|a| {
+                        let point = a.to_display_point(&map);
+                        point..point
+                    }))
+                })
+            };
         });
     }
 
     pub(crate) fn push_to_change_list(&mut self, window: &mut Window, cx: &mut Context<Self>) {
-        let Some((map, selections, buffer)) = self.update_editor(window, cx, |_, editor, _, cx| {
+        let Some((new_positions, buffer)) = self.update_editor(window, cx, |vim, editor, _, cx| {
             let (map, selections) = editor.selections.all_adjusted_display(cx);
             let buffer = editor.buffer().clone();
-            (map, selections, buffer)
+
+            let pop_state = editor
+                .change_list
+                .last()
+                .map(|previous| {
+                    previous.len() == selections.len()
+                        && previous.iter().enumerate().all(|(ix, p)| {
+                            p.to_display_point(&map).row() == selections[ix].head().row()
+                        })
+                })
+                .unwrap_or(false);
+
+            let new_positions = selections
+                .into_iter()
+                .map(|s| {
+                    let point = if vim.mode == Mode::Insert {
+                        movement::saturating_left(&map, s.head())
+                    } else {
+                        s.head()
+                    };
+                    map.display_point_to_anchor(point, Bias::Left)
+                })
+                .collect::<Vec<_>>();
+
+            editor
+                .change_list
+                .push_to_change_list(pop_state, new_positions.clone());
+
+            (new_positions, buffer)
         }) else {
             return;
         };
 
-        let pop_state = self
-            .change_list
-            .last()
-            .map(|previous| {
-                previous.len() == selections.len()
-                    && previous.iter().enumerate().all(|(ix, p)| {
-                        p.to_display_point(&map).row() == selections[ix].head().row()
-                    })
-            })
-            .unwrap_or(false);
-
-        let new_positions: Vec<Anchor> = selections
-            .into_iter()
-            .map(|s| {
-                let point = if self.mode == Mode::Insert {
-                    movement::saturating_left(&map, s.head())
-                } else {
-                    s.head()
-                };
-                map.display_point_to_anchor(point, Bias::Left)
-            })
-            .collect();
-
-        self.change_list_position.take();
-        if pop_state {
-            self.change_list.pop();
-        }
-        self.change_list.push(new_positions.clone());
         self.set_mark(".".to_string(), new_positions, &buffer, window, cx)
     }
 }

crates/vim/src/vim.rs 🔗

@@ -323,8 +323,6 @@ pub(crate) struct Vim {
     pub(crate) replacements: Vec<(Range<editor::Anchor>, String)>,
 
     pub(crate) stored_visual_mode: Option<(Mode, Vec<bool>)>,
-    pub(crate) change_list: Vec<Vec<Anchor>>,
-    pub(crate) change_list_position: Option<usize>,
 
     pub(crate) current_tx: Option<TransactionId>,
     pub(crate) current_anchor: Option<Selection<Anchor>>,
@@ -370,8 +368,6 @@ impl Vim {
             replacements: Vec::new(),
 
             stored_visual_mode: None,
-            change_list: Vec::new(),
-            change_list_position: None,
             current_tx: None,
             current_anchor: None,
             undo_modes: HashMap::default(),