From 255a8c5d14609745b113da921d0fe6bb20ab9db9 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 23 Mar 2022 18:45:45 +0100 Subject: [PATCH 1/3] Don't push a duplicate nav entry when changing selections via the mouse Co-Authored-By: Keith Simmons --- crates/editor/src/editor.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 49cc4abe69..1db7cc3e69 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1393,8 +1393,6 @@ impl Editor { } } - self.push_to_nav_history(newest_selection.head(), Some(end.to_point(&buffer)), cx); - let selection = Selection { id: post_inc(&mut self.next_selection_id), start, @@ -6279,6 +6277,7 @@ mod tests { editor.selected_display_ranges(cx), &[DisplayPoint::new(3, 0)..DisplayPoint::new(3, 0)] ); + assert!(nav_history.borrow_mut().pop_backward().is_none()); // Move the cursor a small distance via the mouse. // Nothing is added to the navigation history. @@ -6305,6 +6304,7 @@ mod tests { editor.selected_display_ranges(cx), &[DisplayPoint::new(5, 0)..DisplayPoint::new(5, 0)] ); + assert!(nav_history.borrow_mut().pop_backward().is_none()); editor }); From 5cd94b5b92b8835ef5220363b92dee0e0ea2c942 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 23 Mar 2022 19:05:46 +0100 Subject: [PATCH 2/3] WIP --- crates/diagnostics/src/diagnostics.rs | 4 +- crates/editor/src/items.rs | 18 +++++-- crates/search/src/project_search.rs | 4 +- crates/workspace/src/pane.rs | 69 +++++++++++++++------------ crates/workspace/src/workspace.rs | 10 ++-- 5 files changed, 61 insertions(+), 44 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 7bee815ad9..a0182902f1 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -454,9 +454,9 @@ impl workspace::Item for ProjectDiagnosticsEditor { None } - fn navigate(&mut self, data: Box, cx: &mut ViewContext) { + fn navigate(&mut self, data: Box, cx: &mut ViewContext) -> bool { self.editor - .update(cx, |editor, cx| editor.navigate(data, cx)); + .update(cx, |editor, cx| editor.navigate(data, cx)) } fn is_dirty(&self, cx: &AppContext) -> bool { diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 5971cbc07b..246a462019 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -241,7 +241,7 @@ fn deserialize_selection( } impl Item for Editor { - fn navigate(&mut self, data: Box, cx: &mut ViewContext) { + fn navigate(&mut self, data: Box, cx: &mut ViewContext) -> bool { if let Some(data) = data.downcast_ref::() { let buffer = self.buffer.read(cx).read(cx); let offset = if buffer.can_resolve(&data.anchor) { @@ -249,11 +249,19 @@ impl Item for Editor { } else { buffer.clip_offset(data.offset, Bias::Left) }; - + let newest_selection = self.newest_selection_with_snapshot::(&buffer); drop(buffer); - let nav_history = self.nav_history.take(); - self.select_ranges([offset..offset], Some(Autoscroll::Fit), cx); - self.nav_history = nav_history; + + if newest_selection.head() == offset { + false + } else { + let nav_history = self.nav_history.take(); + self.select_ranges([offset..offset], Some(Autoscroll::Fit), cx); + self.nav_history = nav_history; + true + } + } else { + false } } diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index 1302040d19..212e1c66a7 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -302,9 +302,9 @@ impl Item for ProjectSearchView { }); } - fn navigate(&mut self, data: Box, cx: &mut ViewContext) { + fn navigate(&mut self, data: Box, cx: &mut ViewContext) -> bool { self.results_editor - .update(cx, |editor, cx| editor.navigate(data, cx)); + .update(cx, |editor, cx| editor.navigate(data, cx)) } fn should_update_tab_on_event(event: &ViewEvent) -> bool { diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index e04af153ba..df30d48dbe 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -212,40 +212,47 @@ impl Pane { workspace.activate_pane(pane.clone(), cx); let to_load = pane.update(cx, |pane, cx| { - // Retrieve the weak item handle from the history. - let entry = pane.nav_history.borrow_mut().pop(mode)?; + loop { + // Retrieve the weak item handle from the history. + let entry = pane.nav_history.borrow_mut().pop(mode)?; - // If the item is still present in this pane, then activate it. - if let Some(index) = entry - .item - .upgrade(cx) - .and_then(|v| pane.index_for_item(v.as_ref())) - { - if let Some(item) = pane.active_item() { - pane.nav_history.borrow_mut().set_mode(mode); - item.deactivated(cx); - pane.nav_history + // If the item is still present in this pane, then activate it. + if let Some(index) = entry + .item + .upgrade(cx) + .and_then(|v| pane.index_for_item(v.as_ref())) + { + if let Some(item) = pane.active_item() { + pane.nav_history.borrow_mut().set_mode(mode); + item.deactivated(cx); + pane.nav_history + .borrow_mut() + .set_mode(NavigationMode::Normal); + } + + let prev_active_index = mem::replace(&mut pane.active_item_index, index); + pane.focus_active_item(cx); + let mut navigated = prev_active_index != pane.active_item_index; + if let Some(data) = entry.data { + navigated |= pane.active_item()?.navigate(data, cx); + } + + if navigated { + cx.notify(); + break None; + } + } + // If the item is no longer present in this pane, then retrieve its + // project path in order to reopen it. + else { + break pane + .nav_history .borrow_mut() - .set_mode(NavigationMode::Normal); + .paths_by_item + .get(&entry.item.id()) + .cloned() + .map(|project_path| (project_path, entry)); } - - pane.active_item_index = index; - pane.focus_active_item(cx); - if let Some(data) = entry.data { - pane.active_item()?.navigate(data, cx); - } - cx.notify(); - None - } - // If the item is no longer present in this pane, then retrieve its - // project path in order to reopen it. - else { - pane.nav_history - .borrow_mut() - .paths_by_item - .get(&entry.item.id()) - .cloned() - .map(|project_path| (project_path, entry)) } }); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index 691f17ca55..65300c5f34 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -203,7 +203,9 @@ pub struct JoinProjectParams { pub trait Item: View { fn deactivated(&mut self, _: &mut ViewContext) {} - fn navigate(&mut self, _: Box, _: &mut ViewContext) {} + fn navigate(&mut self, _: Box, _: &mut ViewContext) -> bool { + false + } fn tab_content(&self, style: &theme::Tab, cx: &AppContext) -> ElementBox; fn project_path(&self, cx: &AppContext) -> Option; fn project_entry_id(&self, cx: &AppContext) -> Option; @@ -362,7 +364,7 @@ pub trait ItemHandle: 'static + fmt::Debug { cx: &mut ViewContext, ); fn deactivated(&self, cx: &mut MutableAppContext); - fn navigate(&self, data: Box, cx: &mut MutableAppContext); + fn navigate(&self, data: Box, cx: &mut MutableAppContext) -> bool; fn id(&self) -> usize; fn to_any(&self) -> AnyViewHandle; fn is_dirty(&self, cx: &AppContext) -> bool; @@ -510,8 +512,8 @@ impl ItemHandle for ViewHandle { self.update(cx, |this, cx| this.deactivated(cx)); } - fn navigate(&self, data: Box, cx: &mut MutableAppContext) { - self.update(cx, |this, cx| this.navigate(data, cx)); + fn navigate(&self, data: Box, cx: &mut MutableAppContext) -> bool { + self.update(cx, |this, cx| this.navigate(data, cx)) } fn save(&self, project: ModelHandle, cx: &mut MutableAppContext) -> Task> { From e36104f30dbc657f3ec68476ce7dd5073d8a7232 Mon Sep 17 00:00:00 2001 From: Keith Simmons Date: Wed, 23 Mar 2022 11:32:18 -0700 Subject: [PATCH 3/3] Add navigation deduping Co-authored-by: Antonio Scandurra --- crates/editor/src/editor.rs | 2 +- crates/zed/src/zed.rs | 46 +++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 1db7cc3e69..b396337e01 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -5060,7 +5060,7 @@ impl Editor { cx.notify(); } - fn transact( + pub fn transact( &mut self, cx: &mut ViewContext, update: impl FnOnce(&mut Self, &mut ViewContext), diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index c33a96a94b..1302d54067 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -893,6 +893,52 @@ mod tests { (file3.clone(), DisplayPoint::new(0, 0)) ); + // Modify file to remove nav history location, and ensure duplicates are skipped + editor1.update(cx, |editor, cx| { + editor.select_display_ranges(&[DisplayPoint::new(15, 0)..DisplayPoint::new(15, 0)], cx) + }); + + for _ in 0..5 { + editor1.update(cx, |editor, cx| { + editor + .select_display_ranges(&[DisplayPoint::new(3, 0)..DisplayPoint::new(3, 0)], cx); + }); + editor1.update(cx, |editor, cx| { + editor.select_display_ranges( + &[DisplayPoint::new(13, 0)..DisplayPoint::new(13, 0)], + cx, + ) + }); + } + + editor1.update(cx, |editor, cx| { + editor.transact(cx, |editor, cx| { + editor.select_display_ranges( + &[DisplayPoint::new(2, 0)..DisplayPoint::new(14, 0)], + cx, + ); + editor.insert("", cx); + }) + }); + + editor1.update(cx, |editor, cx| { + editor.select_display_ranges(&[DisplayPoint::new(1, 0)..DisplayPoint::new(1, 0)], cx) + }); + workspace + .update(cx, |w, cx| Pane::go_back(w, None, cx)) + .await; + assert_eq!( + active_location(&workspace, cx), + (file1.clone(), DisplayPoint::new(2, 0)) + ); + workspace + .update(cx, |w, cx| Pane::go_back(w, None, cx)) + .await; + assert_eq!( + active_location(&workspace, cx), + (file1.clone(), DisplayPoint::new(3, 0)) + ); + fn active_location( workspace: &ViewHandle, cx: &mut TestAppContext,