From e25885bbe6e3ff7af565756290173a2f60b7acc8 Mon Sep 17 00:00:00 2001 From: Dino Date: Thu, 9 Apr 2026 18:49:16 +0100 Subject: [PATCH] project_panel: Add redo and restore support (#53311) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Introduce `project_panel::Redo` action - Update all platform keymaps in order to map `redo`/`ctrl-shift-z`/`cmd-shift-z` to the `project_panel::Redo` action ### Restore Entry Support - Update both `Project::delete_entry` and `Worktree::delete_entry` to return the resulting `fs::TrashedEntry` - Introduce both `Project::restore_entry` and `Worktree::restore_entry` to allow restoring an entry in a worktree, given the `fs::TrashedEntry` - Worth pointing out that support for restoring is not yet implemented for remote worktrees, as that will be dealt with in a separate pull request ### Undo Manager - Split `ProjectPanelOperation` into two different enums, `Change` and `Operation` - While thinking through this, we noticed that simply recording the operation that user was performing was not enough, specifically in the case where undoing would restore the file, as in that specific case, we needed the `trash::TrashedEntry` in order to be able to restore, so we actually needed the result of executing the operation. - Having that in mind, we decided to separate the operation (intent) from the change (result), and record the change instead. With the change being recorded, we can easily building the operation that needs to be executed in order to invert that change. - For example, if an user creates a new file, we record the `ProjectPath` where the file was created, so that undoing can be a matter of trashing that file. When undoing, we keep track of the `trash::TrashedEntry` resulting from trashing the originally created file, such that, redoing is a matter of restoring the `trash::TrashedEntry`. - Refer to the documentation in the `project_panel::undo` module for a better breakdown on how this is implemented/handled. - Introduce a task queue for dealing with recording changes, as well as undo and redo requests in a sequential manner - This meant moving some of the details in `UndoManager` to a `project_panel::undo::Inner` implementation, and `UndoManager` now serves as a simple wrapper/client around the inner implementation, simply communicating with it to record changes and handle undo/redo requests - Callers that depend on the `UndoManager` now simply record which changes they wish to track, which are then sent to the undo manager's inner implementation - Same for the undo and redo requests, those are simply sent to the undo manager's inner implementation, which then deals with picking the correct change from the history and executing its inverse operation - Introduce support for tracking restore changes and operations - `project_panel::undo::Change::Restored` – Keeps track that the file/directory associated with the `ProjectPath` was a result of restoring a trashed entry, for which we now that reverting is simply a matter of trashing the path again - `project_panel::undo::Operation::Restore` – Keeps track of both the worktree id and the `TrashedEntry`, from which we can build the original `ProjectPath` where the trashed entry needs to be restored - Move project panel's undo tests to a separate module `project_panel::tests::undo` to avoid growing the `project::project_panel_tests` module into a monolithic test module - Some of the functions in `project::project_panel_tests` were made `pub(crate)` in order for us to be able to call those from `project_panel::tests::undo` ### FS Changes - Refactored the `Fs::trash_file` and `Fs::trash_dir` methods into a single `Fs::trash` method - This can now be done because `RealFs::trash_dir` and `RealFs::trash_file` were simply calling `trash::delete_with_info`, so we can simplify the trait - Tests have also been simplified to reflect this new change, so we no longer need a separate test for trashing a file and trashing a directory - Update `Fs::trash` and `Fs::restore` to be async - On the `RealFs` implementation we're now spawning a thread to perform the trash/restore operation Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Relates to #5039 Release Notes: - N/A --------- Co-authored-by: Yara Co-authored-by: Miguel Raz Guzmán Macedo Co-authored-by: Marshall Bowers --- Cargo.lock | 3 + Cargo.toml | 2 +- assets/keymaps/default-linux.json | 2 + assets/keymaps/default-macos.json | 1 + assets/keymaps/default-windows.json | 1 + crates/action_log/src/action_log.rs | 9 +- crates/feature_flags/src/flags.rs | 2 +- crates/fs/Cargo.toml | 1 + crates/fs/src/fs.rs | 106 ++- crates/fs/tests/integration/fs.rs | 72 +- crates/project/src/project.rs | 25 +- crates/project_panel/Cargo.toml | 2 + crates/project_panel/src/project_panel.rs | 162 ++-- .../project_panel/src/project_panel_tests.rs | 568 +------------- crates/project_panel/src/tests.rs | 1 + crates/project_panel/src/tests/undo.rs | 384 +++++++++ crates/project_panel/src/undo.rs | 740 ++++++++++++------ crates/worktree/src/worktree.rs | 126 ++- crates/worktree/tests/integration/main.rs | 9 +- 19 files changed, 1236 insertions(+), 980 deletions(-) create mode 100644 crates/project_panel/src/tests.rs create mode 100644 crates/project_panel/src/tests/undo.rs diff --git a/Cargo.lock b/Cargo.lock index 6fd8ea62fd5d5f31890fdb58e8b4cf4e8d108fa8..67495074258f02a658b5b95eb9b8e6625d6cbeb0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6482,6 +6482,7 @@ dependencies = [ "smol", "tempfile", "text", + "thiserror 2.0.17", "time", "trash", "util", @@ -13349,6 +13350,8 @@ dependencies = [ "editor", "feature_flags", "file_icons", + "fs", + "futures 0.3.32", "git", "git_ui", "gpui", diff --git a/Cargo.toml b/Cargo.toml index b9e99a5a87020a7eda8c8a2983bcf7b07fabc82c..9825c8319a7bb3440782b155d9952619096bdfd5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -571,7 +571,7 @@ encoding_rs = "0.8" exec = "0.3.1" fancy-regex = "0.17.0" fork = "0.4.0" -futures = "0.3" +futures = "0.3.32" futures-concurrency = "7.7.1" futures-lite = "1.13" gh-workflow = { git = "https://github.com/zed-industries/gh-workflow", rev = "37f3c0575d379c218a9c455ee67585184e40d43f" } diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index fb305fe768931dd6f52f1b5d890ad6771b7b5cac..6433a420b87be8cf0678dc615e7c4736eed9d0b0 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -936,6 +936,8 @@ "alt-ctrl-shift-c": "workspace::CopyRelativePath", "undo": "project_panel::Undo", "ctrl-z": "project_panel::Undo", + "redo": "project_panel::Redo", + "ctrl-shift-z": "project_panel::Redo", "enter": "project_panel::Rename", "f2": "project_panel::Rename", "backspace": ["project_panel::Trash", { "skip_prompt": false }], diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 5fb408640b2c5083f4d3379bf927178c96bed4b6..5e27dbd99e41861f0c9bace86c03d98788264d81 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -991,6 +991,7 @@ "cmd-alt-c": "workspace::CopyPath", "alt-cmd-shift-c": "workspace::CopyRelativePath", "cmd-z": "project_panel::Undo", + "cmd-shift-z": "project_panel::Redo", "enter": "project_panel::Rename", "f2": "project_panel::Rename", "backspace": ["project_panel::Trash", { "skip_prompt": false }], diff --git a/assets/keymaps/default-windows.json b/assets/keymaps/default-windows.json index 34d161577ee315857becf7c9e3c9353402e56876..1e33a71d8815e72daba07adbc76b330795ff9d52 100644 --- a/assets/keymaps/default-windows.json +++ b/assets/keymaps/default-windows.json @@ -929,6 +929,7 @@ "shift-alt-c": "project_panel::CopyPath", "ctrl-k ctrl-shift-c": "workspace::CopyRelativePath", "ctrl-z": "project_panel::Undo", + "ctrl-shift-z": "project_panel::Redo", "enter": "project_panel::Rename", "f2": "project_panel::Rename", "backspace": ["project_panel::Trash", { "skip_prompt": false }], diff --git a/crates/action_log/src/action_log.rs b/crates/action_log/src/action_log.rs index 1f17d38f7d2a2770350026f2f145a53723ef7481..cd17392704e1c6c932a3e4d8716b1c6f37489576 100644 --- a/crates/action_log/src/action_log.rs +++ b/crates/action_log/src/action_log.rs @@ -777,7 +777,7 @@ impl ActionLog { initial_version == current_version && current_content == tracked_content; if is_ai_only_content { - buffer + let task = buffer .read(cx) .entry_id(cx) .and_then(|entry_id| { @@ -785,7 +785,12 @@ impl ActionLog { project.delete_entry(entry_id, false, cx) }) }) - .unwrap_or(Task::ready(Ok(()))) + .unwrap_or_else(|| Task::ready(Ok(None))); + + cx.background_spawn(async move { + task.await?; + Ok(()) + }) } else { // Not sure how to disentangle edits made by the user // from edits made by the AI at this point. diff --git a/crates/feature_flags/src/flags.rs b/crates/feature_flags/src/flags.rs index 4a206a2bb4c48db951f1364d1aa408947165c24b..474f5b35bb536349ce7c4693f5dbedd6ef8b474a 100644 --- a/crates/feature_flags/src/flags.rs +++ b/crates/feature_flags/src/flags.rs @@ -63,6 +63,6 @@ impl FeatureFlag for ProjectPanelUndoRedoFeatureFlag { const NAME: &'static str = "project-panel-undo-redo"; fn enabled_for_staff() -> bool { - false + true } } diff --git a/crates/fs/Cargo.toml b/crates/fs/Cargo.toml index f8c5ae9169972137fa922606a6e5428131c01e63..e7b8dcd4ebda7810ef8087e112ae43819702bdf6 100644 --- a/crates/fs/Cargo.toml +++ b/crates/fs/Cargo.toml @@ -32,6 +32,7 @@ parking_lot.workspace = true paths.workspace = true rope.workspace = true proto.workspace = true +thiserror.workspace = true serde.workspace = true serde_json.workspace = true smol.workspace = true diff --git a/crates/fs/src/fs.rs b/crates/fs/src/fs.rs index bdeb139088bf33e1251bc23a5583a0ee3c9f4bf2..e44f557646239da5dd84354e364422cf16e14233 100644 --- a/crates/fs/src/fs.rs +++ b/crates/fs/src/fs.rs @@ -115,21 +115,16 @@ pub trait Fs: Send + Sync { /// system trash. async fn remove_dir(&self, path: &Path, options: RemoveOptions) -> Result<()>; - /// Moves a directory to the system trash. + /// Moves a file or directory to the system trash. /// Returns a [`TrashedEntry`] that can be used to keep track of the - /// location of the trashed directory in the system's trash. - async fn trash_dir(&self, path: &Path) -> Result; + /// location of the trashed item in the system's trash. + async fn trash(&self, path: &Path, options: RemoveOptions) -> Result; /// Removes a file from the filesystem. /// There is no expectation that the file will be preserved in the system /// trash. async fn remove_file(&self, path: &Path, options: RemoveOptions) -> Result<()>; - /// Moves a file to the system trash. - /// Returns a [`TrashedEntry`] that can be used to keep track of the - /// location of the trashed file in the system's trash. - async fn trash_file(&self, path: &Path) -> Result; - async fn open_handle(&self, path: &Path) -> Result>; async fn open_sync(&self, path: &Path) -> Result>; async fn load(&self, path: &Path) -> Result { @@ -175,7 +170,7 @@ pub trait Fs: Send + Sync { async fn restore( &self, trashed_entry: TrashedEntry, - ) -> std::result::Result<(), TrashRestoreError>; + ) -> std::result::Result; #[cfg(feature = "test-support")] fn as_fake(&self) -> Arc { @@ -188,7 +183,7 @@ pub trait Fs: Send + Sync { // tests from changes to that crate's API surface. /// Represents a file or directory that has been moved to the system trash, /// retaining enough information to restore it to its original location. -#[derive(Clone, PartialEq)] +#[derive(Clone, PartialEq, Debug)] pub struct TrashedEntry { /// Platform-specific identifier for the file/directory in the trash. /// @@ -196,9 +191,9 @@ pub struct TrashedEntry { /// * macOS & Windows – Full path to the file/directory in the system's /// trash. pub id: OsString, - /// Original name of the file/directory before it was moved to the trash. + /// Name of the file/directory at the time of trashing, including extension. pub name: OsString, - /// Original parent directory. + /// Absolute path to the parent directory at the time of trashing. pub original_parent: PathBuf, } @@ -225,13 +220,13 @@ impl TrashedEntry { } } -#[derive(Debug)] +#[derive(Debug, thiserror::Error)] pub enum TrashRestoreError { - /// The specified `path` was not found in the system's trash. + #[error("The specified `path` ({}) was not found in the system's trash.", path.display())] NotFound { path: PathBuf }, - /// A file or directory already exists at the restore destination. + #[error("File or directory ({}) already exists at the restore destination.", path.display())] Collision { path: PathBuf }, - /// Any other platform-specific error. + #[error("Unknown error ({description})")] Unknown { description: String }, } @@ -801,12 +796,26 @@ impl Fs for RealFs { } } - async fn trash_file(&self, path: &Path) -> Result { - Ok(trash::delete_with_info(path)?.into()) - } + async fn trash(&self, path: &Path, _options: RemoveOptions) -> Result { + // We must make the path absolute or trash will make a weird abomination + // of the zed working directory (not usually the worktree) and whatever + // the path variable holds. + let path = self + .canonicalize(path) + .await + .context("Could not canonicalize the path of the file")?; + + let (tx, rx) = futures::channel::oneshot::channel(); + std::thread::Builder::new() + .name("trash file or dir".to_string()) + .spawn(|| tx.send(trash::delete_with_info(path))) + .expect("The os can spawn threads"); - async fn trash_dir(&self, path: &Path) -> Result { - self.trash_file(path).await + Ok(rx + .await + .context("Tx dropped or fs.restore panicked")? + .context("Could not trash file or dir")? + .into()) } async fn open_sync(&self, path: &Path) -> Result> { @@ -1258,8 +1267,19 @@ impl Fs for RealFs { async fn restore( &self, trashed_entry: TrashedEntry, - ) -> std::result::Result<(), TrashRestoreError> { - trash::restore_all([trashed_entry.into_trash_item()]).map_err(Into::into) + ) -> std::result::Result { + let restored_item_path = trashed_entry.original_parent.join(&trashed_entry.name); + + let (tx, rx) = futures::channel::oneshot::channel(); + std::thread::Builder::new() + .name("restore trashed item".to_string()) + .spawn(move || { + let res = trash::restore_all([trashed_entry.into_trash_item()]); + tx.send(res) + }) + .expect("The OS can spawn a threads"); + rx.await.expect("Restore all never panics")?; + Ok(restored_item_path) } } @@ -2794,16 +2814,17 @@ impl Fs for FakeFs { self.remove_dir_inner(path, options).await.map(|_| ()) } - async fn trash_dir(&self, path: &Path) -> Result { + async fn trash(&self, path: &Path, options: RemoveOptions) -> Result { let normalized_path = normalize_path(path); let parent_path = normalized_path.parent().context("cannot remove the root")?; let base_name = normalized_path.file_name().unwrap(); - let options = RemoveOptions { - recursive: true, - ..Default::default() + let result = if self.is_dir(path).await { + self.remove_dir_inner(path, options).await? + } else { + self.remove_file_inner(path, options).await? }; - match self.remove_dir_inner(path, options).await? { + match result { Some(fake_entry) => { let trashed_entry = TrashedEntry { id: base_name.to_str().unwrap().into(), @@ -2823,27 +2844,6 @@ impl Fs for FakeFs { self.remove_file_inner(path, options).await.map(|_| ()) } - async fn trash_file(&self, path: &Path) -> Result { - let normalized_path = normalize_path(path); - let parent_path = normalized_path.parent().context("cannot remove the root")?; - let base_name = normalized_path.file_name().unwrap(); - - match self.remove_file_inner(path, Default::default()).await? { - Some(fake_entry) => { - let trashed_entry = TrashedEntry { - id: base_name.to_str().unwrap().into(), - name: base_name.to_str().unwrap().into(), - original_parent: parent_path.to_path_buf(), - }; - - let mut state = self.state.lock(); - state.trash.push((trashed_entry.clone(), fake_entry)); - Ok(trashed_entry) - } - None => anyhow::bail!("{normalized_path:?} does not exist"), - } - } - async fn open_sync(&self, path: &Path) -> Result> { let bytes = self.load_internal(path).await?; Ok(Box::new(io::Cursor::new(bytes))) @@ -3092,10 +3092,7 @@ impl Fs for FakeFs { receiver } - async fn restore( - &self, - trashed_entry: TrashedEntry, - ) -> std::result::Result<(), TrashRestoreError> { + async fn restore(&self, trashed_entry: TrashedEntry) -> Result { let mut state = self.state.lock(); let Some((trashed_entry, fake_entry)) = state @@ -3126,7 +3123,8 @@ impl Fs for FakeFs { match result { Ok(_) => { state.trash.retain(|(entry, _)| *entry != trashed_entry); - Ok(()) + state.emit_event([(path.clone(), Some(PathEventKind::Created))]); + Ok(path) } Err(_) => { // For now we'll just assume that this failed because it was a diff --git a/crates/fs/tests/integration/fs.rs b/crates/fs/tests/integration/fs.rs index fce8a98dea64fb153cacb5998f005a5cbd5cc11a..97ec90bea09651bc888dfdea332ad6a4964ede2f 100644 --- a/crates/fs/tests/integration/fs.rs +++ b/crates/fs/tests/integration/fs.rs @@ -628,64 +628,66 @@ async fn test_realfs_symlink_loop_metadata(executor: BackgroundExecutor) { } #[gpui::test] -async fn test_fake_fs_trash_file(executor: BackgroundExecutor) { +async fn test_fake_fs_trash(executor: BackgroundExecutor) { let fs = FakeFs::new(executor.clone()); fs.insert_tree( path!("/root"), json!({ + "src": { + "file_c.txt": "File C", + "file_d.txt": "File D" + }, "file_a.txt": "File A", "file_b.txt": "File B", }), ) .await; + // Trashing a file. let root_path = PathBuf::from(path!("/root")); let path = path!("/root/file_a.txt").as_ref(); let trashed_entry = fs - .trash_file(path) + .trash(path, Default::default()) .await .expect("should be able to trash {path:?}"); assert_eq!(trashed_entry.name, "file_a.txt"); assert_eq!(trashed_entry.original_parent, root_path); - assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_b.txt"))]); + assert_eq!( + fs.files(), + vec![ + PathBuf::from(path!("/root/file_b.txt")), + PathBuf::from(path!("/root/src/file_c.txt")), + PathBuf::from(path!("/root/src/file_d.txt")) + ] + ); let trash_entries = fs.trash_entries(); assert_eq!(trash_entries.len(), 1); assert_eq!(trash_entries[0].name, "file_a.txt"); assert_eq!(trash_entries[0].original_parent, root_path); -} -#[gpui::test] -async fn test_fake_fs_trash_dir(executor: BackgroundExecutor) { - let fs = FakeFs::new(executor.clone()); - fs.insert_tree( - path!("/root"), - json!({ - "src": { - "file_a.txt": "File A", - "file_b.txt": "File B", - }, - "file_c.txt": "File C", - }), - ) - .await; - - let root_path = PathBuf::from(path!("/root")); + // Trashing a directory. let path = path!("/root/src").as_ref(); let trashed_entry = fs - .trash_dir(path) + .trash( + path, + RemoveOptions { + recursive: true, + ..Default::default() + }, + ) .await .expect("should be able to trash {path:?}"); assert_eq!(trashed_entry.name, "src"); assert_eq!(trashed_entry.original_parent, root_path); - assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]); + assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_b.txt"))]); let trash_entries = fs.trash_entries(); - assert_eq!(trash_entries.len(), 1); - assert_eq!(trash_entries[0].name, "src"); - assert_eq!(trash_entries[0].original_parent, root_path); + assert_eq!(trash_entries.len(), 2); + assert_eq!(trash_entries[1].name, "src"); + assert_eq!(trash_entries[1].original_parent, root_path); } #[gpui::test] @@ -704,8 +706,8 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) { .await; // Providing a non-existent `TrashedEntry` should result in an error. - let id: OsString = "/trash/file_c.txt".into(); - let name: OsString = "file_c.txt".into(); + let id = OsString::from("/trash/file_c.txt"); + let name = OsString::from("file_c.txt"); let original_parent = PathBuf::from(path!("/root")); let trashed_entry = TrashedEntry { id, @@ -719,7 +721,7 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) { // it as part of its list of files, restore it and verify that the list of // files and trash has been updated accordingly. let path = path!("/root/src/file_a.txt").as_ref(); - let trashed_entry = fs.trash_file(path).await.unwrap(); + let trashed_entry = fs.trash(path, Default::default()).await.unwrap(); assert_eq!(fs.trash_entries().len(), 1); assert_eq!( @@ -745,8 +747,12 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) { // Deleting and restoring a directory should also remove all of its files // but create a single trashed entry, which should be removed after // restoration. + let options = RemoveOptions { + recursive: true, + ..Default::default() + }; let path = path!("/root/src/").as_ref(); - let trashed_entry = fs.trash_dir(path).await.unwrap(); + let trashed_entry = fs.trash(path, options).await.unwrap(); assert_eq!(fs.trash_entries().len(), 1); assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]); @@ -766,7 +772,7 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) { // A collision error should be returned in case a file is being restored to // a path where a file already exists. let path = path!("/root/src/file_a.txt").as_ref(); - let trashed_entry = fs.trash_file(path).await.unwrap(); + let trashed_entry = fs.trash(path, Default::default()).await.unwrap(); assert_eq!(fs.trash_entries().len(), 1); assert_eq!( @@ -798,8 +804,12 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) { // A collision error should be returned in case a directory is being // restored to a path where a directory already exists. + let options = RemoveOptions { + recursive: true, + ..Default::default() + }; let path = path!("/root/src/").as_ref(); - let trashed_entry = fs.trash_dir(path).await.unwrap(); + let trashed_entry = fs.trash(path, options).await.unwrap(); assert_eq!(fs.trash_entries().len(), 2); assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]); diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index e992f86fd2fbc49d27f94b8bc80fe0666c162c15..abfea741aeac56bfb921560a505e11281a254fe2 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -2566,7 +2566,7 @@ impl Project { path: ProjectPath, trash: bool, cx: &mut Context, - ) -> Option>> { + ) -> Option>>> { let entry = self.entry_for_path(&path, cx)?; self.delete_entry(entry.id, trash, cx) } @@ -2577,7 +2577,7 @@ impl Project { entry_id: ProjectEntryId, trash: bool, cx: &mut Context, - ) -> Option>> { + ) -> Option>>> { let worktree = self.worktree_for_entry(entry_id, cx)?; cx.emit(Event::DeletedEntry(worktree.read(cx).id(), entry_id)); worktree.update(cx, |worktree, cx| { @@ -2585,6 +2585,27 @@ impl Project { }) } + #[inline] + pub fn restore_entry( + &self, + worktree_id: WorktreeId, + trash_entry: TrashedEntry, + cx: &mut Context<'_, Self>, + ) -> Task> { + let Some(worktree) = self.worktree_for_id(worktree_id, cx) else { + return Task::ready(Err(anyhow!("No worktree for id {worktree_id:?}"))); + }; + + cx.spawn(async move |_, cx| { + Worktree::restore_entry(trash_entry, worktree, cx) + .await + .map(|rel_path_buf| ProjectPath { + worktree_id: worktree_id, + path: Arc::from(rel_path_buf.as_rel_path()), + }) + }) + } + #[inline] pub fn expand_entry( &mut self, diff --git a/crates/project_panel/Cargo.toml b/crates/project_panel/Cargo.toml index 2192b8daf3a301d580a3cef73426f6348508a566..62ebe3eb9f5aa06bc7a1a06e611a71c8f1f6215a 100644 --- a/crates/project_panel/Cargo.toml +++ b/crates/project_panel/Cargo.toml @@ -22,6 +22,7 @@ collections.workspace = true command_palette_hooks.workspace = true editor.workspace = true file_icons.workspace = true +futures.workspace = true git_ui.workspace = true git.workspace = true gpui.workspace = true @@ -48,6 +49,7 @@ zed_actions.workspace = true telemetry.workspace = true notifications.workspace = true feature_flags.workspace = true +fs.workspace = true [dev-dependencies] client = { workspace = true, features = ["test-support"] } diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 3d10903eaa7881a75199eb6b1f981479659498f4..b409962d9fd20621ad4c1153ab723cf9e08d85a0 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -47,16 +47,16 @@ use settings::{ update_settings_file, }; use smallvec::SmallVec; -use std::ops::Neg; -use std::{any::TypeId, time::Instant}; use std::{ + any::TypeId, cell::OnceCell, cmp, collections::HashSet, + ops::Neg, ops::Range, path::{Path, PathBuf}, sync::Arc, - time::Duration, + time::{Duration, Instant}, }; use theme_settings::ThemeSettings; use ui::{ @@ -84,7 +84,7 @@ use zed_actions::{ use crate::{ project_panel_settings::ProjectPanelScrollbarProxy, - undo::{ProjectPanelOperation, UndoManager}, + undo::{Change, UndoManager}, }; const PROJECT_PANEL_KEY: &str = "ProjectPanel"; @@ -401,6 +401,8 @@ actions!( CompareMarkedFiles, /// Undoes the last file operation. Undo, + /// Redoes the last undone file operation. + Redo, ] ); @@ -861,6 +863,7 @@ impl ProjectPanel { .detach(); let scroll_handle = UniformListScrollHandle::new(); + let weak_project_panel = cx.weak_entity(); let mut this = Self { project: project.clone(), hover_scroll_task: None, @@ -896,7 +899,7 @@ impl ProjectPanel { unfolded_dir_ids: Default::default(), }, update_visible_entries_task: Default::default(), - undo_manager: UndoManager::new(workspace.weak_handle()), + undo_manager: UndoManager::new(workspace.weak_handle(), weak_project_panel, &cx), }; this.update_visible_entries(None, false, false, window, cx); @@ -1176,6 +1179,11 @@ impl ProjectPanel { "Undo", Box::new(Undo), ) + .action_disabled_when( + !self.undo_manager.can_redo(), + "Redo", + Box::new(Redo), + ) }) .when(is_remote, |menu| { menu.separator() @@ -1874,16 +1882,12 @@ impl ProjectPanel { // Record the operation if the edit was applied if new_entry.is_ok() { let operation = if let Some(old_entry) = edited_entry { - ProjectPanelOperation::Rename { - old_path: (worktree_id, old_entry.path).into(), - new_path: new_project_path, - } + Change::Renamed((worktree_id, old_entry.path).into(), new_project_path) } else { - ProjectPanelOperation::Create { - project_path: new_project_path, - } + Change::Created(new_project_path) }; - project_panel.undo_manager.record(operation); + + project_panel.undo_manager.record([operation]).log_err(); } cx.notify(); @@ -2136,9 +2140,12 @@ impl ProjectPanel { } } - pub fn undo(&mut self, _: &Undo, _window: &mut Window, cx: &mut Context) { - self.undo_manager.undo(cx); - cx.notify(); + pub fn undo(&mut self, _: &Undo, _window: &mut Window, _cx: &mut Context) { + self.undo_manager.undo().log_err(); + } + + pub fn redo(&mut self, _: &Redo, _window: &mut Window, _cx: &mut Context) { + self.undo_manager.redo().log_err(); } fn rename_impl( @@ -2331,6 +2338,7 @@ impl ProjectPanel { Some(( selection.entry_id, + selection.worktree_id, project_path.path.file_name()?.to_string(), )) }) @@ -2346,7 +2354,7 @@ impl ProjectPanel { "Are you sure you want to permanently delete" }; let prompt = match file_paths.first() { - Some((_, path)) if file_paths.len() == 1 => { + Some((_, _, path)) if file_paths.len() == 1 => { let unsaved_warning = if dirty_buffers > 0 { "\n\nIt has unsaved changes, which will be lost." } else { @@ -2361,7 +2369,7 @@ impl ProjectPanel { let truncated_path_counts = file_paths.len() - CUTOFF_POINT; let mut paths = file_paths .iter() - .map(|(_, path)| path.clone()) + .map(|(_, _, path)| path.clone()) .take(CUTOFF_POINT) .collect::>(); paths.truncate(CUTOFF_POINT); @@ -2372,7 +2380,7 @@ impl ProjectPanel { } paths } else { - file_paths.iter().map(|(_, path)| path.clone()).collect() + file_paths.iter().map(|(_, _, path)| path.clone()).collect() }; let unsaved_warning = if dirty_buffers == 0 { String::new() @@ -2409,8 +2417,11 @@ impl ProjectPanel { { return anyhow::Ok(()); } - for (entry_id, _) in file_paths { - panel + + let mut changes = Vec::new(); + + for (entry_id, worktree_id, _) in file_paths { + let trashed_entry = panel .update(cx, |panel, cx| { panel .project @@ -2418,8 +2429,19 @@ impl ProjectPanel { .context("no such entry") })?? .await?; + + // Keep track of trashed change so that we can then record + // all of the changes at once, such that undoing and redoing + // restores or trashes all files in batch. + if trash && let Some(trashed_entry) = trashed_entry { + changes.push(Change::Trashed(worktree_id, trashed_entry)); + } } panel.update_in(cx, |panel, window, cx| { + if trash { + panel.undo_manager.record(changes).log_err(); + } + if let Some(next_selection) = next_selection { panel.update_visible_entries( Some((next_selection.worktree_id, next_selection.entry_id)), @@ -3071,8 +3093,8 @@ impl ProjectPanel { enum PasteTask { Rename { task: Task>, - old_path: ProjectPath, - new_path: ProjectPath, + from: ProjectPath, + to: ProjectPath, }, Copy { task: Task>>, @@ -3089,14 +3111,14 @@ impl ProjectPanel { let clip_entry_id = clipboard_entry.entry_id; let destination: ProjectPath = (worktree_id, new_path).into(); let task = if clipboard_entries.is_cut() { - let old_path = self.project.read(cx).path_for_entry(clip_entry_id, cx)?; + let original_path = self.project.read(cx).path_for_entry(clip_entry_id, cx)?; let task = self.project.update(cx, |project, cx| { project.rename_entry(clip_entry_id, destination.clone(), cx) }); PasteTask::Rename { task, - old_path, - new_path: destination, + from: original_path, + to: destination, } } else { let task = self.project.update(cx, |project, cx| { @@ -3113,21 +3135,16 @@ impl ProjectPanel { cx.spawn_in(window, async move |project_panel, mut cx| { let mut last_succeed = None; - let mut operations = Vec::new(); + let mut changes = Vec::new(); for task in paste_tasks { match task { - PasteTask::Rename { - task, - old_path, - new_path, - } => { + PasteTask::Rename { task, from, to } => { if let Some(CreatedEntry::Included(entry)) = task .await .notify_workspace_async_err(workspace.clone(), &mut cx) { - operations - .push(ProjectPanelOperation::Rename { old_path, new_path }); + changes.push(Change::Renamed(from, to)); last_succeed = Some(entry); } } @@ -3136,9 +3153,7 @@ impl ProjectPanel { .await .notify_workspace_async_err(workspace.clone(), &mut cx) { - operations.push(ProjectPanelOperation::Create { - project_path: destination, - }); + changes.push(Change::Created(destination)); last_succeed = Some(entry); } } @@ -3147,7 +3162,7 @@ impl ProjectPanel { project_panel .update(cx, |this, _| { - this.undo_manager.record_batch(operations); + this.undo_manager.record(changes).log_err(); }) .ok(); @@ -4371,6 +4386,20 @@ impl ProjectPanel { this.marked_entries.clear(); this.update_visible_entries(new_selection, false, false, window, cx); } + + let changes: Vec = opened_entries + .iter() + .filter_map(|entry_id| { + worktree.read(cx).entry_for_id(*entry_id).map(|entry| { + Change::Created(ProjectPath { + worktree_id, + path: entry.path.clone(), + }) + }) + }) + .collect(); + + this.undo_manager.record(changes).log_err(); }) } .log_err() @@ -4449,33 +4478,30 @@ impl ProjectPanel { cx.spawn_in(window, async move |project_panel, cx| { let mut last_succeed = None; - let mut operations = Vec::new(); + let mut changes = Vec::new(); for task in copy_tasks.into_iter() { if let Some(Some(entry)) = task.await.log_err() { last_succeed = Some(entry.id); - operations.push(ProjectPanelOperation::Create { - project_path: (worktree_id, entry.path).into(), - }); + changes.push(Change::Created((worktree_id, entry.path).into())); } } // update selection if let Some(entry_id) = last_succeed { - project_panel - .update_in(cx, |project_panel, window, cx| { - project_panel.selection = Some(SelectedEntry { - worktree_id, - entry_id, - }); - - project_panel.undo_manager.record_batch(operations); + project_panel.update_in(cx, |project_panel, window, cx| { + project_panel.selection = Some(SelectedEntry { + worktree_id, + entry_id, + }); + // if only one entry was dragged and it was disambiguated, open the rename editor + if item_count == 1 && disambiguation_range.is_some() { + project_panel.rename_impl(disambiguation_range, window, cx); + } - // if only one entry was dragged and it was disambiguated, open the rename editor - if item_count == 1 && disambiguation_range.is_some() { - project_panel.rename_impl(disambiguation_range, window, cx); - } - }) - .ok(); + project_panel.undo_manager.record(changes) + })??; } + + std::result::Result::Ok::<(), anyhow::Error>(()) }) .detach(); Some(()) @@ -4551,7 +4577,7 @@ impl ProjectPanel { let workspace = self.workspace.clone(); if folded_selection_info.is_empty() { cx.spawn_in(window, async move |project_panel, mut cx| { - let mut operations = Vec::new(); + let mut changes = Vec::new(); for (entry_id, task) in move_tasks { if let Some(CreatedEntry::Included(new_entry)) = task .await @@ -4560,16 +4586,16 @@ impl ProjectPanel { if let (Some(old_path), Some(worktree_id)) = (old_paths.get(&entry_id), destination_worktree_id) { - operations.push(ProjectPanelOperation::Rename { - old_path: old_path.clone(), - new_path: (worktree_id, new_entry.path).into(), - }); + changes.push(Change::Renamed( + old_path.clone(), + (worktree_id, new_entry.path).into(), + )); } } } project_panel .update(cx, |this, _| { - this.undo_manager.record_batch(operations); + this.undo_manager.record(changes).log_err(); }) .ok(); }) @@ -4587,10 +4613,10 @@ impl ProjectPanel { if let (Some(old_path), Some(worktree_id)) = (old_paths.get(&entry_id), destination_worktree_id) { - operations.push(ProjectPanelOperation::Rename { - old_path: old_path.clone(), - new_path: (worktree_id, new_entry.path.clone()).into(), - }); + operations.push(Change::Renamed( + old_path.clone(), + (worktree_id, new_entry.path.clone()).into(), + )); } move_results.push((entry_id, new_entry)); } @@ -4602,7 +4628,7 @@ impl ProjectPanel { project_panel .update(cx, |this, _| { - this.undo_manager.record_batch(operations); + this.undo_manager.record(operations).log_err(); }) .ok(); @@ -6640,6 +6666,7 @@ impl Render for ProjectPanel { .on_action(cx.listener(Self::compare_marked_files)) .when(cx.has_flag::(), |el| { el.on_action(cx.listener(Self::undo)) + .on_action(cx.listener(Self::redo)) }) .when(!project.is_read_only(cx), |el| { el.on_action(cx.listener(Self::new_file)) @@ -7333,3 +7360,4 @@ fn git_status_indicator(git_status: GitSummary) -> Option<(&'static str, Color)> #[cfg(test)] mod project_panel_tests; +mod tests; diff --git a/crates/project_panel/src/project_panel_tests.rs b/crates/project_panel/src/project_panel_tests.rs index a49b32a694620d4313d4496390d21d85839e4230..db4e8ba3b6b0d7733bf6bee0cf778aa5f5281d40 100644 --- a/crates/project_panel/src/project_panel_tests.rs +++ b/crates/project_panel/src/project_panel_tests.rs @@ -1,4 +1,5 @@ use super::*; +// use crate::undo::tests::{build_create_operation, build_rename_operation}; use collections::HashSet; use editor::MultiBufferOffset; use gpui::{Empty, Entity, TestAppContext, VisualTestContext}; @@ -1994,555 +1995,6 @@ async fn test_copy_paste_nested_and_root_entries(cx: &mut gpui::TestAppContext) ); } -#[gpui::test] -async fn test_undo_rename(cx: &mut gpui::TestAppContext) { - init_test(cx); - - let fs = FakeFs::new(cx.executor()); - fs.insert_tree( - "/root", - json!({ - "a.txt": "", - "b.txt": "", - }), - ) - .await; - - let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; - let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - let workspace = window - .read_with(cx, |mw, _| mw.workspace().clone()) - .unwrap(); - let cx = &mut VisualTestContext::from_window(window.into(), cx); - let panel = workspace.update_in(cx, ProjectPanel::new); - cx.run_until_parked(); - - select_path(&panel, "root/a.txt", cx); - panel.update_in(cx, |panel, window, cx| panel.rename(&Rename, window, cx)); - cx.run_until_parked(); - - let confirm = panel.update_in(cx, |panel, window, cx| { - panel - .filename_editor - .update(cx, |editor, cx| editor.set_text("renamed.txt", window, cx)); - panel.confirm_edit(true, window, cx).unwrap() - }); - confirm.await.unwrap(); - cx.run_until_parked(); - - assert!( - find_project_entry(&panel, "root/renamed.txt", cx).is_some(), - "File should be renamed to renamed.txt" - ); - assert_eq!( - find_project_entry(&panel, "root/a.txt", cx), - None, - "Original file should no longer exist" - ); - - panel.update_in(cx, |panel, window, cx| { - panel.undo(&Undo, window, cx); - }); - cx.run_until_parked(); - - assert!( - find_project_entry(&panel, "root/a.txt", cx).is_some(), - "File should be restored to original name after undo" - ); - assert_eq!( - find_project_entry(&panel, "root/renamed.txt", cx), - None, - "Renamed file should no longer exist after undo" - ); -} - -#[gpui::test] -async fn test_undo_create_file(cx: &mut gpui::TestAppContext) { - init_test(cx); - - let fs = FakeFs::new(cx.executor()); - fs.insert_tree( - "/root", - json!({ - "existing.txt": "", - }), - ) - .await; - - let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; - let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - let workspace = window - .read_with(cx, |mw, _| mw.workspace().clone()) - .unwrap(); - let cx = &mut VisualTestContext::from_window(window.into(), cx); - let panel = workspace.update_in(cx, ProjectPanel::new); - cx.run_until_parked(); - - select_path(&panel, "root", cx); - panel.update_in(cx, |panel, window, cx| panel.new_file(&NewFile, window, cx)); - cx.run_until_parked(); - - let confirm = panel.update_in(cx, |panel, window, cx| { - panel - .filename_editor - .update(cx, |editor, cx| editor.set_text("new.txt", window, cx)); - panel.confirm_edit(true, window, cx).unwrap() - }); - confirm.await.unwrap(); - cx.run_until_parked(); - - assert!( - find_project_entry(&panel, "root/new.txt", cx).is_some(), - "New file should exist" - ); - - panel.update_in(cx, |panel, window, cx| { - panel.undo(&Undo, window, cx); - }); - cx.run_until_parked(); - - assert_eq!( - find_project_entry(&panel, "root/new.txt", cx), - None, - "New file should be removed after undo" - ); - assert!( - find_project_entry(&panel, "root/existing.txt", cx).is_some(), - "Existing file should still be present" - ); -} - -#[gpui::test] -async fn test_undo_create_directory(cx: &mut gpui::TestAppContext) { - init_test(cx); - - let fs = FakeFs::new(cx.executor()); - fs.insert_tree( - "/root", - json!({ - "existing.txt": "", - }), - ) - .await; - - let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; - let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - let workspace = window - .read_with(cx, |mw, _| mw.workspace().clone()) - .unwrap(); - let cx = &mut VisualTestContext::from_window(window.into(), cx); - let panel = workspace.update_in(cx, ProjectPanel::new); - cx.run_until_parked(); - - select_path(&panel, "root", cx); - panel.update_in(cx, |panel, window, cx| { - panel.new_directory(&NewDirectory, window, cx) - }); - cx.run_until_parked(); - - let confirm = panel.update_in(cx, |panel, window, cx| { - panel - .filename_editor - .update(cx, |editor, cx| editor.set_text("new_dir", window, cx)); - panel.confirm_edit(true, window, cx).unwrap() - }); - confirm.await.unwrap(); - cx.run_until_parked(); - - assert!( - find_project_entry(&panel, "root/new_dir", cx).is_some(), - "New directory should exist" - ); - - panel.update_in(cx, |panel, window, cx| { - panel.undo(&Undo, window, cx); - }); - cx.run_until_parked(); - - assert_eq!( - find_project_entry(&panel, "root/new_dir", cx), - None, - "New directory should be removed after undo" - ); -} - -#[gpui::test] -async fn test_undo_cut_paste(cx: &mut gpui::TestAppContext) { - init_test(cx); - - let fs = FakeFs::new(cx.executor()); - fs.insert_tree( - "/root", - json!({ - "src": { - "file.txt": "content", - }, - "dst": {}, - }), - ) - .await; - - let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; - let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - let workspace = window - .read_with(cx, |mw, _| mw.workspace().clone()) - .unwrap(); - let cx = &mut VisualTestContext::from_window(window.into(), cx); - let panel = workspace.update_in(cx, ProjectPanel::new); - cx.run_until_parked(); - - toggle_expand_dir(&panel, "root/src", cx); - - select_path_with_mark(&panel, "root/src/file.txt", cx); - panel.update_in(cx, |panel, window, cx| { - panel.cut(&Default::default(), window, cx); - }); - - select_path(&panel, "root/dst", cx); - panel.update_in(cx, |panel, window, cx| { - panel.paste(&Default::default(), window, cx); - }); - cx.run_until_parked(); - - assert!( - find_project_entry(&panel, "root/dst/file.txt", cx).is_some(), - "File should be moved to dst" - ); - assert_eq!( - find_project_entry(&panel, "root/src/file.txt", cx), - None, - "File should no longer be in src" - ); - - panel.update_in(cx, |panel, window, cx| { - panel.undo(&Undo, window, cx); - }); - cx.run_until_parked(); - - assert!( - find_project_entry(&panel, "root/src/file.txt", cx).is_some(), - "File should be back in src after undo" - ); - assert_eq!( - find_project_entry(&panel, "root/dst/file.txt", cx), - None, - "File should no longer be in dst after undo" - ); -} - -#[gpui::test] -async fn test_undo_drag_single_entry(cx: &mut gpui::TestAppContext) { - init_test(cx); - - let fs = FakeFs::new(cx.executor()); - fs.insert_tree( - "/root", - json!({ - "src": { - "main.rs": "", - }, - "dst": {}, - }), - ) - .await; - - let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; - let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - let workspace = window - .read_with(cx, |mw, _| mw.workspace().clone()) - .unwrap(); - let cx = &mut VisualTestContext::from_window(window.into(), cx); - let panel = workspace.update_in(cx, ProjectPanel::new); - cx.run_until_parked(); - - toggle_expand_dir(&panel, "root/src", cx); - - panel.update(cx, |panel, _| panel.marked_entries.clear()); - select_path_with_mark(&panel, "root/src/main.rs", cx); - drag_selection_to(&panel, "root/dst", false, cx); - - assert!( - find_project_entry(&panel, "root/dst/main.rs", cx).is_some(), - "File should be in dst after drag" - ); - assert_eq!( - find_project_entry(&panel, "root/src/main.rs", cx), - None, - "File should no longer be in src after drag" - ); - - panel.update_in(cx, |panel, window, cx| { - panel.undo(&Undo, window, cx); - }); - cx.run_until_parked(); - - assert!( - find_project_entry(&panel, "root/src/main.rs", cx).is_some(), - "File should be back in src after undo" - ); - assert_eq!( - find_project_entry(&panel, "root/dst/main.rs", cx), - None, - "File should no longer be in dst after undo" - ); -} - -#[gpui::test] -async fn test_undo_drag_multiple_entries(cx: &mut gpui::TestAppContext) { - init_test(cx); - - let fs = FakeFs::new(cx.executor()); - fs.insert_tree( - "/root", - json!({ - "src": { - "alpha.txt": "", - "beta.txt": "", - }, - "dst": {}, - }), - ) - .await; - - let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; - let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - let workspace = window - .read_with(cx, |mw, _| mw.workspace().clone()) - .unwrap(); - let cx = &mut VisualTestContext::from_window(window.into(), cx); - let panel = workspace.update_in(cx, ProjectPanel::new); - cx.run_until_parked(); - - toggle_expand_dir(&panel, "root/src", cx); - - panel.update(cx, |panel, _| panel.marked_entries.clear()); - select_path_with_mark(&panel, "root/src/alpha.txt", cx); - select_path_with_mark(&panel, "root/src/beta.txt", cx); - drag_selection_to(&panel, "root/dst", false, cx); - - assert!( - find_project_entry(&panel, "root/dst/alpha.txt", cx).is_some(), - "alpha.txt should be in dst after drag" - ); - assert!( - find_project_entry(&panel, "root/dst/beta.txt", cx).is_some(), - "beta.txt should be in dst after drag" - ); - - // A single undo should revert the entire batch - panel.update_in(cx, |panel, window, cx| { - panel.undo(&Undo, window, cx); - }); - cx.run_until_parked(); - - assert!( - find_project_entry(&panel, "root/src/alpha.txt", cx).is_some(), - "alpha.txt should be back in src after undo" - ); - assert!( - find_project_entry(&panel, "root/src/beta.txt", cx).is_some(), - "beta.txt should be back in src after undo" - ); - assert_eq!( - find_project_entry(&panel, "root/dst/alpha.txt", cx), - None, - "alpha.txt should no longer be in dst after undo" - ); - assert_eq!( - find_project_entry(&panel, "root/dst/beta.txt", cx), - None, - "beta.txt should no longer be in dst after undo" - ); -} - -#[gpui::test] -async fn test_multiple_sequential_undos(cx: &mut gpui::TestAppContext) { - init_test(cx); - - let fs = FakeFs::new(cx.executor()); - fs.insert_tree( - "/root", - json!({ - "a.txt": "", - }), - ) - .await; - - let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; - let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - let workspace = window - .read_with(cx, |mw, _| mw.workspace().clone()) - .unwrap(); - let cx = &mut VisualTestContext::from_window(window.into(), cx); - let panel = workspace.update_in(cx, ProjectPanel::new); - cx.run_until_parked(); - - select_path(&panel, "root/a.txt", cx); - panel.update_in(cx, |panel, window, cx| panel.rename(&Rename, window, cx)); - cx.run_until_parked(); - let confirm = panel.update_in(cx, |panel, window, cx| { - panel - .filename_editor - .update(cx, |editor, cx| editor.set_text("b.txt", window, cx)); - panel.confirm_edit(true, window, cx).unwrap() - }); - confirm.await.unwrap(); - cx.run_until_parked(); - - assert!(find_project_entry(&panel, "root/b.txt", cx).is_some()); - - select_path(&panel, "root", cx); - panel.update_in(cx, |panel, window, cx| panel.new_file(&NewFile, window, cx)); - cx.run_until_parked(); - let confirm = panel.update_in(cx, |panel, window, cx| { - panel - .filename_editor - .update(cx, |editor, cx| editor.set_text("c.txt", window, cx)); - panel.confirm_edit(true, window, cx).unwrap() - }); - confirm.await.unwrap(); - cx.run_until_parked(); - - assert!(find_project_entry(&panel, "root/b.txt", cx).is_some()); - assert!(find_project_entry(&panel, "root/c.txt", cx).is_some()); - - panel.update_in(cx, |panel, window, cx| { - panel.undo(&Undo, window, cx); - }); - cx.run_until_parked(); - - assert_eq!( - find_project_entry(&panel, "root/c.txt", cx), - None, - "c.txt should be removed after first undo" - ); - assert!( - find_project_entry(&panel, "root/b.txt", cx).is_some(), - "b.txt should still exist after first undo" - ); - - panel.update_in(cx, |panel, window, cx| { - panel.undo(&Undo, window, cx); - }); - cx.run_until_parked(); - - assert!( - find_project_entry(&panel, "root/a.txt", cx).is_some(), - "a.txt should be restored after second undo" - ); - assert_eq!( - find_project_entry(&panel, "root/b.txt", cx), - None, - "b.txt should no longer exist after second undo" - ); -} - -#[gpui::test] -async fn test_undo_with_empty_stack(cx: &mut gpui::TestAppContext) { - init_test(cx); - - let fs = FakeFs::new(cx.executor()); - fs.insert_tree( - "/root", - json!({ - "a.txt": "", - }), - ) - .await; - - let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; - let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - let workspace = window - .read_with(cx, |mw, _| mw.workspace().clone()) - .unwrap(); - let cx = &mut VisualTestContext::from_window(window.into(), cx); - let panel = workspace.update_in(cx, ProjectPanel::new); - cx.run_until_parked(); - - panel.update_in(cx, |panel, window, cx| { - panel.undo(&Undo, window, cx); - }); - cx.run_until_parked(); - - assert!( - find_project_entry(&panel, "root/a.txt", cx).is_some(), - "File tree should be unchanged after undo on empty stack" - ); -} - -#[gpui::test] -async fn test_undo_batch(cx: &mut gpui::TestAppContext) { - init_test(cx); - - let fs = FakeFs::new(cx.executor()); - fs.insert_tree( - "/root", - json!({ - "src": { - "main.rs": "// Code!" - } - }), - ) - .await; - - let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; - let window = cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - let workspace = window - .read_with(cx, |mw, _| mw.workspace().clone()) - .unwrap(); - let cx = &mut VisualTestContext::from_window(window.into(), cx); - let panel = workspace.update_in(cx, ProjectPanel::new); - let worktree_id = project.update(cx, |project, cx| { - project.visible_worktrees(cx).next().unwrap().read(cx).id() - }); - cx.run_until_parked(); - - // Since there currently isn't a way to both create a folder and the file - // within it as two separate operations batched under the same - // `ProjectPanelOperation::Batch` operation, we'll simply record those - // ourselves, knowing that the filesystem already has the folder and file - // being provided in the operations. - panel.update(cx, |panel, _cx| { - panel.undo_manager.record_batch(vec![ - ProjectPanelOperation::Create { - project_path: ProjectPath { - worktree_id, - path: Arc::from(rel_path("src/main.rs")), - }, - }, - ProjectPanelOperation::Create { - project_path: ProjectPath { - worktree_id, - path: Arc::from(rel_path("src/")), - }, - }, - ]); - }); - - // Ensure that `src/main.rs` is present in the filesystem before proceeding, - // otherwise this test is irrelevant. - assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/src/main.rs"))]); - assert_eq!( - fs.directories(false), - vec![ - PathBuf::from(path!("/")), - PathBuf::from(path!("/root/")), - PathBuf::from(path!("/root/src/")) - ] - ); - - panel.update_in(cx, |panel, window, cx| { - panel.undo(&Undo, window, cx); - }); - cx.run_until_parked(); - - assert_eq!(fs.files().len(), 0); - assert_eq!( - fs.directories(false), - vec![PathBuf::from(path!("/")), PathBuf::from(path!("/root/"))] - ); -} - #[gpui::test] async fn test_paste_external_paths(cx: &mut gpui::TestAppContext) { init_test(cx); @@ -7348,7 +6800,11 @@ async fn test_selection_fallback_to_next_highest_worktree(cx: &mut gpui::TestApp ); } -fn toggle_expand_dir(panel: &Entity, path: &str, cx: &mut VisualTestContext) { +pub(crate) fn toggle_expand_dir( + panel: &Entity, + path: &str, + cx: &mut VisualTestContext, +) { let path = rel_path(path); panel.update_in(cx, |panel, window, cx| { for worktree in panel.project.read(cx).worktrees(cx).collect::>() { @@ -9700,7 +9156,7 @@ async fn test_hide_hidden_entries(cx: &mut gpui::TestAppContext) { ); } -fn select_path(panel: &Entity, path: &str, cx: &mut VisualTestContext) { +pub(crate) fn select_path(panel: &Entity, path: &str, cx: &mut VisualTestContext) { let path = rel_path(path); panel.update_in(cx, |panel, window, cx| { for worktree in panel.project.read(cx).worktrees(cx).collect::>() { @@ -9722,7 +9178,11 @@ fn select_path(panel: &Entity, path: &str, cx: &mut VisualTestCont cx.run_until_parked(); } -fn select_path_with_mark(panel: &Entity, path: &str, cx: &mut VisualTestContext) { +pub(crate) fn select_path_with_mark( + panel: &Entity, + path: &str, + cx: &mut VisualTestContext, +) { let path = rel_path(path); panel.update(cx, |panel, cx| { for worktree in panel.project.read(cx).worktrees(cx).collect::>() { @@ -9810,7 +9270,7 @@ fn set_folded_active_ancestor( }); } -fn drag_selection_to( +pub(crate) fn drag_selection_to( panel: &Entity, target_path: &str, is_file: bool, @@ -9835,7 +9295,7 @@ fn drag_selection_to( cx.executor().run_until_parked(); } -fn find_project_entry( +pub(crate) fn find_project_entry( panel: &Entity, path: &str, cx: &mut VisualTestContext, diff --git a/crates/project_panel/src/tests.rs b/crates/project_panel/src/tests.rs new file mode 100644 index 0000000000000000000000000000000000000000..e726758b64b0f5fa9acec59dfa53690f7f9f6f6a --- /dev/null +++ b/crates/project_panel/src/tests.rs @@ -0,0 +1 @@ +pub(crate) mod undo; diff --git a/crates/project_panel/src/tests/undo.rs b/crates/project_panel/src/tests/undo.rs new file mode 100644 index 0000000000000000000000000000000000000000..4315a6ecb4c3d2e5b64487b15144d1956d046228 --- /dev/null +++ b/crates/project_panel/src/tests/undo.rs @@ -0,0 +1,384 @@ +#![cfg(test)] + +use collections::HashSet; +use fs::{FakeFs, Fs}; +use gpui::{Entity, VisualTestContext}; +use project::Project; +use serde_json::{Value, json}; +use std::path::Path; +use std::sync::Arc; +use workspace::MultiWorkspace; + +use crate::project_panel_tests::{self, find_project_entry, select_path}; +use crate::{NewDirectory, NewFile, ProjectPanel, Redo, Rename, Trash, Undo}; + +struct TestContext { + panel: Entity, + fs: Arc, + cx: VisualTestContext, +} + +// Using the `util::path` macro requires a string literal, which would mean that +// callers of, for example, `rename`, would now need to know about `/` and +// use `path!` in tests. +// +// As such, we define it as a function here to make the helper methods more +// ergonomic for our use case. +fn path(path: impl AsRef) -> String { + let path = path.as_ref(); + #[cfg(target_os = "windows")] + { + let mut path = path.replace("/", "\\"); + if path.starts_with("\\") { + path = format!("C:{}", &path); + } + path + } + + #[cfg(not(target_os = "windows"))] + { + path.to_string() + } +} + +impl TestContext { + async fn undo(&mut self) { + self.panel.update_in(&mut self.cx, |panel, window, cx| { + panel.undo(&Undo, window, cx); + }); + self.cx.run_until_parked(); + } + async fn redo(&mut self) { + self.panel.update_in(&mut self.cx, |panel, window, cx| { + panel.redo(&Redo, window, cx); + }); + self.cx.run_until_parked(); + } + + /// Note this only works when every file has an extension + fn assert_fs_state_is(&mut self, state: &[&str]) { + let state: HashSet<_> = state + .into_iter() + .map(|s| path(format!("/workspace/{s}"))) + .chain([path("/workspace"), path("/")]) + .map(|s| Path::new(&s).to_path_buf()) + .collect(); + + let dirs: HashSet<_> = state + .iter() + .map(|p| match p.extension() { + Some(_) => p.parent().unwrap_or(Path::new(&path("/"))).to_owned(), + None => p.clone(), + }) + .collect(); + + assert_eq!( + self.fs + .directories(true) + .into_iter() + .collect::>(), + dirs + ); + assert_eq!( + self.fs.paths(true).into_iter().collect::>(), + state + ); + } + + fn assert_exists(&mut self, file: &str) { + assert!( + find_project_entry(&self.panel, &format!("workspace/{file}"), &mut self.cx).is_some(), + "{file} should exist" + ); + } + + fn assert_not_exists(&mut self, file: &str) { + assert_eq!( + find_project_entry(&self.panel, &format!("workspace/{file}"), &mut self.cx), + None, + "{file} should not exist" + ); + } + + async fn rename(&mut self, from: &str, to: &str) { + let from = format!("workspace/{from}"); + let Self { panel, cx, .. } = self; + select_path(&panel, &from, cx); + panel.update_in(cx, |panel, window, cx| panel.rename(&Rename, window, cx)); + cx.run_until_parked(); + + let confirm = panel.update_in(cx, |panel, window, cx| { + panel + .filename_editor + .update(cx, |editor, cx| editor.set_text(to, window, cx)); + panel.confirm_edit(true, window, cx).unwrap() + }); + confirm.await.unwrap(); + cx.run_until_parked(); + } + + async fn create_file(&mut self, path: &str) { + let Self { panel, cx, .. } = self; + select_path(&panel, "workspace", cx); + panel.update_in(cx, |panel, window, cx| panel.new_file(&NewFile, window, cx)); + cx.run_until_parked(); + + let confirm = panel.update_in(cx, |panel, window, cx| { + panel + .filename_editor + .update(cx, |editor, cx| editor.set_text(path, window, cx)); + panel.confirm_edit(true, window, cx).unwrap() + }); + confirm.await.unwrap(); + cx.run_until_parked(); + } + + async fn create_directory(&mut self, path: &str) { + let Self { panel, cx, .. } = self; + + select_path(&panel, "workspace", cx); + panel.update_in(cx, |panel, window, cx| { + panel.new_directory(&NewDirectory, window, cx) + }); + cx.run_until_parked(); + + let confirm = panel.update_in(cx, |panel, window, cx| { + panel + .filename_editor + .update(cx, |editor, cx| editor.set_text(path, window, cx)); + panel.confirm_edit(true, window, cx).unwrap() + }); + confirm.await.unwrap(); + cx.run_until_parked(); + } + + /// Drags the `files` to the provided `directory`. + fn drag(&mut self, files: &[&str], directory: &str) { + self.panel + .update(&mut self.cx, |panel, _| panel.marked_entries.clear()); + files.into_iter().for_each(|file| { + project_panel_tests::select_path_with_mark( + &self.panel, + &format!("workspace/{file}"), + &mut self.cx, + ) + }); + project_panel_tests::drag_selection_to( + &self.panel, + &format!("workspace/{directory}"), + false, + &mut self.cx, + ); + } + + /// Only supports files in root (otherwise would need toggle_expand_dir). + /// For undo redo the paths themselves do not matter so this is fine + async fn cut(&mut self, file: &str) { + project_panel_tests::select_path_with_mark( + &self.panel, + &format!("workspace/{file}"), + &mut self.cx, + ); + self.panel.update_in(&mut self.cx, |panel, window, cx| { + panel.cut(&Default::default(), window, cx); + }); + } + + /// Only supports files in root (otherwise would need toggle_expand_dir). + /// For undo redo the paths themselves do not matter so this is fine + async fn paste(&mut self, directory: &str) { + select_path(&self.panel, &format!("workspace/{directory}"), &mut self.cx); + self.panel.update_in(&mut self.cx, |panel, window, cx| { + panel.paste(&Default::default(), window, cx); + }); + self.cx.run_until_parked(); + } + + async fn trash(&mut self, paths: &[&str]) { + paths.iter().for_each(|p| { + project_panel_tests::select_path_with_mark( + &self.panel, + &format!("workspace/{p}"), + &mut self.cx, + ) + }); + + self.panel.update_in(&mut self.cx, |panel, window, cx| { + panel.trash(&Trash { skip_prompt: true }, window, cx); + }); + + self.cx.run_until_parked(); + } + + /// The test tree is: + /// ```txt + /// a.txt + /// b.txt + /// ``` + /// a and b are empty, x has the text "content" inside + async fn new(cx: &mut gpui::TestAppContext) -> TestContext { + Self::new_with_tree( + cx, + json!({ + "a.txt": "", + "b.txt": "", + }), + ) + .await + } + + async fn new_with_tree(cx: &mut gpui::TestAppContext, tree: Value) -> TestContext { + project_panel_tests::init_test(cx); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree("/workspace", tree).await; + let project = Project::test(fs.clone(), ["/workspace".as_ref()], cx).await; + let window = + cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); + let workspace = window + .read_with(cx, |mw, _| mw.workspace().clone()) + .unwrap(); + let mut cx = VisualTestContext::from_window(window.into(), cx); + let panel = workspace.update_in(&mut cx, ProjectPanel::new); + cx.run_until_parked(); + + TestContext { panel, fs, cx } + } +} + +#[gpui::test] +async fn rename_undo_redo(cx: &mut gpui::TestAppContext) { + let mut cx = TestContext::new(cx).await; + + cx.rename("a.txt", "renamed.txt").await; + cx.assert_fs_state_is(&["b.txt", "renamed.txt"]); + + cx.undo().await; + cx.assert_fs_state_is(&["a.txt", "b.txt"]); + + cx.redo().await; + cx.assert_fs_state_is(&["b.txt", "renamed.txt"]); +} + +#[gpui::test] +async fn create_undo_redo(cx: &mut gpui::TestAppContext) { + let mut cx = TestContext::new(cx).await; + let path = path("/workspace/c.txt"); + + cx.create_file("c.txt").await; + cx.assert_exists("c.txt"); + + // We'll now insert some content into `c.txt` in order to ensure that, after + // undoing the trash operation, i.e., when the file is restored, the actual + // file's contents are preserved instead of a new one with the same path + // being created. + cx.fs.write(Path::new(&path), b"Hello!").await.unwrap(); + + cx.undo().await; + cx.assert_not_exists("c.txt"); + + cx.redo().await; + cx.assert_exists("c.txt"); + assert_eq!(cx.fs.load(Path::new(&path)).await.unwrap(), "Hello!"); +} + +#[gpui::test] +async fn create_dir_undo(cx: &mut gpui::TestAppContext) { + let mut cx = TestContext::new(cx).await; + + cx.create_directory("new_dir").await; + cx.assert_exists("new_dir"); + cx.undo().await; + cx.assert_not_exists("new_dir"); +} + +#[gpui::test] +async fn cut_paste_undo(cx: &mut gpui::TestAppContext) { + let mut cx = TestContext::new(cx).await; + + cx.create_directory("files").await; + cx.cut("a.txt").await; + cx.paste("files").await; + cx.assert_fs_state_is(&["b.txt", "files/", "files/a.txt"]); + + cx.undo().await; + cx.assert_fs_state_is(&["a.txt", "b.txt", "files/"]); + + cx.redo().await; + cx.assert_fs_state_is(&["b.txt", "files/", "files/a.txt"]); +} + +#[gpui::test] +async fn drag_undo_redo(cx: &mut gpui::TestAppContext) { + let mut cx = TestContext::new(cx).await; + + cx.create_directory("src").await; + cx.create_file("src/a.rs").await; + cx.assert_fs_state_is(&["a.txt", "b.txt", "src/", "src/a.rs"]); + + cx.drag(&["src/a.rs"], ""); + cx.assert_fs_state_is(&["a.txt", "b.txt", "a.rs", "src/"]); + + cx.undo().await; + cx.assert_fs_state_is(&["a.txt", "b.txt", "src/", "src/a.rs"]); + + cx.redo().await; + cx.assert_fs_state_is(&["a.txt", "b.txt", "a.rs", "src/"]); +} + +#[gpui::test] +async fn drag_multiple_undo_redo(cx: &mut gpui::TestAppContext) { + let mut cx = TestContext::new(cx).await; + + cx.create_directory("src").await; + cx.create_file("src/x.rs").await; + cx.create_file("src/y.rs").await; + + cx.drag(&["src/x.rs", "src/y.rs"], ""); + cx.assert_fs_state_is(&["a.txt", "b.txt", "x.rs", "y.rs", "src/"]); + + cx.undo().await; + cx.assert_fs_state_is(&["a.txt", "b.txt", "src/", "src/x.rs", "src/y.rs"]); + + cx.redo().await; + cx.assert_fs_state_is(&["a.txt", "b.txt", "x.rs", "y.rs", "src/"]); +} + +#[gpui::test] +async fn two_sequential_undos(cx: &mut gpui::TestAppContext) { + let mut cx = TestContext::new(cx).await; + + cx.rename("a.txt", "x.txt").await; + cx.create_file("y.txt").await; + cx.assert_fs_state_is(&["b.txt", "x.txt", "y.txt"]); + + cx.undo().await; + cx.assert_fs_state_is(&["b.txt", "x.txt"]); + + cx.undo().await; + cx.assert_fs_state_is(&["a.txt", "b.txt"]); +} + +#[gpui::test] +async fn undo_without_history(cx: &mut gpui::TestAppContext) { + let mut cx = TestContext::new(cx).await; + + // Undoing without any history should just result in the filesystem state + // remaining unchanged. + cx.undo().await; + cx.assert_fs_state_is(&["a.txt", "b.txt"]) +} + +#[gpui::test] +async fn trash_undo_redo(cx: &mut gpui::TestAppContext) { + let mut cx = TestContext::new(cx).await; + + cx.trash(&["a.txt", "b.txt"]).await; + cx.assert_fs_state_is(&[]); + + cx.undo().await; + cx.assert_fs_state_is(&["a.txt", "b.txt"]); + + cx.redo().await; + cx.assert_fs_state_is(&[]); +} diff --git a/crates/project_panel/src/undo.rs b/crates/project_panel/src/undo.rs index 3a8baa23c55db8f3572174ee667196936e633281..ca4fc9ed375a9a297955f8613fa3df9fe24d568a 100644 --- a/crates/project_panel/src/undo.rs +++ b/crates/project_panel/src/undo.rs @@ -1,286 +1,558 @@ -use anyhow::anyhow; -use gpui::{AppContext, SharedString, Task, WeakEntity}; -use project::ProjectPath; -use std::collections::VecDeque; -use ui::{App, IntoElement, Label, ParentElement, Styled, v_flex}; +//! # Undo Manager +//! +//! ## Operations and Results +//! +//! Undo and Redo actions execute an operation against the filesystem, producing +//! a result that is recorded back into the history in place of the original +//! entry. Each result is the semantic inverse of its paired operation, so the +//! cycle can repeat for continued undo and redo. +//! +//! Operations Results +//! ───────────────────────────────── ────────────────────────────────────── +//! Create(ProjectPath) → Created(ProjectPath) +//! Trash(ProjectPath) → Trashed(TrashedEntry) +//! Rename(ProjectPath, ProjectPath) → Renamed(ProjectPath, ProjectPath) +//! Restore(TrashedEntry) → Restored(ProjectPath) +//! Batch(Vec) → Batch(Vec) +//! +//! +//! ## History and Cursor +//! +//! The undo manager maintains an operation history with a cursor position (↑). +//! Recording an operation appends it to the history and advances the cursor to +//! the end. The cursor separates past entries (left of ↑) from future entries +//! (right of ↑). +//! +//! ─ **Undo**: Takes the history entry just *before* ↑, executes its inverse, +//! records the result back in its place, and moves ↑ one step to the left. +//! ─ **Redo**: Takes the history entry just *at* ↑, executes its inverse, +//! records the result back in its place, and advances ↑ one step to the right. +//! +//! +//! ## Example +//! +//! User Operation Create(src/main.rs) +//! History +//! 0 Created(src/main.rs) +//! 1 +++cursor+++ +//! +//! User Operation Rename(README.md, readme.md) +//! History +//! 0 Created(src/main.rs) +//! 1 Renamed(README.md, readme.md) +//! 2 +++cursor+++ +//! +//! User Operation Create(CONTRIBUTING.md) +//! History +//! 0 Created(src/main.rs) +//! 1 Renamed(README.md, readme.md) +//! 2 Created(CONTRIBUTING.md) ──┐ +//! 3 +++cursor+++ │(before the cursor) +//! │ +//! ┌──────────────────────────────┴─────────────────────────────────────────────┐ +//! Redoing will take the result at the cursor position, convert that into the +//! operation that can revert that result, execute that operation and replace +//! the result in the history with the new result, obtained from running the +//! inverse operation, advancing the cursor position. +//! └──────────────────────────────┬─────────────────────────────────────────────┘ +//! │ +//! │ +//! User Operation Undo v +//! Execute Created(CONTRIBUTING.md) ────────> Trash(CONTRIBUTING.md) +//! Record Trashed(TrashedEntry(1)) +//! History +//! 0 Created(src/main.rs) +//! 1 Renamed(README.md, readme.md) ─┐ +//! 2 +++cursor+++ │(before the cursor) +//! 2 Trashed(TrashedEntry(1)) │ +//! │ +//! User Operation Undo v +//! Execute Renamed(README.md, readme.md) ───> Rename(readme.md, README.md) +//! Record Renamed(readme.md, README.md) +//! History +//! 0 Created(src/main.rs) +//! 1 +++cursor+++ +//! 1 Renamed(readme.md, README.md) ─┐ (at the cursor) +//! 2 Trashed(TrashedEntry(1)) │ +//! │ +//! ┌──────────────────────────────────┴─────────────────────────────────────────┐ +//! Redoing will take the result at the cursor position, convert that into the +//! operation that can revert that result, execute that operation and replace +//! the result in the history with the new result, obtained from running the +//! inverse operation, advancing the cursor position. +//! └──────────────────────────────────┬─────────────────────────────────────────┘ +//! │ +//! │ +//! User Operation Redo v +//! Execute Renamed(readme.md, README.md) ───> Rename(README.md, readme.md) +//! Record Renamed(README.md, readme.md) +//! History +//! 0 Created(src/main.rs) +//! 1 Renamed(README.md, readme.md) +//! 2 +++cursor+++ +//! 2 Trashed(TrashedEntry(1))────┐ (at the cursor) +//! │ +//! User Operation Redo v +//! Execute Trashed(TrashedEntry(1)) ────────> Restore(TrashedEntry(1)) +//! Record Restored(ProjectPath) +//! History +//! 0 Created(src/main.rs) +//! 1 Renamed(README.md, readme.md) +//! 2 Restored(ProjectPath) +//! 2 +++cursor+++ + +//! +//! create A; A +//! rename A -> B; B +//! undo (rename B -> A) (takes 10s for some reason) B (still b cause it's hanging for 10s) +//! remove B _ +//! create B B +//! put important content in B B +//! undo manger renames (does not hang) A +//! remove A _ +//! user sad + +//! +//! create A; A +//! rename A -> B; B +//! undo (rename B -> A) (takes 10s for some reason) B (still b cause it's hanging for 10s) +//! create C B +//! -- src/c.rs +//! -- + +//! +//! create docs/files/ directory docs/files/ +//! create docs/files/a.txt docs/files/ +//! undo (rename B -> A) (takes 10s for some reason) B (still b cause it's hanging for 10s) +//! create C B +//! -- src/c.rs +//! -- + +//! List of "tainted files" that the user may not operate on + +use crate::ProjectPanel; +use anyhow::{Context, Result, anyhow}; +use fs::TrashedEntry; +use futures::channel::mpsc; +use gpui::{AppContext, AsyncApp, SharedString, Task, WeakEntity}; +use project::{ProjectPath, WorktreeId}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::{collections::VecDeque, sync::Arc}; +use ui::App; use workspace::{ Workspace, notifications::{NotificationId, simple_message_notification::MessageNotification}, }; +use worktree::CreatedEntry; -const MAX_UNDO_OPERATIONS: usize = 10_000; +enum Operation { + Trash(ProjectPath), + Rename(ProjectPath, ProjectPath), + Restore(WorktreeId, TrashedEntry), + Batch(Vec), +} -#[derive(Clone)] -pub enum ProjectPanelOperation { - Batch(Vec), - Create { - project_path: ProjectPath, - }, - Rename { - old_path: ProjectPath, - new_path: ProjectPath, - }, +impl Operation { + async fn execute(self, undo_manager: &Inner, cx: &mut AsyncApp) -> Result { + Ok(match self { + Operation::Trash(project_path) => { + let trash_entry = undo_manager.trash(&project_path, cx).await?; + Change::Trashed(project_path.worktree_id, trash_entry) + } + Operation::Rename(from, to) => { + undo_manager.rename(&from, &to, cx).await?; + Change::Renamed(from, to) + } + Operation::Restore(worktree_id, trashed_entry) => { + let project_path = undo_manager.restore(worktree_id, trashed_entry, cx).await?; + Change::Restored(project_path) + } + Operation::Batch(operations) => { + let mut res = Vec::new(); + for op in operations { + res.push(Box::pin(op.execute(undo_manager, cx)).await?); + } + Change::Batched(res) + } + }) + } } -pub struct UndoManager { +#[derive(Clone, Debug)] +pub(crate) enum Change { + Created(ProjectPath), + Trashed(WorktreeId, TrashedEntry), + Renamed(ProjectPath, ProjectPath), + Restored(ProjectPath), + Batched(Vec), +} + +impl Change { + fn to_inverse(self) -> Operation { + match self { + Change::Created(project_path) => Operation::Trash(project_path), + Change::Trashed(worktree_id, trashed_entry) => { + Operation::Restore(worktree_id, trashed_entry) + } + Change::Renamed(from, to) => Operation::Rename(to, from), + Change::Restored(project_path) => Operation::Trash(project_path), + // When inverting a batch of operations, we reverse the order of + // operations to handle dependencies between them. For example, if a + // batch contains the following order of operations: + // + // 1. Create `src/` + // 2. Create `src/main.rs` + // + // If we first tried to revert the directory creation, it would fail + // because there's still files inside the directory. + Change::Batched(changes) => { + Operation::Batch(changes.into_iter().rev().map(Change::to_inverse).collect()) + } + } + } +} + +// Imagine pressing undo 10000+ times?! +const MAX_UNDO_OPERATIONS: usize = 10_000; + +struct Inner { workspace: WeakEntity, - stack: VecDeque, - /// Maximum number of operations to keep on the undo stack. + panel: WeakEntity, + history: VecDeque, + cursor: usize, + /// Maximum number of operations to keep on the undo history. limit: usize, + can_undo: Arc, + can_redo: Arc, + rx: mpsc::Receiver, +} + +/// pls arc this +#[derive(Clone)] +pub struct UndoManager { + tx: mpsc::Sender, + can_undo: Arc, + can_redo: Arc, } impl UndoManager { - pub fn new(workspace: WeakEntity) -> Self { - Self::new_with_limit(workspace, MAX_UNDO_OPERATIONS) + pub fn new( + workspace: WeakEntity, + panel: WeakEntity, + cx: &App, + ) -> Self { + let (tx, rx) = mpsc::channel(1024); + let inner = Inner::new(workspace, panel, rx); + + let this = Self { + tx, + can_undo: Arc::clone(&inner.can_undo), + can_redo: Arc::clone(&inner.can_redo), + }; + + cx.spawn(async move |cx| inner.manage_undo_and_redo(cx.clone()).await) + .detach(); + + this } - pub fn new_with_limit(workspace: WeakEntity, limit: usize) -> Self { + pub fn undo(&mut self) -> Result<()> { + self.tx + .try_send(UndoMessage::Undo) + .context("Undo and redo task can not keep up") + } + pub fn redo(&mut self) -> Result<()> { + self.tx + .try_send(UndoMessage::Redo) + .context("Undo and redo task can not keep up") + } + pub fn record(&mut self, changes: impl IntoIterator) -> Result<()> { + self.tx + .try_send(UndoMessage::Changed(changes.into_iter().collect())) + .context("Undo and redo task can not keep up") + } + /// just for the UI, an undo may still fail if there are concurrent file + /// operations happening. + pub fn can_undo(&self) -> bool { + self.can_undo.load(Ordering::Relaxed) + } + /// just for the UI, an undo may still fail if there are concurrent file + /// operations happening. + pub fn can_redo(&self) -> bool { + self.can_redo.load(Ordering::Relaxed) + } +} + +#[derive(Debug)] +enum UndoMessage { + Changed(Vec), + Undo, + Redo, +} + +impl UndoMessage { + fn error_title(&self) -> &'static str { + match self { + UndoMessage::Changed(_) => { + "this is a bug in the manage_undo_and_redo task please report" + } + UndoMessage::Undo => "Undo failed", + UndoMessage::Redo => "Redo failed", + } + } +} + +impl Inner { + async fn manage_undo_and_redo(mut self, mut cx: AsyncApp) { + loop { + let Ok(new) = self.rx.recv().await else { + // project panel got closed + return; + }; + + let error_title = new.error_title(); + let res = match new { + UndoMessage::Changed(changes) => { + self.record(changes); + Ok(()) + } + UndoMessage::Undo => { + let res = self.undo(&mut cx).await; + let _ = self.panel.update(&mut cx, |_, cx| cx.notify()); + res + } + UndoMessage::Redo => { + let res = self.redo(&mut cx).await; + let _ = self.panel.update(&mut cx, |_, cx| cx.notify()); + res + } + }; + + if let Err(e) = res { + Self::show_error(error_title, self.workspace.clone(), e.to_string(), &mut cx); + } + + self.can_undo.store(self.can_undo(), Ordering::Relaxed); + self.can_redo.store(self.can_redo(), Ordering::Relaxed); + } + } +} + +impl Inner { + pub fn new( + workspace: WeakEntity, + panel: WeakEntity, + rx: mpsc::Receiver, + ) -> Self { + Self::new_with_limit(workspace, panel, MAX_UNDO_OPERATIONS, rx) + } + + pub fn new_with_limit( + workspace: WeakEntity, + panel: WeakEntity, + limit: usize, + rx: mpsc::Receiver, + ) -> Self { Self { workspace, + panel, + history: VecDeque::new(), + cursor: 0usize, limit, - stack: VecDeque::new(), + can_undo: Arc::new(AtomicBool::new(false)), + can_redo: Arc::new(AtomicBool::new(false)), + rx, } } pub fn can_undo(&self) -> bool { - !self.stack.is_empty() + self.cursor > 0 } - pub fn undo(&mut self, cx: &mut App) { - if let Some(operation) = self.stack.pop_back() { - let task = self.revert_operation(operation, cx); - let workspace = self.workspace.clone(); - - cx.spawn(async move |cx| { - let errors = task.await; - if !errors.is_empty() { - cx.update(|cx| { - let messages = errors - .iter() - .map(|err| SharedString::from(err.to_string())) - .collect(); - - Self::show_errors(workspace, messages, cx) - }) - } - }) - .detach(); + pub fn can_redo(&self) -> bool { + self.cursor < self.history.len() + } + + pub async fn undo(&mut self, cx: &mut AsyncApp) -> Result<()> { + if !self.can_undo() { + return Ok(()); } + + // Undo failure: + // + // History + // 0 Created(src/main.rs) + // 1 Renamed(README.md, readme.md) ─┐ + // 2 +++cursor+++ │(before the cursor) + // 2 Trashed(TrashedEntry(1)) │ + // │ + // User Operation Undo v + // Failed execute Renamed(README.md, readme.md) ───> Rename(readme.md, README.md) + // Record nothing + // History + // 0 Created(src/main.rs) + // 1 +++cursor+++ + // 1 Trashed(TrashedEntry(1)) ----- + // |(at the cursor) + // User Operation Redo v + // Execute Trashed(TrashedEntry(1)) ────────> Restore(TrashedEntry(1)) + // Record Restored(ProjectPath) + // History + // 0 Created(src/main.rs) + // 1 Restored(ProjectPath) + // 1 +++cursor+++ + + // We always want to move the cursor back regardless of whether undoing + // succeeds or fails, otherwise the cursor could end up pointing to a + // position outside of the history, as we remove the change before the + // cursor, in case undo fails. + let before_cursor = self.cursor - 1; // see docs above + self.cursor -= 1; // take a step back into the past + + // If undoing fails, the user would be in a stuck state from which + // manual intervention would likely be needed in order to undo. As such, + // we remove the change from the `history` even before attempting to + // execute its inversion. + let undo_change = self + .history + .remove(before_cursor) + .expect("we can undo") + .to_inverse() + .execute(self, cx) + .await?; + self.history.insert(before_cursor, undo_change); + Ok(()) } - pub fn record(&mut self, operation: ProjectPanelOperation) { - if self.stack.len() >= self.limit { - self.stack.pop_front(); + pub async fn redo(&mut self, cx: &mut AsyncApp) -> Result<()> { + if !self.can_redo() { + return Ok(()); } - self.stack.push_back(operation); + // If redoing fails, the user would be in a stuck state from which + // manual intervention would likely be needed in order to redo. As such, + // we remove the change from the `history` even before attempting to + // execute its inversion. + let redo_change = self + .history + .remove(self.cursor) + .expect("we can redo") + .to_inverse() + .execute(self, cx) + .await?; + self.history.insert(self.cursor, redo_change); + self.cursor += 1; + Ok(()) } - pub fn record_batch(&mut self, operations: impl IntoIterator) { - let mut operations = operations.into_iter().collect::>(); - let operation = match operations.len() { + /// Passed in changes will always be performed as a single step + pub fn record(&mut self, mut changes: Vec) { + let change = match changes.len() { 0 => return, - 1 => operations.pop().unwrap(), - _ => ProjectPanelOperation::Batch(operations), + 1 => changes.remove(0), + _ => Change::Batched(changes), }; - self.record(operation); + // When recording a new change, discard any changes that could still be + // redone. + if self.cursor < self.history.len() { + self.history.drain(self.cursor..); + } + + // Ensure that the number of recorded changes does not exceed the + // maximum amount of tracked changes. + if self.history.len() >= self.limit { + self.history.pop_front(); + } else { + self.cursor += 1; + } + + self.history.push_back(change); } - /// Attempts to revert the provided `operation`, returning a vector of errors - /// in case there was any failure while reverting the operation. - /// - /// For all operations other than [`crate::undo::ProjectPanelOperation::Batch`], a maximum - /// of one error is returned. - fn revert_operation( + async fn rename( &self, - operation: ProjectPanelOperation, - cx: &mut App, - ) -> Task> { - match operation { - ProjectPanelOperation::Create { project_path } => { - let Some(workspace) = self.workspace.upgrade() else { - return Task::ready(vec![anyhow!("Failed to obtain workspace.")]); - }; - - let result = workspace.update(cx, |workspace, cx| { - workspace.project().update(cx, |project, cx| { - let entry_id = project - .entry_for_path(&project_path, cx) - .map(|entry| entry.id) - .ok_or_else(|| anyhow!("No entry for path."))?; - - project - .delete_entry(entry_id, true, cx) - .ok_or_else(|| anyhow!("Failed to trash entry.")) - }) - }); - - let task = match result { - Ok(task) => task, - Err(err) => return Task::ready(vec![err]), - }; - - cx.spawn(async move |_| match task.await { - Ok(_) => vec![], - Err(err) => vec![err], - }) - } - ProjectPanelOperation::Rename { old_path, new_path } => { - let Some(workspace) = self.workspace.upgrade() else { - return Task::ready(vec![anyhow!("Failed to obtain workspace.")]); - }; - - let result = workspace.update(cx, |workspace, cx| { - workspace.project().update(cx, |project, cx| { - let entry_id = project - .entry_for_path(&new_path, cx) - .map(|entry| entry.id) - .ok_or_else(|| anyhow!("No entry for path."))?; - - Ok(project.rename_entry(entry_id, old_path.clone(), cx)) - }) - }); - - let task = match result { - Ok(task) => task, - Err(err) => return Task::ready(vec![err]), - }; - - cx.spawn(async move |_| match task.await { - Ok(_) => vec![], - Err(err) => vec![err], + from: &ProjectPath, + to: &ProjectPath, + cx: &mut AsyncApp, + ) -> Result { + let Some(workspace) = self.workspace.upgrade() else { + return Err(anyhow!("Failed to obtain workspace.")); + }; + + let res: Result>> = workspace.update(cx, |workspace, cx| { + workspace.project().update(cx, |project, cx| { + let entry_id = project + .entry_for_path(from, cx) + .map(|entry| entry.id) + .ok_or_else(|| anyhow!("No entry for path."))?; + + Ok(project.rename_entry(entry_id, to.clone(), cx)) + }) + }); + + res?.await + } + + async fn trash(&self, project_path: &ProjectPath, cx: &mut AsyncApp) -> Result { + let Some(workspace) = self.workspace.upgrade() else { + return Err(anyhow!("Failed to obtain workspace.")); + }; + + workspace + .update(cx, |workspace, cx| { + workspace.project().update(cx, |project, cx| { + let entry_id = project + .entry_for_path(&project_path, cx) + .map(|entry| entry.id) + .ok_or_else(|| anyhow!("No entry for path."))?; + + project + .delete_entry(entry_id, true, cx) + .ok_or_else(|| anyhow!("Worktree entry should exist")) }) - } - ProjectPanelOperation::Batch(operations) => { - // When reverting operations in a batch, we reverse the order of - // operations to handle dependencies between them. For example, - // if a batch contains the following order of operations: - // - // 1. Create `src/` - // 2. Create `src/main.rs` - // - // If we first try to revert the directory creation, it would - // fail because there's still files inside the directory. - // Operations are also reverted sequentially in order to avoid - // this same problem. - let tasks: Vec<_> = operations - .into_iter() - .rev() - .map(|operation| self.revert_operation(operation, cx)) - .collect(); - - cx.spawn(async move |_| { - let mut errors = Vec::new(); - for task in tasks { - errors.extend(task.await); - } - errors + })? + .await + .and_then(|entry| { + entry.ok_or_else(|| anyhow!("When trashing we should always get a trashentry")) + }) + } + + async fn restore( + &self, + worktree_id: WorktreeId, + trashed_entry: TrashedEntry, + cx: &mut AsyncApp, + ) -> Result { + let Some(workspace) = self.workspace.upgrade() else { + return Err(anyhow!("Failed to obtain workspace.")); + }; + + workspace + .update(cx, |workspace, cx| { + workspace.project().update(cx, |project, cx| { + project.restore_entry(worktree_id, trashed_entry, cx) }) - } - } + }) + .await } - /// Displays a notification with the list of provided errors ensuring that, - /// when more than one error is provided, which can be the case when dealing - /// with undoing a [`crate::undo::ProjectPanelOperation::Batch`], a list is - /// displayed with each of the errors, instead of a single message. - fn show_errors(workspace: WeakEntity, messages: Vec, cx: &mut App) { + /// Displays a notification with the provided `title` and `error`. + fn show_error( + title: impl Into, + workspace: WeakEntity, + error: String, + cx: &mut AsyncApp, + ) { workspace .update(cx, move |workspace, cx| { let notification_id = NotificationId::Named(SharedString::new_static("project_panel_undo")); workspace.show_notification(notification_id, cx, move |cx| { - cx.new(|cx| { - if let [err] = messages.as_slice() { - MessageNotification::new(err.to_string(), cx) - .with_title("Failed to undo Project Panel Operation") - } else { - MessageNotification::new_from_builder(cx, move |_, _| { - v_flex() - .gap_1() - .children( - messages - .iter() - .map(|message| Label::new(format!("- {message}"))), - ) - .into_any_element() - }) - .with_title("Failed to undo Project Panel Operations") - } - }) + cx.new(|cx| MessageNotification::new(error, cx).with_title(title)) }) }) .ok(); } } - -#[cfg(test)] -mod test { - use crate::{ - ProjectPanel, project_panel_tests, - undo::{ProjectPanelOperation, UndoManager}, - }; - use gpui::{Entity, TestAppContext, VisualTestContext}; - use project::{FakeFs, Project, ProjectPath}; - use std::sync::Arc; - use util::rel_path::rel_path; - use workspace::MultiWorkspace; - - struct TestContext { - project: Entity, - panel: Entity, - } - - async fn init_test(cx: &mut TestAppContext) -> TestContext { - project_panel_tests::init_test(cx); - - let fs = FakeFs::new(cx.executor()); - let project = Project::test(fs.clone(), ["/root".as_ref()], cx).await; - let window = - cx.add_window(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx)); - let workspace = window - .read_with(cx, |mw, _| mw.workspace().clone()) - .unwrap(); - let cx = &mut VisualTestContext::from_window(window.into(), cx); - let panel = workspace.update_in(cx, ProjectPanel::new); - cx.run_until_parked(); - - TestContext { project, panel } - } - - #[gpui::test] - async fn test_limit(cx: &mut TestAppContext) { - let test_context = init_test(cx).await; - let worktree_id = test_context.project.update(cx, |project, cx| { - project.visible_worktrees(cx).next().unwrap().read(cx).id() - }); - - let build_create_operation = |file_name: &str| ProjectPanelOperation::Create { - project_path: ProjectPath { - path: Arc::from(rel_path(file_name)), - worktree_id, - }, - }; - - // Since we're updating the `ProjectPanel`'s undo manager with one whose - // limit is 3 operations, we only need to create 4 operations which - // we'll record, in order to confirm that the oldest operation is - // evicted. - let operation_a = build_create_operation("file_a.txt"); - let operation_b = build_create_operation("file_b.txt"); - let operation_c = build_create_operation("file_c.txt"); - let operation_d = build_create_operation("file_d.txt"); - - test_context.panel.update(cx, move |panel, _cx| { - panel.undo_manager = UndoManager::new_with_limit(panel.workspace.clone(), 3); - panel.undo_manager.record(operation_a); - panel.undo_manager.record(operation_b); - panel.undo_manager.record(operation_c); - panel.undo_manager.record(operation_d); - - assert_eq!(panel.undo_manager.stack.len(), 3); - }); - } -} diff --git a/crates/worktree/src/worktree.rs b/crates/worktree/src/worktree.rs index 7046e4afdfd95ba341b7b87846c9e3e4520849f6..ab8d448d793fb63a1c5bb5fe1c3bb05886c0866d 100644 --- a/crates/worktree/src/worktree.rs +++ b/crates/worktree/src/worktree.rs @@ -8,7 +8,8 @@ use clock::ReplicaId; use collections::{HashMap, HashSet, VecDeque}; use encoding_rs::Encoding; use fs::{ - Fs, MTime, PathEvent, PathEventKind, RemoveOptions, Watcher, copy_recursive, read_dir_items, + Fs, MTime, PathEvent, PathEventKind, RemoveOptions, TrashedEntry, Watcher, copy_recursive, + read_dir_items, }; use futures::{ FutureExt as _, Stream, StreamExt, @@ -70,7 +71,7 @@ use text::{LineEnding, Rope}; use util::{ ResultExt, maybe, paths::{PathMatcher, PathStyle, SanitizedPath, home_dir}, - rel_path::RelPath, + rel_path::{RelPath, RelPathBuf}, }; pub use worktree_settings::WorktreeSettings; @@ -848,7 +849,7 @@ impl Worktree { entry_id: ProjectEntryId, trash: bool, cx: &mut Context, - ) -> Option>> { + ) -> Option>>> { let task = match self { Worktree::Local(this) => this.delete_entry(entry_id, trash, cx), Worktree::Remote(this) => this.delete_entry(entry_id, trash, cx), @@ -870,6 +871,20 @@ impl Worktree { Some(task) } + pub async fn restore_entry( + trash_entry: TrashedEntry, + worktree: Entity, + cx: &mut AsyncApp, + ) -> Result { + let is_local = worktree.read_with(cx, |this, _| this.is_local()); + if is_local { + LocalWorktree::restore_entry(trash_entry, worktree, cx).await + } else { + // TODO(dino): Add support for restoring entries in remote worktrees. + Err(anyhow!("Unsupported")) + } + } + fn get_children_ids_recursive(&self, path: &RelPath, ids: &mut Vec) { let children_iter = self.child_entries(path); for child in children_iter { @@ -1685,35 +1700,46 @@ impl LocalWorktree { entry_id: ProjectEntryId, trash: bool, cx: &Context, - ) -> Option>> { + ) -> Option>>> { let entry = self.entry_for_id(entry_id)?.clone(); let abs_path = self.absolutize(&entry.path); let fs = self.fs.clone(); let delete = cx.background_spawn(async move { - if entry.is_file() { - if trash { - fs.trash_file(&abs_path).await?; - } else { + let trashed_entry = match (entry.is_file(), trash) { + (true, true) => Some(fs.trash(&abs_path, Default::default()).await?), + (false, true) => Some( + fs.trash( + &abs_path, + RemoveOptions { + recursive: true, + ignore_if_not_exists: false, + }, + ) + .await?, + ), + (true, false) => { fs.remove_file(&abs_path, Default::default()).await?; + None } - } else if trash { - fs.trash_dir(&abs_path).await?; - } else { - fs.remove_dir( - &abs_path, - RemoveOptions { - recursive: true, - ignore_if_not_exists: false, - }, - ) - .await?; - } - anyhow::Ok(entry.path) + (false, false) => { + fs.remove_dir( + &abs_path, + RemoveOptions { + recursive: true, + ignore_if_not_exists: false, + }, + ) + .await?; + None + } + }; + + anyhow::Ok((trashed_entry, entry.path)) }); Some(cx.spawn(async move |this, cx| { - let path = delete.await?; + let (trashed_entry, path) = delete.await?; this.update(cx, |this, _| { this.as_local_mut() .unwrap() @@ -1721,10 +1747,39 @@ impl LocalWorktree { })? .recv() .await; - Ok(()) + + Ok(trashed_entry) })) } + pub async fn restore_entry( + trash_entry: TrashedEntry, + this: Entity, + cx: &mut AsyncApp, + ) -> Result { + let Some((fs, worktree_abs_path, path_style)) = this.read_with(cx, |this, _cx| { + let local_worktree = match this { + Worktree::Local(local_worktree) => local_worktree, + Worktree::Remote(_) => return None, + }; + + let fs = local_worktree.fs.clone(); + let path_style = local_worktree.path_style(); + Some((fs, Arc::clone(local_worktree.abs_path()), path_style)) + }) else { + return Err(anyhow!("Localworktree should not change into a remote one")); + }; + + let path_buf = fs.restore(trash_entry).await?; + let path = path_buf + .strip_prefix(worktree_abs_path) + .context("Could not strip prefix")?; + let path = RelPath::new(&path, path_style)?; + let path = path.into_owned(); + + Ok(path) + } + pub fn copy_external_entries( &self, target_directory: Arc, @@ -2092,7 +2147,7 @@ impl RemoteWorktree { entry_id: ProjectEntryId, trash: bool, cx: &Context, - ) -> Option>> { + ) -> Option>>> { let response = self.client.request(proto::DeleteProjectEntry { project_id: self.project_id, entry_id: entry_id.to_proto(), @@ -2112,6 +2167,12 @@ impl RemoteWorktree { let snapshot = &mut this.background_snapshot.lock().0; snapshot.delete_entry(entry_id); this.snapshot = snapshot.clone(); + + // TODO: How can we actually track the deleted entry when + // working in remote? We likely only need to keep this + // information on the remote side in order to support restoring + // the trashed file. + None }) })) } @@ -2578,15 +2639,14 @@ impl Snapshot { } pub fn entry_for_path(&self, path: &RelPath) -> Option<&Entry> { - self.traverse_from_path(true, true, true, path) - .entry() - .and_then(|entry| { - if entry.path.as_ref() == path { - Some(entry) - } else { - None - } - }) + let entry = self.traverse_from_path(true, true, true, path).entry(); + entry.and_then(|entry| { + if entry.path.as_ref() == path { + Some(entry) + } else { + None + } + }) } /// Resolves a path to an executable using the following heuristics: diff --git a/crates/worktree/tests/integration/main.rs b/crates/worktree/tests/integration/main.rs index 633a04ad7ac1b7cb0aea93ddcc60ca38fba5fe98..47ce5e6b0a98baab6c710cd4116bef52f45dc8a1 100644 --- a/crates/worktree/tests/integration/main.rs +++ b/crates/worktree/tests/integration/main.rs @@ -2207,7 +2207,14 @@ fn randomly_mutate_worktree( match rng.random_range(0_u32..100) { 0..=33 if entry.path.as_ref() != RelPath::empty() => { log::info!("deleting entry {:?} ({})", entry.path, entry.id.to_usize()); - worktree.delete_entry(entry.id, false, cx).unwrap() + let task = worktree + .delete_entry(entry.id, false, cx) + .unwrap_or_else(|| Task::ready(Ok(None))); + + cx.background_spawn(async move { + task.await?; + Ok(()) + }) } _ => { if entry.is_dir() {