Only store one nav history entry when opening excerpts

Nathan Sobo created 3 years ago

Also: Introduce the ability to disable and enable the nav history directly. This allows us to explicitly push an entry when opening excerpts and then disable all pushes as we open individual buffers.

Change summary

crates/editor/src/editor.rs  | 32 +++++++++++++++++---------------
crates/workspace/src/pane.rs | 22 ++++++++--------------
2 files changed, 25 insertions(+), 29 deletions(-)

Detailed changes

crates/editor/src/editor.rs

@@ -4172,6 +4172,7 @@ impl Editor {
         cx.spawn(|workspace, mut cx| async move {
             let definitions = definitions.await?;
             workspace.update(&mut cx, |workspace, cx| {
+                let nav_history = workspace.active_pane().read(cx).nav_history().clone();

                 for definition in definitions {
                     let range = definition.range.to_offset(definition.buffer.read(cx));
                     let target_editor_handle = workspace
@@ -4182,15 +4183,11 @@ impl Editor {
                     target_editor_handle.update(cx, |target_editor, cx| {
                         // When selecting a definition in a different buffer, disable the nav history
                         // to avoid creating a history entry at the previous cursor location.
-                        let prev_nav_history_len = target_editor

-                            .nav_history()

-                            .map_or(0, |history| history.len());

-                        target_editor.select_ranges([range], Some(Autoscroll::Center), cx);

                         if editor_handle != target_editor_handle {
-                            if let Some(history) = target_editor.nav_history() {

-                                history.truncate(prev_nav_history_len);

-                            }

+                            nav_history.borrow_mut().disable();

                         }
+                        target_editor.select_ranges([range], Some(Autoscroll::Center), cx);

+                        nav_history.borrow_mut().enable();

                     });
                 }
             });
@@ -5389,28 +5386,33 @@ impl Editor {
             }
         }
 
+        editor_handle.update(cx, |editor, cx| {

+            editor.push_to_nav_history(editor.newest_anchor_selection().head(), None, cx);

+        });

+        let nav_history = workspace.active_pane().read(cx).nav_history().clone();

+        nav_history.borrow_mut().disable();

+

         // We defer the pane interaction because we ourselves are a workspace item
         // and activating a new item causes the pane to call a method on us reentrantly,
         // which panics if we're on the stack.
-        cx.defer(|workspace, cx| {

-            for (buffer, ranges) in new_selections_by_buffer {

+        cx.defer(move |workspace, cx| {

+            for (ix, (buffer, ranges)) in new_selections_by_buffer.into_iter().enumerate() {

                 let buffer = BufferItemHandle(buffer);
-                if !workspace.activate_pane_for_item(&buffer, cx) {

+                if ix == 0 && !workspace.activate_pane_for_item(&buffer, cx) {

                     workspace.activate_next_pane(cx);
                 }
+

                 let editor = workspace
                     .open_item(buffer, cx)
                     .downcast::<Editor>()
                     .unwrap();
+

                 editor.update(cx, |editor, cx| {
-                    let prev_nav_history_len =

-                        editor.nav_history().map_or(0, |history| history.len());

                     editor.select_ranges(ranges, Some(Autoscroll::Newest), cx);
-                    if let Some(history) = editor.nav_history() {

-                        history.truncate(prev_nav_history_len);

-                    }

                 });
             }
+

+            nav_history.borrow_mut().enable();

         });
     }
 }

crates/workspace/src/pane.rs

@@ -123,6 +123,7 @@ enum NavigationMode {
     Normal,
     GoingBack,
     GoingForward,
+    Disabled,
 }
 
 impl Default for NavigationMode {
@@ -149,7 +150,7 @@ impl Pane {
         }
     }
 
-    pub(crate) fn nav_history(&self) -> &Rc<RefCell<NavHistory>> {
+    pub fn nav_history(&self) -> &Rc<RefCell<NavHistory>> {
         &self.nav_history
     }
 
@@ -649,14 +650,6 @@ impl<T: Toolbar> ToolbarHandle for ViewHandle<T> {
 }
 
 impl ItemNavHistory {
-    pub fn len(&self) -> usize {
-        self.history.borrow().len()
-    }
-
-    pub fn truncate(&self, len: usize) {
-        self.history.borrow_mut().truncate(len)
-    }
-
     pub fn new<T: ItemView>(history: Rc<RefCell<NavHistory>>, item_view: &ViewHandle<T>) -> Self {
         Self {
             history,
@@ -674,12 +667,12 @@ impl ItemNavHistory {
 }
 
 impl NavHistory {
-    pub fn len(&self) -> usize {
-        self.backward_stack.len()
+    pub fn disable(&mut self) {
+        self.mode = NavigationMode::Disabled;
     }
 
-    pub fn truncate(&mut self, len: usize) {
-        self.backward_stack.truncate(len);
+    pub fn enable(&mut self) {
+        self.mode = NavigationMode::Normal;
     }
 
     pub fn pop_backward(&mut self) -> Option<NavigationEntry> {
@@ -692,7 +685,7 @@ impl NavHistory {
 
     fn pop(&mut self, mode: NavigationMode) -> Option<NavigationEntry> {
         match mode {
-            NavigationMode::Normal => None,
+            NavigationMode::Normal | NavigationMode::Disabled => None,
             NavigationMode::GoingBack => self.pop_backward(),
             NavigationMode::GoingForward => self.pop_forward(),
         }
@@ -708,6 +701,7 @@ impl NavHistory {
         item_view: Rc<dyn WeakItemViewHandle>,
     ) {
         match self.mode {
+            NavigationMode::Disabled => {}
             NavigationMode::Normal => {
                 if self.backward_stack.len() >= MAX_NAVIGATION_HISTORY_LEN {
                     self.backward_stack.pop_front();