diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 621b75381e1..509a8e8fd36 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -2610,6 +2610,30 @@ impl Pane { &self.toolbar } + pub fn remove_navigation_history_paths( + &mut self, + project_paths: &HashSet, + cx: &mut Context, + ) { + let open_item_ids = self + .items + .iter() + .map(|item| item.item_id()) + .collect::>(); + let mut item_ids = HashSet::default(); + self.nav_history + .for_each_entry(cx, &mut |entry, (path, _)| { + if project_paths.contains(&path) && !open_item_ids.contains(&entry.item.id()) { + item_ids.insert(entry.item.id()); + } + }); + if item_ids.is_empty() { + return; + } + self.nav_history.remove_items(&item_ids); + self.history_updated(cx); + } + pub fn handle_deleted_project_item( &mut self, entry_id: ProjectEntryId, @@ -4949,18 +4973,15 @@ impl NavHistory { pub fn remove_item(&mut self, item_id: EntityId) { let mut state = self.0.lock(); state.paths_by_item.remove(&item_id); - state - .backward_stack - .retain(|entry| entry.item.id() != item_id); - state - .forward_stack - .retain(|entry| entry.item.id() != item_id); - state - .closed_stack - .retain(|entry| entry.item.id() != item_id); - state - .tag_stack - .retain(|entry| entry.origin.item.id() != item_id && entry.target.item.id() != item_id); + state.retain_entries(|id| id != item_id); + } + + pub fn remove_items(&mut self, item_ids: &HashSet) { + let mut state = self.0.lock(); + for item_id in item_ids { + state.paths_by_item.remove(item_id); + } + state.retain_entries(|id| !item_ids.contains(&id)); } pub fn rename_item( @@ -5025,6 +5046,22 @@ impl NavHistoryState { }); } } + + fn retain_entries(&mut self, retain: impl Fn(EntityId) -> bool) { + self.backward_stack.retain(|entry| retain(entry.item.id())); + self.forward_stack.retain(|entry| retain(entry.item.id())); + self.closed_stack.retain(|entry| retain(entry.item.id())); + let old_position = self.tag_stack_pos; + let mut index = 0; + self.tag_stack.retain(|entry| { + let keep = retain(entry.origin.item.id()) && retain(entry.target.item.id()); + if index < old_position && !keep { + self.tag_stack_pos -= 1; + } + index += 1; + keep + }); + } } fn dirty_message_for(buffer_path: Option, path_style: PathStyle) -> String { @@ -5167,6 +5204,86 @@ mod tests { } } + #[gpui::test] + async fn test_remove_navigation_history_paths(cx: &mut TestAppContext) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + let project = Project::test(fs, [], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project, window, cx)); + let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); + let [live, first_removed, second_removed, retained] = std::array::from_fn(|index| { + cx.new(|cx| { + TestItem::new(cx).with_project_items(&[TestProjectItem::new( + index as u64, + &format!("{index}.rs"), + cx, + )]) + }) + }); + pane.update_in(cx, |pane, window, cx| { + pane.add_item(Box::new(live.clone()), false, false, None, window, cx); + }); + let entry = |item: &Entity| NavigationEntry { + item: Arc::new(item.downgrade()), + data: None, + timestamp: 0, + is_preview: false, + row: None, + }; + pane.update(cx, |pane, cx| { + { + let mut state = pane.nav_history.0.lock(); + let items = [&live, &first_removed, &second_removed, &retained]; + state.backward_stack = items.map(entry).into(); + state.forward_stack = state.backward_stack.clone(); + state.closed_stack = state.backward_stack.clone(); + for item in items { + state + .paths_by_item + .insert(item.entity_id(), (item.project_path(cx).unwrap(), None)); + } + } + let history = pane.nav_history_mut(); + history.push_tag(entry(&first_removed), entry(&second_removed)); + history.push_tag(entry(&live), entry(&retained)); + assert_eq!( + history.pop_tag(TagNavigationMode::Older).unwrap().item.id(), + live.entity_id() + ); + let paths = [&live, &first_removed, &second_removed] + .map(|item| item.project_path(cx).unwrap()) + .into_iter() + .collect::>(); + pane.remove_navigation_history_paths(&paths, cx); + { + let state = pane.nav_history.0.lock(); + for stack in [ + &state.backward_stack, + &state.forward_stack, + &state.closed_stack, + ] { + assert_eq!( + stack + .iter() + .map(|entry| entry.item.id()) + .collect::>(), + vec![live.entity_id(), retained.entity_id()] + ); + } + assert_eq!(state.paths_by_item.len(), 2); + } + assert_eq!( + pane.nav_history_mut() + .pop_tag(TagNavigationMode::Newer) + .unwrap() + .item + .id(), + retained.entity_id() + ); + }); + } + #[gpui::test] async fn test_add_item_capped_to_max_tabs(cx: &mut TestAppContext) { init_test(cx); diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index d15fa395770..ad224f2e4ee 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -98,8 +98,8 @@ pub use persistence::{ use persistence::{SerializedWindowBounds, model::SerializedWorkspace}; use postage::stream::Stream; use project::{ - DirectoryLister, Project, ProjectEntryId, ProjectPath, ResolvedPath, Worktree, WorktreeId, - WorktreeSettings, + DirectoryLister, PathChange, Project, ProjectEntryId, ProjectPath, ResolvedPath, + UpdatedEntriesSet, Worktree, WorktreeId, WorktreeSettings, debugger::{breakpoint_store::BreakpointStoreEvent, session::ThreadStatus}, git_store::{GitStoreEvent, RepositoryEvent}, project_settings::ProjectSettings, @@ -1774,7 +1774,8 @@ impl Workspace { this.update_history(cx); } } - project::Event::WorktreeUpdatedEntries(..) => { + project::Event::WorktreeUpdatedEntries(worktree_id, changes) => { + this.remove_deleted_navigation_history(*worktree_id, changes, cx); this.update_window_title(window, cx); this.serialize_workspace(window, cx); } @@ -3068,6 +3069,7 @@ impl Workspace { } } + recent_history.retain(|(path, _)| !self.is_deleted_history_path(path, cx)); recent_history.into_iter() } @@ -3090,6 +3092,103 @@ impl Workspace { self.serialize_workspace(window, cx); } + fn is_deleted_history_path(&self, path: &ProjectPath, cx: &App) -> bool { + let project = self.project.read(cx); + project.get_open_buffer(path, cx).is_some_and(|buffer| { + buffer + .read(cx) + .file() + .is_some_and(|file| file.disk_state().is_deleted()) + }) && project + .worktree_for_id(path.worktree_id, cx) + .is_some_and(|worktree| { + Self::history_path_is_removed(worktree.read(cx), &path.path, cx) + }) + } + + fn history_path_is_removed(worktree: &Worktree, path: &RelPath, cx: &App) -> bool { + let settings = WorktreeSettings::get( + Some(SettingsLocation { + worktree_id: worktree.id(), + path: RelPath::empty(), + }), + cx, + ); + worktree.entry_for_path(path).is_none() + && !settings.is_path_excluded(path) + && !path + .ancestors() + .skip(1) + .find_map(|ancestor| worktree.entry_for_path(ancestor)) + .is_some_and(|entry| entry.kind.is_unloaded()) + } + + fn remove_deleted_navigation_history( + &mut self, + worktree_id: WorktreeId, + changes: &UpdatedEntriesSet, + cx: &mut Context, + ) { + let project = self.project.read(cx); + let Some(worktree) = project.worktree_for_id(worktree_id, cx) else { + return; + }; + let worktree = worktree.read(cx); + let removed_paths = changes + .iter() + .filter(|(path, entry_id, change)| { + *change == PathChange::Removed + && worktree.entry_for_id(*entry_id).is_none() + && Self::history_path_is_removed(worktree, path, cx) + }) + .map(|(path, _, _)| path.clone()) + .collect::>(); + if removed_paths.is_empty() { + return; + } + + let (paths, absolute_paths): (HashSet<_>, HashSet<_>) = + self.recent_navigation_history_iter(cx) + .chain(self.persisted_recent_navigation_history.iter().filter_map( + |absolute_path| { + let path = project.project_path_for_absolute_path(absolute_path, cx)?; + Some((path, Some(absolute_path.clone()))) + }, + )) + .filter(|(path, _)| { + path.worktree_id == worktree_id + && path + .path + .ancestors() + .any(|ancestor| removed_paths.contains(ancestor)) + && worktree.entry_for_path(&path.path).is_none() + }) + .filter_map(|(path, _)| { + let absolute_path = project.absolute_path(&path, cx)?; + Some((path, absolute_path)) + }) + .unzip(); + + if paths.is_empty() { + return; + } + self.persisted_recent_navigation_history + .retain(|path| !absolute_paths.contains(path)); + for pane in &self.panes { + pane.update(cx, |pane, cx| { + pane.remove_navigation_history_paths(&paths, cx) + }); + } + if self + .last_active_project_path + .as_ref() + .is_some_and(|path| paths.contains(path)) + { + self.last_active_project_path = None; + } + cx.notify(); + } + fn rename_persisted_navigation_history_paths( &mut self, old_path: &Path, @@ -6256,6 +6355,14 @@ impl Workspace { serialize_workspace = false; } pane::Event::RemovedItem { item } => { + if item + .project_path(cx) + .is_some_and(|path| self.is_deleted_history_path(&path, cx)) + { + pane.update(cx, |pane, _| { + pane.nav_history_mut().remove_item(item.item_id()) + }); + } cx.emit(Event::ActiveItemChanged); self.update_window_edited(window, cx); if let hash_map::Entry::Occupied(entry) = self.panes_by_item.entry(item.item_id()) @@ -6750,7 +6857,10 @@ impl Workspace { if active_project_path_changed { match active_entry.as_ref() { None => self.last_active_project_path = None, - Some(path) if self.remember_navigation_history_path(path, cx) => { + Some(path) + if !self.is_deleted_history_path(path, cx) + && self.remember_navigation_history_path(path, cx) => + { self.serialize_workspace(window, cx); } Some(_) => {} @@ -12611,14 +12721,14 @@ mod tests { }, persistence::model::DockData, }; - use fs::FakeFs; + use fs::{FakeFs, RemoveOptions}; use gpui::{ DismissEvent, Empty, EventEmitter, FocusHandle, Focusable, Render, TestAppContext, UpdateGlobal, VisualTestContext, px, }; use project::{Project, ProjectEntryId, WorktreeId}; use serde_json::json; - use settings::SettingsStore; + use settings::{SettingsStore, SplicingVec}; use util::path; use util::rel_path::rel_path; @@ -19683,6 +19793,339 @@ mod tests { assert_eq!(path, None); } + #[gpui::test] + async fn test_recent_navigation_history_tracks_removed_entries(cx: &mut TestAppContext) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/project"), + json!({ + "dir": { "deleted.rs": "", "excluded.rs": "" }, + "dir2": { "retained.rs": "" }, + "replaced.rs": "old", + }), + ) + .await; + fs.insert_tree(path!("/other"), json!({ "dir": { "deleted.rs": "" } })) + .await; + let project = Project::test( + fs.clone(), + [path!("/project").as_ref(), path!("/other").as_ref()], + cx, + ) + .await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx)); + let database = cx.update(|_, cx| WorkspaceDb::global(cx)); + let workspace_id = database.next_id().await.unwrap(); + let original_history = [ + path!("/project/dir/deleted.rs"), + path!("/project/dir/excluded.rs"), + path!("/project/dir2/retained.rs"), + path!("/project/replaced.rs"), + path!("/other/dir/deleted.rs"), + ] + .map(PathBuf::from) + .to_vec(); + workspace.update(cx, |workspace, _| { + workspace.set_database_id(workspace_id); + workspace.persisted_recent_navigation_history = original_history.clone(); + }); + let excluded_path = project.read_with(cx, |project, cx| { + project + .project_path_for_absolute_path(Path::new(path!("/project/dir/excluded.rs")), cx) + .unwrap() + }); + let _excluded_buffer = project + .update(cx, |project, cx| project.open_buffer(excluded_path, cx)) + .await + .unwrap(); + cx.update(|_, cx| { + cx.update_global::(|store, cx| { + store.update_user_settings(cx, |settings| { + settings.project.worktree.file_scan_exclusions = + Some(SplicingVec::from(vec!["**/excluded.rs".to_owned()])); + }); + }); + }); + cx.run_until_parked(); + project.read_with(cx, |project, cx| { + let path = project + .project_path_for_absolute_path(Path::new(path!("/project/dir/excluded.rs")), cx) + .unwrap(); + assert!(project.entry_for_path(&path, cx).is_none()); + }); + assert_eq!( + workspace.read_with(cx, |workspace, _| workspace + .persisted_recent_navigation_history + .clone()), + original_history + ); + + workspace.read_with(cx, |workspace, cx| { + assert_eq!( + workspace + .recent_navigation_history_iter(cx) + .map(|(_, path)| path.unwrap()) + .collect::>(), + original_history + ); + }); + + fs.pause_events(); + fs.remove_file( + Path::new(path!("/project/replaced.rs")), + RemoveOptions::default(), + ) + .await + .unwrap(); + fs.insert_file(path!("/project/replaced.rs"), b"new".to_vec()) + .await; + fs.unpause_events_and_flush(); + cx.run_until_parked(); + assert_eq!( + workspace.read_with(cx, |workspace, _| workspace + .persisted_recent_navigation_history + .clone()), + original_history + ); + + let (worktree_id, directory_entry_id) = project.read_with(cx, |project, cx| { + let path = project + .project_path_for_absolute_path(Path::new(path!("/project/dir")), cx) + .unwrap(); + ( + path.worktree_id, + project.entry_for_path(&path, cx).unwrap().id, + ) + }); + fs.remove_dir( + Path::new(path!("/project/dir")), + RemoveOptions { + recursive: true, + ..RemoveOptions::default() + }, + ) + .await + .unwrap(); + cx.run_until_parked(); + let expected = [ + path!("/project/dir2/retained.rs"), + path!("/project/replaced.rs"), + path!("/other/dir/deleted.rs"), + ] + .map(PathBuf::from) + .to_vec(); + workspace.read_with(cx, |workspace, cx| { + assert_eq!(workspace.persisted_recent_navigation_history, expected); + assert_eq!( + workspace + .recent_navigation_history_iter(cx) + .map(|(_, path)| path.unwrap()) + .collect::>(), + expected + ); + }); + let metadata_calls = fs.metadata_call_count(); + workspace.update(cx, |workspace, cx| { + workspace.persisted_recent_navigation_history = original_history; + workspace.remove_deleted_navigation_history( + worktree_id, + &vec![( + rel_path("dir").into(), + directory_entry_id, + PathChange::Removed, + )] + .into(), + cx, + ); + assert_eq!(workspace.persisted_recent_navigation_history, expected); + }); + cx.run_until_parked(); + assert_eq!(fs.metadata_call_count(), metadata_calls); + cx.executor().advance_clock(SERIALIZATION_THROTTLE_TIME); + cx.run_until_parked(); + assert_eq!( + database + .workspace_for_id(workspace_id) + .unwrap() + .recent_navigation_history, + expected + ); + } + + #[gpui::test] + async fn test_recent_navigation_history_preserves_live_deleted_buffers( + cx: &mut TestAppContext, + ) { + init_test(cx); + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/project"), + json!({ "a.rs": "a", "b.rs": "b", "c.rs": "c" }), + ) + .await; + let project = Project::test(fs.clone(), [path!("/project").as_ref()], cx).await; + let (workspace, cx) = + cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx)); + let path_a = project.read_with(cx, |project, cx| { + project + .project_path_for_absolute_path(Path::new(path!("/project/a.rs")), cx) + .unwrap() + }); + let buffer = project + .update(cx, |project, cx| project.open_buffer(path_a.clone(), cx)) + .await + .unwrap(); + let (entry_id_a, entry_id_b) = project.read_with(cx, |project, cx| { + let path_b = ProjectPath { + worktree_id: path_a.worktree_id, + path: rel_path("b.rs").into(), + }; + ( + project.entry_for_path(&path_a, cx).unwrap().id.to_proto(), + project.entry_for_path(&path_b, cx).unwrap().id.to_proto(), + ) + }); + let item_a = cx.new(|cx| { + TestItem::new(cx).with_dirty(true).with_project_items(&[ + TestProjectItem::new_in_worktree(entry_id_a, "a.rs", path_a.worktree_id, cx), + ]) + }); + let item_b = cx.new(|cx| { + TestItem::new(cx).with_project_items(&[TestProjectItem::new_in_worktree( + entry_id_b, + "b.rs", + path_a.worktree_id, + cx, + )]) + }); + let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone()); + workspace.update_in(cx, |workspace, window, cx| { + workspace.add_item_to_active_pane(Box::new(item_a.clone()), None, true, window, cx); + }); + workspace.update_in(cx, |workspace, window, cx| { + workspace.add_item_to_active_pane(Box::new(item_b.clone()), None, true, window, cx); + }); + workspace.update_in(cx, |workspace, window, cx| { + workspace.activate_item(&item_a, false, false, window, cx); + }); + workspace.read_with(cx, |workspace, _| { + assert_eq!( + workspace.persisted_recent_navigation_history, + vec![ + PathBuf::from(path!("/project/a.rs")), + PathBuf::from(path!("/project/b.rs")) + ] + ); + }); + fs.remove_file(Path::new(path!("/project/a.rs")), RemoveOptions::default()) + .await + .unwrap(); + cx.run_until_parked(); + let expected = vec![PathBuf::from(path!("/project/b.rs"))]; + workspace.read_with(cx, |workspace, cx| { + assert_eq!(workspace.persisted_recent_navigation_history, expected); + assert_eq!(workspace.last_active_project_path, None); + assert_eq!( + workspace + .recent_navigation_history_iter(cx) + .map(|(_, path)| path.unwrap()) + .collect::>(), + expected + ); + }); + pane.read_with(cx, |pane, cx| { + assert_eq!(pane.items_len(), 2); + let mut paths = Vec::new(); + pane.nav_history() + .for_each_entry(cx, &mut |entry, (path, _)| { + if entry.item.id() == item_a.entity_id() { + paths.push(path); + } + }); + assert_eq!(paths, vec![path_a.clone()]); + }); + workspace.update_in(cx, |workspace, window, cx| { + workspace.activate_item(&item_b, false, false, window, cx); + }); + workspace + .update_in(cx, |workspace, window, cx| { + workspace.go_back(pane.downgrade(), window, cx) + }) + .await + .unwrap(); + assert_eq!( + workspace.read_with(cx, |workspace, cx| workspace + .active_item(cx) + .unwrap() + .item_id()), + item_a.entity_id() + ); + project + .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx)) + .await + .unwrap(); + item_a.update(cx, |_, cx| cx.emit(ItemEvent::UpdateBreadcrumbs)); + workspace.read_with(cx, |workspace, _| { + assert_eq!( + workspace.persisted_recent_navigation_history, + vec![ + PathBuf::from(path!("/project/a.rs")), + PathBuf::from(path!("/project/b.rs")) + ] + ); + }); + + let path_c = ProjectPath { + worktree_id: path_a.worktree_id, + path: rel_path("c.rs").into(), + }; + let buffer_c = project + .update(cx, |project, cx| project.open_buffer(path_c.clone(), cx)) + .await + .unwrap(); + let entry_id_c = project.read_with(cx, |project, cx| { + project.entry_for_path(&path_c, cx).unwrap().id.to_proto() + }); + let item_c = cx.new(|cx| { + TestItem::new(cx).with_project_items(&[TestProjectItem::new_in_worktree( + entry_id_c, + "c.rs", + path_a.worktree_id, + cx, + )]) + }); + workspace.update_in(cx, |workspace, window, cx| { + workspace.add_item_to_active_pane(Box::new(item_c.clone()), None, true, window, cx); + }); + for path in [path!("/project/a.rs"), path!("/project/c.rs")] { + fs.remove_file(Path::new(path), RemoveOptions::default()) + .await + .unwrap(); + } + cx.run_until_parked(); + pane.update_in(cx, |pane, window, cx| { + pane.remove_item(item_a.entity_id(), false, false, window, cx); + pane.remove_item(item_c.entity_id(), false, false, window, cx); + }); + drop(buffer); + drop(buffer_c); + drop(item_a); + drop(item_c); + cx.run_until_parked(); + workspace.read_with(cx, |workspace, cx| { + assert_eq!(workspace.persisted_recent_navigation_history, expected); + assert_eq!( + workspace + .recent_navigation_history_iter(cx) + .map(|(_, path)| path.unwrap()) + .collect::>(), + expected + ); + }); + } + #[gpui::test] async fn test_recent_navigation_history_includes_persisted_paths( cx: &mut gpui::TestAppContext, diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index fe6b459a459..18a70751c19 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -5505,7 +5505,6 @@ mod tests { (file3.clone(), DisplayPoint::new(DisplayRow(0), 0), 0.) ); - // Go back to an item that has been closed and removed from disk workspace .update_in(cx, |_, window, cx| { pane.update(cx, |pane, cx| { @@ -5531,7 +5530,7 @@ mod tests { .unwrap(); assert_eq!( active_location(&workspace, cx), - (file2.clone(), DisplayPoint::new(DisplayRow(0), 0), 0.) + (file1.clone(), DisplayPoint::new(DisplayRow(10), 0), 0.) ); workspace .update_in(cx, |w, window, cx| {