feat: support trashing in remote

dino and Yara created

* Ensure that trashing a file or directory is now properly supported in
  remote, ensuring that it does return a `TrashId` that can be then used
  to keep track of the change in the Undo Manager.
* Separate `delete_entry` and `trash_entry` at both the project and
  worktree level, to make it easier to reason about and ensure we don't
  need to mess with `Option<TrashId>`, deleting will never return a
  `TrashId`.

Worth noting that undoing a trash operation in remote is not yet
supported as `RemoteWorktre::restore_entry` is not yet available.

Co-authored-by: Yara <git@yara.blue>

Change summary

crates/action_log/src/action_log.rs                  |  17 
crates/agent/src/tools/delete_path_tool.rs           |   4 
crates/collab/src/rpc.rs                             |   2 
crates/collab/tests/integration/integration_tests.rs |   4 
crates/edit_prediction/src/udiff.rs                  |   2 
crates/fs/src/fs.rs                                  |   1 
crates/git_ui/src/git_panel.rs                       |   4 
crates/project/src/project.rs                        |  38 ++
crates/project/src/worktree_store.rs                 |  23 +
crates/project_panel/src/project_panel.rs            |  49 ++-
crates/project_panel/src/undo.rs                     |  57 ++--
crates/proto/proto/worktree.proto                    |   1 
crates/worktree/src/worktree.rs                      | 166 ++++++++-----
crates/worktree/tests/integration/main.rs            |   4 
14 files changed, 223 insertions(+), 149 deletions(-)

Detailed changes

crates/action_log/src/action_log.rs 🔗

@@ -781,11 +781,10 @@ impl ActionLog {
                             .read(cx)
                             .entry_id(cx)
                             .and_then(|entry_id| {
-                                self.project.update(cx, |project, cx| {
-                                    project.delete_entry(entry_id, false, cx)
-                                })
+                                self.project
+                                    .update(cx, |project, cx| project.delete_entry(entry_id, cx))
                             })
-                            .unwrap_or_else(|| Task::ready(Ok(None)));
+                            .unwrap_or_else(|| Task::ready(Ok(())));
 
                         cx.background_spawn(async move {
                             task.await?;
@@ -1847,14 +1846,14 @@ mod tests {
         action_log.update(cx, |log, cx| log.will_delete_buffer(buffer2.clone(), cx));
         project
             .update(cx, |project, cx| {
-                project.delete_file(file1_path.clone(), false, cx)
+                project.delete_file(file1_path.clone(), cx)
             })
             .unwrap()
             .await
             .unwrap();
         project
             .update(cx, |project, cx| {
-                project.delete_file(file2_path.clone(), false, cx)
+                project.delete_file(file2_path.clone(), cx)
             })
             .unwrap()
             .await
@@ -2159,9 +2158,7 @@ mod tests {
             action_log.update(cx, |log, cx| log.will_delete_buffer(buffer.clone(), cx));
         });
         project
-            .update(cx, |project, cx| {
-                project.delete_file(file_path.clone(), false, cx)
-            })
+            .update(cx, |project, cx| project.delete_file(file_path.clone(), cx))
             .unwrap()
             .await
             .unwrap();
@@ -2990,7 +2987,7 @@ mod tests {
             child_log.update(cx, |log, cx| log.will_delete_buffer(buffer.clone(), cx));
         });
         project
-            .update(cx, |project, cx| project.delete_file(file_path, false, cx))
+            .update(cx, |project, cx| project.delete_file(file_path, cx))
             .unwrap()
             .await
             .unwrap();

crates/agent/src/tools/delete_path_tool.rs 🔗

@@ -205,9 +205,7 @@ impl AgentTool for DeletePathTool {
             }
 
             let deletion_task = project
-                .update(cx, |project, cx| {
-                    project.delete_file(project_path, false, cx)
-                })
+                .update(cx, |project, cx| project.delete_file(project_path, cx))
                 .ok_or_else(|| {
                     format!("Couldn't delete {path} because that path isn't in this project.")
                 })?;

crates/collab/src/rpc.rs 🔗

@@ -347,6 +347,7 @@ impl Server {
             .add_request_handler(forward_mutating_project_request::<proto::RenameProjectEntry>)
             .add_request_handler(forward_mutating_project_request::<proto::CopyProjectEntry>)
             .add_request_handler(forward_mutating_project_request::<proto::DeleteProjectEntry>)
+            .add_request_handler(forward_mutating_project_request::<proto::TrashProjectEntry>)
             .add_request_handler(forward_mutating_project_request::<proto::ExpandProjectEntry>)
             .add_request_handler(
                 forward_mutating_project_request::<proto::ExpandAllForProjectEntry>,
@@ -442,7 +443,6 @@ impl Server {
             .add_request_handler(forward_mutating_project_request::<proto::CheckForPushedCommits>)
             .add_request_handler(forward_mutating_project_request::<proto::ToggleLspLogs>)
             .add_message_handler(broadcast_project_message_from_host::<proto::LanguageServerLog>)
-            // TODO!(yara) add new message here
             .add_request_handler(share_agent_thread)
             .add_request_handler(get_shared_agent_thread)
             .add_request_handler(forward_project_search_chunk);

crates/collab/tests/integration/integration_tests.rs 🔗

@@ -3377,7 +3377,7 @@ async fn test_fs_operations(
 
     project_b
         .update(cx_b, |project, cx| {
-            project.delete_entry(dir_entry.id, false, cx).unwrap()
+            project.delete_entry(dir_entry.id, cx).unwrap()
         })
         .await
         .unwrap();
@@ -3405,7 +3405,7 @@ async fn test_fs_operations(
 
     project_b
         .update(cx_b, |project, cx| {
-            project.delete_entry(entry.id, false, cx).unwrap()
+            project.delete_entry(entry.id, cx).unwrap()
         })
         .await
         .unwrap();

crates/edit_prediction/src/udiff.rs 🔗

@@ -65,7 +65,7 @@ pub async fn apply_diff(
                 if status == FileStatus::Deleted {
                     let delete_task = project.update(cx, |project, cx| {
                         if let Some(path) = project.find_project_path(path.as_ref(), cx) {
-                            project.delete_file(path, false, cx)
+                            project.delete_file(path, cx)
                         } else {
                             None
                         }

crates/fs/src/fs.rs 🔗

@@ -396,7 +396,6 @@ impl From<MTime> for proto::Timestamp {
     }
 }
 
-// TODO!(yara) for protocol get out u64 via keydata().ffi()
 slotmap::new_key_type! { pub struct TrashId; }
 
 impl TrashId {

crates/git_ui/src/git_panel.rs 🔗

@@ -1533,7 +1533,7 @@ impl GitPanel {
                     let task = workspace.update(cx, |workspace, cx| {
                         workspace
                             .project()
-                            .update(cx, |project, cx| project.delete_file(path, true, cx))
+                            .update(cx, |project, cx| project.trash_file(path, cx))
                     })?;
                     if let Some(task) = task {
                         task.await?;
@@ -1727,7 +1727,7 @@ impl GitPanel {
                             let project_path = active_repo
                                 .read(cx)
                                 .repo_path_to_project_path(&entry.repo_path, cx)?;
-                            project.delete_file(project_path, true, cx)
+                            project.trash_file(project_path, cx)
                         })
                     })
                     .collect::<Vec<_>>()

crates/project/src/project.rs 🔗

@@ -2560,37 +2560,53 @@ impl Project {
         })
     }
 
+    #[inline]
+    pub fn trash_file(
+        &mut self,
+        path: ProjectPath,
+        cx: &mut Context<Self>,
+    ) -> Option<Task<Result<TrashId>>> {
+        let entry = self.entry_for_path(&path, cx)?;
+        self.trash_entry(entry.id, cx)
+    }
+
     #[inline]
     pub fn delete_file(
         &mut self,
         path: ProjectPath,
-        trash: bool,
         cx: &mut Context<Self>,
-    ) -> Option<Task<Result<Option<TrashedEntry>>>> {
+    ) -> Option<Task<Result<()>>> {
         let entry = self.entry_for_path(&path, cx)?;
-        self.delete_entry(entry.id, trash, cx)
+        self.delete_entry(entry.id, cx)
+    }
+
+    #[inline]
+    pub fn trash_entry(
+        &mut self,
+        entry_id: ProjectEntryId,
+        cx: &mut Context<Self>,
+    ) -> Option<Task<Result<TrashId>>> {
+        let worktree = self.worktree_for_entry(entry_id, cx)?;
+        cx.emit(Event::DeletedEntry(worktree.read(cx).id(), entry_id));
+        worktree.update(cx, |worktree, cx| worktree.trash_entry(entry_id, cx))
     }
 
     #[inline]
     pub fn delete_entry(
         &mut self,
         entry_id: ProjectEntryId,
-        trash: bool,
         cx: &mut Context<Self>,
-    ) -> Option<Task<Result<Option<TrashedEntry>>>> {
-        dbg!("Project::delete_entry");
+    ) -> Option<Task<Result<()>>> {
         let worktree = self.worktree_for_entry(entry_id, cx)?;
         cx.emit(Event::DeletedEntry(worktree.read(cx).id(), entry_id));
-        worktree.update(cx, |worktree, cx| {
-            worktree.delete_entry(entry_id, trash, cx)
-        })
+        worktree.update(cx, |worktree, cx| worktree.delete_entry(entry_id, cx))
     }
 
     #[inline]
     pub fn restore_entry(
         &self,
         worktree_id: WorktreeId,
-        trash_entry: TrashedEntry,
+        trash_id: TrashId,
         cx: &mut Context<'_, Self>,
     ) -> Task<Result<ProjectPath>> {
         let Some(worktree) = self.worktree_for_id(worktree_id, cx) else {
@@ -2598,7 +2614,7 @@ impl Project {
         };
 
         cx.spawn(async move |_, cx| {
-            Worktree::restore_entry(trash_entry, worktree, cx)
+            Worktree::restore_entry(trash_id, worktree, cx)
                 .await
                 .map(|rel_path_buf| ProjectPath {
                     worktree_id: worktree_id,

crates/project/src/worktree_store.rs 🔗

@@ -101,6 +101,7 @@ impl WorktreeStore {
         client.add_entity_request_handler(Self::handle_create_project_entry);
         client.add_entity_request_handler(Self::handle_copy_project_entry);
         client.add_entity_request_handler(Self::handle_delete_project_entry);
+        client.add_entity_request_handler(Self::handle_trash_project_entry);
         client.add_entity_request_handler(Self::handle_expand_project_entry);
         client.add_entity_request_handler(Self::handle_expand_all_for_project_entry);
     }
@@ -1161,6 +1162,28 @@ impl WorktreeStore {
         })
     }
 
+    pub async fn handle_trash_project_entry(
+        this: Entity<Self>,
+        envelope: TypedEnvelope<proto::TrashProjectEntry>,
+        mut cx: AsyncApp,
+    ) -> Result<proto::TrashProjectEntryResponse> {
+        let entry_id = ProjectEntryId::from_proto(envelope.payload.entry_id);
+        let worktree = this.update(&mut cx, |this, cx| {
+            let Some((_, project_id)) = this.downstream_client else {
+                bail!("no downstream client")
+            };
+            let Some(entry) = this.entry_for_id(entry_id, cx) else {
+                bail!("no entry")
+            };
+            if entry.is_private && project_id != REMOTE_SERVER_PROJECT_ID {
+                bail!("entry is private")
+            }
+            this.worktree_for_entry(entry_id, cx)
+                .context("worktree not found")
+        })?;
+        Worktree::handle_trash_entry(worktree, envelope.payload, cx).await
+    }
+
     pub async fn handle_delete_project_entry(
         this: Entity<Self>,
         envelope: TypedEnvelope<proto::DeleteProjectEntry>,

crates/project_panel/src/project_panel.rs 🔗

@@ -1208,7 +1208,6 @@ impl ProjectPanel {
                             .when(!should_hide_rename, |menu| {
                                 menu.separator().action("Rename", Box::new(Rename))
                             })
-                            // .when(!is_root && !is_remote, |menu| {
                             .when(!is_root, |menu| {
                                 menu.action("Trash", Box::new(Trash { skip_prompt: false }))
                             })
@@ -2213,15 +2212,11 @@ impl ProjectPanel {
         self.rename_impl(None, window, cx);
     }
 
-    // TODO!(dino): Ensure that the listener in remote is enabled, right now is
-    // disabled.
     fn trash(&mut self, action: &Trash, window: &mut Window, cx: &mut Context<Self>) {
-        dbg!("ProjectPanel::trash");
         self.remove(true, action.skip_prompt, window, cx);
     }
 
     fn delete(&mut self, action: &Delete, window: &mut Window, cx: &mut Context<Self>) {
-        dbg!("ProjectPanel::delete");
         self.remove(false, action.skip_prompt, window, cx);
     }
 
@@ -2319,6 +2314,9 @@ impl ProjectPanel {
         });
     }
 
+    // TODO!(yara|dino) this should be split up, trashing and deleting are
+    // fundamentally different. Adding to the existing differences trashing will
+    // no longer need a confirmation with the introduction of undo
     fn remove(
         &mut self,
         trash: bool,
@@ -2426,20 +2424,29 @@ impl ProjectPanel {
                 let mut changes = Vec::new();
 
                 for (entry_id, worktree_id, _) in file_paths {
-                    let trashed_entry = panel
-                        .update(cx, |panel, cx| {
-                            panel
-                                .project
-                                .update(cx, |project, cx| project.delete_entry(entry_id, trash, cx))
-                                .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));
+                    if trash {
+                        let trash_id = panel
+                            .update(cx, |panel, cx| {
+                                panel
+                                    .project
+                                    .update(cx, |project, cx| project.trash_entry(entry_id, cx))
+                                    .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.
+                        changes.push(Change::Trashed(worktree_id, trash_id))
+                    } else {
+                        panel
+                            .update(cx, |panel, cx| {
+                                panel
+                                    .project
+                                    .update(cx, |project, cx| project.delete_entry(entry_id, cx))
+                                    .context("no such entry")
+                            })??
+                            .await?;
                     }
                 }
                 panel.update_in(cx, |panel, window, cx| {
@@ -6683,9 +6690,7 @@ impl Render for ProjectPanel {
                         .on_action(cx.listener(Self::paste))
                         .on_action(cx.listener(Self::duplicate))
                         .on_action(cx.listener(Self::restore_file))
-                        .when(!project.is_remote(), |el| {
-                            el.on_action(cx.listener(Self::trash))
-                        })
+                        .on_action(cx.listener(Self::trash))
                 })
                 .when(
                     project.is_local() || project.is_via_wsl_with_host_interop(cx),

crates/project_panel/src/undo.rs 🔗

@@ -10,9 +10,9 @@
 //!  Operations                            Results
 //!  ─────────────────────────────────  ──────────────────────────────────────
 //!  Create(ProjectPath)               →  Created(ProjectPath)
-//!  Trash(ProjectPath)                →  Trashed(TrashedEntry)
+//!  Trash(ProjectPath)                →  Trashed(TrashId)
 //!  Rename(ProjectPath, ProjectPath)  →  Renamed(ProjectPath, ProjectPath)
-//!  Restore(TrashedEntry)             →  Restored(ProjectPath)
+//!  Restore(TrashId)                  →  Restored(ProjectPath)
 //!  Batch(Vec<Operation>)             →  Batch(Vec<Result>)
 //!
 //!
@@ -59,12 +59,12 @@
 //!                                  │
 //! User Operation  Undo             v
 //! Execute         Created(CONTRIBUTING.md) ────────> Trash(CONTRIBUTING.md)
-//! Record          Trashed(TrashedEntry(1))
+//! Record          Trashed(TrashId(1))
 //! History
 //! 	0 Created(src/main.rs)
 //! 	1 Renamed(README.md, readme.md) ─┐
 //!     2 +++cursor+++                   │(before the cursor)
-//! 	2 Trashed(TrashedEntry(1))       │
+//! 	2 Trashed(TrashId(1))            │
 //!                                      │
 //! User Operation  Undo                 v
 //! Execute         Renamed(README.md, readme.md) ───> Rename(readme.md, README.md)
@@ -73,7 +73,7 @@
 //! 	0 Created(src/main.rs)
 //!     1 +++cursor+++
 //! 	1 Renamed(readme.md, README.md) ─┐ (at the cursor)
-//! 	2 Trashed(TrashedEntry(1))       │
+//! 	2 Trashed(TrashId(1))            │
 //!                                      │
 //!   ┌──────────────────────────────────┴─────────────────────────────────────────┐
 //!     Redoing will take the result at the cursor position, convert that into the
@@ -90,10 +90,10 @@
 //! 	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))
+//! 	2 Trashed(TrashId(1))───────┐ (at the cursor)
+//!                                 │
+//! User Operation  Redo            v
+//! Execute         Trashed(TrashId(1)) ────────> Restore(TrashId(1))
 //! Record          Restored(ProjectPath)
 //! History
 //! 	0 Created(src/main.rs)
@@ -132,7 +132,7 @@
 
 use crate::ProjectPanel;
 use anyhow::{Context, Result, anyhow};
-use fs::TrashedEntry;
+use fs::TrashId;
 use futures::channel::mpsc;
 use gpui::{AppContext, AsyncApp, SharedString, Task, WeakEntity};
 use project::{ProjectPath, WorktreeId};
@@ -148,7 +148,7 @@ use worktree::CreatedEntry;
 enum Operation {
     Trash(ProjectPath),
     Rename(ProjectPath, ProjectPath),
-    Restore(WorktreeId, TrashedEntry),
+    Restore(WorktreeId, TrashId),
     Batch(Vec<Operation>),
 }
 
@@ -156,15 +156,15 @@ impl Operation {
     async fn execute(self, undo_manager: &Inner, cx: &mut AsyncApp) -> Result<Change> {
         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)
+                let trash_id = undo_manager.trash(&project_path, cx).await?;
+                Change::Trashed(project_path.worktree_id, trash_id)
             }
             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?;
+            Operation::Restore(worktree_id, trash_id) => {
+                let project_path = undo_manager.restore(worktree_id, trash_id, cx).await?;
                 Change::Restored(project_path)
             }
             Operation::Batch(operations) => {
@@ -181,7 +181,7 @@ impl Operation {
 #[derive(Clone, Debug)]
 pub(crate) enum Change {
     Created(ProjectPath),
-    Trashed(WorktreeId, TrashedEntry),
+    Trashed(WorktreeId, TrashId),
     Renamed(ProjectPath, ProjectPath),
     Restored(ProjectPath),
     Batched(Vec<Change>),
@@ -191,9 +191,7 @@ 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::Trashed(worktree_id, trash_id) => Operation::Restore(worktree_id, trash_id),
             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
@@ -384,7 +382,7 @@ impl Inner {
         // 	0 Created(src/main.rs)
         // 	1 Renamed(README.md, readme.md) ─┐
         //     2 +++cursor+++                │(before the cursor)
-        // 	2 Trashed(TrashedEntry(1))       │
+        // 	2 Trashed(TrashId(1))            │
         //                                   │
         // User Operation  Undo              v
         // Failed execute  Renamed(README.md, readme.md) ───> Rename(readme.md, README.md)
@@ -392,10 +390,10 @@ impl Inner {
         // 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))
+        // 	1 Trashed(TrashId(1)) ---------
+        //                                |(at the cursor)
+        // User Operation  Redo           v
+        // Execute         Trashed(TrashId(1)) ────────> Restore(TrashId(1))
         // Record          Restored(ProjectPath)
         // History
         // 	0 Created(src/main.rs)
@@ -494,7 +492,7 @@ impl Inner {
         res?.await
     }
 
-    async fn trash(&self, project_path: &ProjectPath, cx: &mut AsyncApp) -> Result<TrashedEntry> {
+    async fn trash(&self, project_path: &ProjectPath, cx: &mut AsyncApp) -> Result<TrashId> {
         let Some(workspace) = self.workspace.upgrade() else {
             return Err(anyhow!("Failed to obtain workspace."));
         };
@@ -508,20 +506,17 @@ impl Inner {
                         .ok_or_else(|| anyhow!("No entry for path."))?;
 
                     project
-                        .delete_entry(entry_id, true, cx)
+                        .trash_entry(entry_id, cx)
                         .ok_or_else(|| anyhow!("Worktree entry should exist"))
                 })
             })?
             .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,
+        trash_id: TrashId,
         cx: &mut AsyncApp,
     ) -> Result<ProjectPath> {
         let Some(workspace) = self.workspace.upgrade() else {
@@ -531,7 +526,7 @@ impl Inner {
         workspace
             .update(cx, |workspace, cx| {
                 workspace.project().update(cx, |project, cx| {
-                    project.restore_entry(worktree_id, trashed_entry, cx)
+                    project.restore_entry(worktree_id, trash_id, cx)
                 })
             })
             .await

crates/proto/proto/worktree.proto 🔗

@@ -151,6 +151,7 @@ message ProjectEntryResponse {
 
 message TrashProjectEntryResponse {
   uint64 trash_id = 1;
+  uint64 worktree_scan_id = 2;
 }
 
 message UpdateWorktreeSettings {

crates/worktree/src/worktree.rs 🔗

@@ -844,26 +844,51 @@ impl Worktree {
         }
     }
 
-    pub fn delete_entry(
+    pub fn trash_entry(
         &mut self,
         entry_id: ProjectEntryId,
-        trash: bool,
         cx: &mut Context<Worktree>,
-    ) -> Option<Task<Result<Option<TrashId>>>> {
+    ) -> Option<Task<Result<TrashId>>> {
+        let entry = match self {
+            Worktree::Local(this) => this.entry_for_id(entry_id),
+            Worktree::Remote(this) => this.entry_for_id(entry_id),
+        }?
+        .clone();
+
         let task = match self {
-            Worktree::Local(this) => this.delete_entry(entry_id, trash, cx),
-            Worktree::Remote(this) => this.delete_entry(entry_id, trash, cx),
-        }?;
+            Worktree::Local(this) => this.trash_entry(entry.clone(), cx),
+            Worktree::Remote(this) => this.trash_entry(entry_id, cx),
+        };
 
-        let entry = match &*self {
+        let mut ids = vec![entry_id];
+        self.get_children_ids_recursive(&entry.path, &mut ids);
+
+        for id in ids {
+            cx.emit(Event::DeletedEntry(id));
+        }
+        Some(task)
+    }
+
+    pub fn delete_entry(
+        &mut self,
+        entry_id: ProjectEntryId,
+        cx: &mut Context<Worktree>,
+    ) -> Option<Task<Result<()>>> {
+        let entry = match self {
             Worktree::Local(this) => this.entry_for_id(entry_id),
             Worktree::Remote(this) => this.entry_for_id(entry_id),
-        }?;
+        }?
+        .clone();
+
+        let task = match self {
+            Worktree::Local(this) => this.delete_entry(entry.clone(), cx),
+            Worktree::Remote(this) => this.delete_entry(entry_id, cx),
+        };
 
         let mut ids = vec![entry_id];
-        let path = &*entry.path;
+        let path = entry.path;
 
-        self.get_children_ids_recursive(path, &mut ids);
+        self.get_children_ids_recursive(&path, &mut ids);
 
         for id in ids {
             cx.emit(Event::DeletedEntry(id));
@@ -880,8 +905,7 @@ impl Worktree {
         if is_local {
             LocalWorktree::restore_entry(trash_id, worktree, cx).await
         } else {
-            // TODO(dino): Add support for restoring entries in remote worktrees.
-            Err(anyhow!("Unsupported"))
+            RemoteWorktree::restore_entry(trash_id, worktree, cx).await
         }
     }
 
@@ -985,6 +1009,27 @@ impl Worktree {
         })
     }
 
+    pub async fn handle_trash_entry(
+        this: Entity<Self>,
+        request: proto::TrashProjectEntry,
+        mut cx: AsyncApp,
+    ) -> Result<proto::TrashProjectEntryResponse> {
+        let (scan_id, task) = this.update(&mut cx, |this, cx| {
+            (
+                this.scan_id(),
+                this.trash_entry(ProjectEntryId::from_proto(request.entry_id), cx),
+            )
+        });
+        let trash_id = task
+            .ok_or_else(|| anyhow::anyhow!("invalid entry"))?
+            .await?;
+
+        Ok(proto::TrashProjectEntryResponse {
+            trash_id: trash_id.to_u64(),
+            worktree_scan_id: scan_id as u64,
+        })
+    }
+
     pub async fn handle_delete_entry(
         this: Entity<Self>,
         request: proto::DeleteProjectEntry,
@@ -993,15 +1038,10 @@ impl Worktree {
         let (scan_id, task) = this.update(&mut cx, |this, cx| {
             (
                 this.scan_id(),
-                this.delete_entry(
-                    ProjectEntryId::from_proto(request.entry_id),
-                    request.use_trash,
-                    cx,
-                ),
+                this.delete_entry(ProjectEntryId::from_proto(request.entry_id), cx),
             )
         });
-        let trash_id = task
-            .ok_or_else(|| anyhow::anyhow!("invalid entry"))?
+        task.ok_or_else(|| anyhow::anyhow!("invalid entry"))?
             .await?;
         Ok(proto::ProjectEntryResponse {
             entry: None,
@@ -1684,61 +1724,64 @@ impl LocalWorktree {
         })
     }
 
-    pub fn delete_entry(
-        &self,
-        entry_id: ProjectEntryId,
-        trash: bool,
-        cx: &Context<Worktree>,
-    ) -> Option<Task<Result<Option<TrashId>>>> {
-        let entry = self.entry_for_id(entry_id)?.clone();
+    pub fn trash_entry(&self, entry: Entry, cx: &Context<Worktree>) -> Task<Result<TrashId>> {
         let abs_path = self.absolutize(&entry.path);
         let fs = self.fs.clone();
 
-        let delete = cx.background_spawn(async move {
-            let trash_id = 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
-                }
-                (false, false) => {
-                    fs.remove_dir(
-                        &abs_path,
-                        RemoveOptions {
-                            recursive: true,
-                            ignore_if_not_exists: false,
-                        },
-                    )
-                    .await?;
-                    None
-                }
+        cx.spawn(async move |this, cx| {
+            let trash_id = if entry.is_file() {
+                fs.trash(&abs_path, Default::default()).await?
+            } else {
+                fs.trash(
+                    &abs_path,
+                    RemoveOptions {
+                        recursive: true,
+                        ignore_if_not_exists: false,
+                    },
+                )
+                .await?
             };
 
-            anyhow::Ok((trash_id, entry.path))
-        });
+            this.update(cx, |this, _| {
+                this.as_local_mut()
+                    .unwrap()
+                    .refresh_entries_for_paths(vec![entry.path])
+            })?
+            .recv()
+            .await;
+
+            Ok(trash_id)
+        })
+    }
+
+    pub fn delete_entry(&self, entry: Entry, cx: &Context<Worktree>) -> Task<Result<()>> {
+        let abs_path = self.absolutize(&entry.path);
+        let fs = self.fs.clone();
+
+        cx.spawn(async move |this, cx| {
+            if entry.is_file() {
+                fs.remove_file(&abs_path, Default::default()).await?
+            } else {
+                fs.remove_dir(
+                    &abs_path,
+                    RemoveOptions {
+                        recursive: true,
+                        ignore_if_not_exists: false,
+                    },
+                )
+                .await?
+            };
 
-        Some(cx.spawn(async move |this, cx| {
-            let (trashed_entry, path) = delete.await?;
             this.update(cx, |this, _| {
                 this.as_local_mut()
                     .unwrap()
-                    .refresh_entries_for_paths(vec![path])
+                    .refresh_entries_for_paths(vec![entry.path])
             })?
             .recv()
             .await;
 
-            Ok(trashed_entry)
-        }))
+            Ok(())
+        })
     }
 
     pub async fn restore_entry(
@@ -2474,7 +2517,6 @@ impl Snapshot {
         }
 
         self.scan_id = update.scan_id as usize;
-        /// TODO!(yara) batch the scans
         if update.is_last_update {
             self.completed_scan_id = update.scan_id as usize;
         }

crates/worktree/tests/integration/main.rs 🔗

@@ -2207,9 +2207,7 @@ 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());
-            let task = worktree
-                .delete_entry(entry.id, false, cx)
-                .unwrap_or_else(|| Task::ready(Ok(None)));
+            let task = worktree.delete_entry(entry.clone(), cx);
 
             cx.background_spawn(async move {
                 task.await?;