Push to navigation history based on jump distance instead of time

crates/editor/src/editor.rs

@@ -53,6 +53,7 @@ use workspace::{Navigation, PathOpener, Workspace};
 
 const CURSOR_BLINK_INTERVAL: Duration = Duration::from_millis(500);
 const MAX_LINE_LEN: usize = 1024;
+const MIN_NAVIGATION_HISTORY_ROW_DELTA: i64 = 10;
 
 action!(Cancel);
 action!(Backspace);
@@ -809,6 +810,8 @@ impl Editor {
 
         let display_map = self.display_map.update(cx, |map, cx| map.snapshot(cx));
         let buffer = &display_map.buffer_snapshot;
+        let newest_selection = self.newest_selection_internal().unwrap().clone();
+
         let start;
         let end;
         let mode;
@@ -843,6 +846,8 @@ impl Editor {
             }
         }
 
+        self.push_to_navigation_history(newest_selection.head(), Some(end.to_point(&buffer)), cx);
+
         let selection = Selection {
             id: post_inc(&mut self.next_selection_id),
             start,
@@ -855,7 +860,6 @@ impl Editor {
             self.update_selections::<usize>(Vec::new(), None, cx);
         } else if click_count > 1 {
             // Remove the newest selection since it was only added as part of this multi-click.
-            let newest_selection = self.newest_selection::<usize>(buffer);
             let mut selections = self.local_selections(cx);
             selections.retain(|selection| selection.id != newest_selection.id);
             self.update_selections::<usize>(selections, None, cx)
@@ -2404,8 +2408,6 @@ impl Editor {
             return;
         }
 
-        self.push_to_navigation_history(cx);
-
         let selection = Selection {
             id: post_inc(&mut self.next_selection_id),
             start: 0,
@@ -2428,8 +2430,6 @@ impl Editor {
             return;
         }
 
-        self.push_to_navigation_history(cx);
-
         let cursor = self.buffer.read(cx).read(cx).len();
         let selection = Selection {
             id: post_inc(&mut self.next_selection_id),
@@ -2441,16 +2441,33 @@ impl Editor {
         self.update_selections(vec![selection], Some(Autoscroll::Fit), cx);
     }
 
-    fn push_to_navigation_history(&self, cx: &mut ViewContext<Self>) {
+    fn push_to_navigation_history(
+        &self,
+        position: Anchor,
+        new_position: Option<Point>,
+        cx: &mut ViewContext<Self>,
+    ) {
         if let Some(navigation) = &self.navigation {
             let buffer = self.buffer.read(cx).read(cx);
-            if let Some(last_selection) = self.selections.iter().max_by_key(|s| s.id) {
+            let offset = position.to_offset(&buffer);
-                let anchor = last_selection.head();
+            let point = position.to_point(&buffer);
-                let offset = anchor.to_offset(&buffer);
             drop(buffer);
-                navigation.push(Some(NavigationData { anchor, offset }), cx);
+
+            if let Some(new_position) = new_position {
+                let row_delta = (new_position.row as i64 - point.row as i64).abs();
+                if row_delta < MIN_NAVIGATION_HISTORY_ROW_DELTA {
+                    return;
                 }
             }
+
+            navigation.push(
+                Some(NavigationData {
+                    anchor: position,
+                    offset,
+                }),
+                cx,
+            );
+        }
     }
 
     pub fn select_to_end(&mut self, _: &SelectToEnd, cx: &mut ViewContext<Self>) {
@@ -3230,14 +3247,14 @@ impl Editor {
         &self,
         snapshot: &MultiBufferSnapshot,
     ) -> Selection<D> {
-        self.pending_selection(snapshot)
+        self.resolve_selection(self.newest_selection_internal().unwrap(), snapshot)
-            .or_else(|| {
+    }
-                self.selections
+
-                    .iter()
+    pub fn newest_selection_internal(&self) -> Option<&Selection<Anchor>> {
-                    .max_by_key(|s| s.id)
+        self.pending_selection
-                    .map(|selection| self.resolve_selection(selection, snapshot))
+            .as_ref()
-            })
+            .map(|s| &s.selection)
-            .unwrap()
+            .or_else(|| self.selections.iter().max_by_key(|s| s.id))
     }
 
     pub fn update_selections<T>(
@@ -3248,10 +3265,11 @@ impl Editor {
     ) where
         T: ToOffset + ToPoint + Ord + std::marker::Copy + std::fmt::Debug,
     {
+        let buffer = self.buffer.read(cx).snapshot(cx);
+        let old_cursor_position = self.newest_selection_internal().map(|s| s.head());
         selections.sort_unstable_by_key(|s| s.start);
 
         // Merge overlapping selections.
-        let buffer = self.buffer.read(cx).snapshot(cx);
         let mut i = 1;
         while i < selections.len() {
             if selections[i - 1].end >= selections[i].start {
@@ -3292,22 +3310,21 @@ impl Editor {
             }
         }
 
+        if let Some(old_cursor_position) = old_cursor_position {
+            let new_cursor_position = selections
+                .iter()
+                .max_by_key(|s| s.id)
+                .map(|s| s.head().to_point(&buffer));
+            if new_cursor_position.is_some() {
+                self.push_to_navigation_history(old_cursor_position, new_cursor_position, cx);
+            }
+        }
+
         if let Some(autoscroll) = autoscroll {
             self.request_autoscroll(autoscroll, cx);
         }
         self.pause_cursor_blinking(cx);
 
-        let prev_newest_selection = self.selections.iter().max_by_key(|s| s.id);
-        let curr_newest_selection = selections.iter().max_by_key(|s| s.id);
-        if let Some((prev_selection, curr_selection)) =
-            prev_newest_selection.zip(curr_newest_selection)
-        {
-            if prev_selection.head().to_offset(&buffer) != curr_selection.head().to_offset(&buffer)
-            {
-                self.push_to_navigation_history(cx);
-            }
-        }
-
         self.set_selections(
             Arc::from_iter(selections.into_iter().map(|selection| {
                 let end_bias = if selection.end > selection.start {

crates/editor/src/items.rs

@@ -149,7 +149,9 @@ impl ItemView for Editor {
     }
 
     fn deactivated(&mut self, cx: &mut ViewContext<Self>) {
-        self.push_to_navigation_history(cx);
+        if let Some(selection) = self.newest_selection_internal() {
+            self.push_to_navigation_history(selection.head(), None, cx);
+        }
     }
 
     fn is_dirty(&self, cx: &AppContext) -> bool {

crates/workspace/src/pane.rs

@@ -11,13 +11,7 @@ use gpui::{
 };
 use postage::watch;
 use project::ProjectPath;
-use std::{
+use std::{any::Any, cell::RefCell, cmp, mem, rc::Rc};
-    any::Any,
-    cell::RefCell,
-    cmp, mem,
-    rc::Rc,
-    time::{Duration, Instant},
-};
 use util::ResultExt;
 
 action!(Split, SplitDirection);
@@ -110,7 +104,6 @@ impl Default for NavigationMode {
 struct NavigationEntry {
     item_view: Box<dyn WeakItemViewHandle>,
     data: Option<Box<dyn Any>>,
-    insertion_time: Option<Instant>,
 }
 
 impl Pane {
@@ -558,20 +551,9 @@ impl Navigation {
         let mut state = self.0.borrow_mut();
         match state.mode {
             NavigationMode::Normal => {
-                let mut insertion_time = Instant::now();
-                if let Some(prev_insertion_time) =
-                    state.backward_stack.last().and_then(|e| e.insertion_time)
-                {
-                    if insertion_time.duration_since(prev_insertion_time) <= Duration::from_secs(2)
-                    {
-                        state.backward_stack.pop();
-                        insertion_time = prev_insertion_time;
-                    }
-                }
                 state.backward_stack.push(NavigationEntry {
                     item_view: Box::new(cx.weak_handle()),
                     data: data.map(|data| Box::new(data) as Box<dyn Any>),
-                    insertion_time: Some(insertion_time),
                 });
                 state.forward_stack.clear();
             }
@@ -579,14 +561,12 @@ impl Navigation {
                 state.forward_stack.push(NavigationEntry {
                     item_view: Box::new(cx.weak_handle()),
                     data: data.map(|data| Box::new(data) as Box<dyn Any>),
-                    insertion_time: None,
                 });
             }
             NavigationMode::GoingForward => {
                 state.backward_stack.push(NavigationEntry {
                     item_view: Box::new(cx.weak_handle()),
                     data: data.map(|data| Box::new(data) as Box<dyn Any>),
-                    insertion_time: None,
                 });
             }
         }

crates/zed/src/zed.rs

@@ -701,9 +701,9 @@ mod tests {
                 "/root",
                 json!({
                     "a": {
-                        "file1": "contents 1",
+                        "file1": "contents 1\n".repeat(20),
-                        "file2": "contents 2",
+                        "file2": "contents 2\n".repeat(20),
-                        "file3": "contents 3",
+                        "file3": "contents 3\n".repeat(20),
                     },
                 }),
             )
@@ -731,7 +731,7 @@ mod tests {
             .downcast::<Editor>()
             .unwrap();
         editor1.update(&mut cx, |editor, cx| {
-            editor.select_display_ranges(&[DisplayPoint::new(0, 1)..DisplayPoint::new(0, 1)], cx);
+            editor.select_display_ranges(&[DisplayPoint::new(10, 0)..DisplayPoint::new(10, 0)], cx);
         });
         let editor2 = workspace
             .update(&mut cx, |w, cx| w.open_path(file2.clone(), cx))
@@ -748,11 +748,11 @@ mod tests {
             .downcast::<Editor>()
             .unwrap();
         editor3.update(&mut cx, |editor, cx| {
-            editor.select_display_ranges(&[DisplayPoint::new(0, 2)..DisplayPoint::new(0, 2)], cx);
+            editor.select_display_ranges(&[DisplayPoint::new(15, 0)..DisplayPoint::new(15, 0)], cx);
         });
         assert_eq!(
             active_location(&workspace, &mut cx),
-            (file3.clone(), DisplayPoint::new(0, 2))
+            (file3.clone(), DisplayPoint::new(15, 0))
         );
 
         workspace
@@ -776,7 +776,7 @@ mod tests {
             .await;
         assert_eq!(
             active_location(&workspace, &mut cx),
-            (file1.clone(), DisplayPoint::new(0, 1))
+            (file1.clone(), DisplayPoint::new(10, 0))
         );
 
         workspace
@@ -801,7 +801,7 @@ mod tests {
             .await;
         assert_eq!(
             active_location(&workspace, &mut cx),
-            (file1.clone(), DisplayPoint::new(0, 1))
+            (file1.clone(), DisplayPoint::new(10, 0))
         );
 
         workspace
@@ -844,7 +844,7 @@ mod tests {
             .await;
         assert_eq!(
             active_location(&workspace, &mut cx),
-            (file1.clone(), DisplayPoint::new(0, 1))
+            (file1.clone(), DisplayPoint::new(10, 0))
         );
         workspace
             .update(&mut cx, |w, cx| Pane::go_forward(w, cx))