Switch from delete file by default to trash file by default (#10875)

Mikayla Maki created

TODO:

- [x] Don't immediately seg fault
- [x] Implement for directories 
- [x] Add cmd-delete to remove files
- [ ] ~~Add setting for trash vs. delete~~ You can just use keybindings
to change the behavior.

fixes https://github.com/zed-industries/zed/issues/7228
fixes https://github.com/zed-industries/zed/issues/5094

Release Notes:

- Added a new `project_panel::Trash` action and changed the default
behavior for `backspace` and `delete` in the project panel to send a
file to the systems trash, instead of permanently deleting it
([#7228](https://github.com/zed-industries/zed/issues/7228),
[#5094](https://github.com/zed-industries/zed/issues/5094)). The
original behavior can be restored by adding the following section to
your keybindings:

```json5
[
// ...Other keybindings...
  {
    "context": "ProjectPanel",
    "bindings": {
        "backspace": "project_panel::Delete",
        "delete": "project_panel::Delete",
    }
  }
]

Change summary

Cargo.lock                                   |  2 +
assets/keymaps/default-linux.json            |  4 +-
assets/keymaps/default-macos.json            |  4 +-
crates/collab/src/tests/integration_tests.rs |  4 +-
crates/fs/Cargo.toml                         |  3 +
crates/fs/src/fs.rs                          | 33 +++++++++++++++++++++
crates/project/src/project.rs                | 10 +++++-
crates/project_panel/src/project_panel.rs    | 24 ++++++++++++--
crates/rpc/proto/zed.proto                   |  1 
crates/worktree/src/worktree.rs              | 34 ++++++++++++++++-----
crates/worktree/src/worktree_tests.rs        |  2 
11 files changed, 99 insertions(+), 22 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -4065,6 +4065,7 @@ dependencies = [
  "anyhow",
  "async-tar",
  "async-trait",
+ "cocoa",
  "collections",
  "fsevent",
  "futures 0.3.28",
@@ -4074,6 +4075,7 @@ dependencies = [
  "lazy_static",
  "libc",
  "notify",
+ "objc",
  "parking_lot",
  "rope",
  "serde",

assets/keymaps/default-linux.json 🔗

@@ -549,8 +549,8 @@
       "alt-ctrl-shift-c": "project_panel::CopyRelativePath",
       "f2": "project_panel::Rename",
       "enter": "project_panel::Rename",
-      "backspace": "project_panel::Delete",
-      "delete": "project_panel::Delete",
+      "backspace": "project_panel::Trash",
+      "delete": "project_panel::Trash",
       "ctrl-backspace": ["project_panel::Delete", { "skip_prompt": true }],
       "ctrl-delete": ["project_panel::Delete", { "skip_prompt": true }],
       "alt-ctrl-r": "project_panel::RevealInFinder",

assets/keymaps/default-macos.json 🔗

@@ -576,8 +576,8 @@
       "alt-cmd-shift-c": "project_panel::CopyRelativePath",
       "f2": "project_panel::Rename",
       "enter": "project_panel::Rename",
-      "backspace": "project_panel::Delete",
-      "delete": "project_panel::Delete",
+      "backspace": "project_panel::Trash",
+      "delete": "project_panel::Trash",
       "cmd-backspace": ["project_panel::Delete", { "skip_prompt": true }],
       "cmd-delete": ["project_panel::Delete", { "skip_prompt": true }],
       "alt-cmd-r": "project_panel::RevealInFinder",

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

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

crates/fs/Cargo.toml 🔗

@@ -36,6 +36,9 @@ gpui = { workspace = true, optional = true }
 
 [target.'cfg(target_os = "macos")'.dependencies]
 fsevent.workspace = true
+objc = "0.2"
+cocoa = "0.25"
+
 
 [target.'cfg(not(target_os = "macos"))'.dependencies]
 notify = "6.1.1"

crates/fs/src/fs.rs 🔗

@@ -49,7 +49,13 @@ pub trait Fs: Send + Sync {
     async fn copy_file(&self, source: &Path, target: &Path, options: CopyOptions) -> Result<()>;
     async fn rename(&self, source: &Path, target: &Path, options: RenameOptions) -> Result<()>;
     async fn remove_dir(&self, path: &Path, options: RemoveOptions) -> Result<()>;
+    async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result<()> {
+        self.remove_dir(path, options).await
+    }
     async fn remove_file(&self, path: &Path, options: RemoveOptions) -> Result<()>;
+    async fn trash_file(&self, path: &Path, options: RemoveOptions) -> Result<()> {
+        self.remove_file(path, options).await
+    }
     async fn open_sync(&self, path: &Path) -> Result<Box<dyn io::Read>>;
     async fn load(&self, path: &Path) -> Result<String>;
     async fn atomic_write(&self, path: PathBuf, text: String) -> Result<()>;
@@ -237,6 +243,33 @@ impl Fs for RealFs {
         }
     }
 
+    #[cfg(target_os = "macos")]
+    async fn trash_file(&self, path: &Path, _options: RemoveOptions) -> Result<()> {
+        use cocoa::{
+            base::{id, nil},
+            foundation::{NSAutoreleasePool, NSString},
+        };
+        use objc::{class, msg_send, sel, sel_impl};
+
+        unsafe {
+            unsafe fn ns_string(string: &str) -> id {
+                NSString::alloc(nil).init_str(string).autorelease()
+            }
+
+            let url: id = msg_send![class!(NSURL), fileURLWithPath: ns_string(path.to_string_lossy().as_ref())];
+            let array: id = msg_send![class!(NSArray), arrayWithObject: url];
+            let workspace: id = msg_send![class!(NSWorkspace), sharedWorkspace];
+
+            let _: id = msg_send![workspace, recycleURLs: array completionHandler: nil];
+        }
+        Ok(())
+    }
+
+    #[cfg(target_os = "macos")]
+    async fn trash_dir(&self, path: &Path, options: RemoveOptions) -> Result<()> {
+        self.trash_file(path, options).await
+    }
+
     async fn open_sync(&self, path: &Path) -> Result<Box<dyn io::Read>> {
         Ok(Box::new(std::fs::File::open(path)?))
     }

crates/project/src/project.rs 🔗

@@ -1513,6 +1513,7 @@ impl Project {
     pub fn delete_entry(
         &mut self,
         entry_id: ProjectEntryId,
+        trash: bool,
         cx: &mut ModelContext<Self>,
     ) -> Option<Task<Result<()>>> {
         let worktree = self.worktree_for_entry(entry_id, cx)?;
@@ -1521,7 +1522,10 @@ impl Project {
 
         if self.is_local() {
             worktree.update(cx, |worktree, cx| {
-                worktree.as_local_mut().unwrap().delete_entry(entry_id, cx)
+                worktree
+                    .as_local_mut()
+                    .unwrap()
+                    .delete_entry(entry_id, trash, cx)
             })
         } else {
             let client = self.client.clone();
@@ -1531,6 +1535,7 @@ impl Project {
                     .request(proto::DeleteProjectEntry {
                         project_id,
                         entry_id: entry_id.to_proto(),
+                        use_trash: trash,
                     })
                     .await?;
                 worktree
@@ -8341,6 +8346,7 @@ impl Project {
         mut cx: AsyncAppContext,
     ) -> Result<proto::ProjectEntryResponse> {
         let entry_id = ProjectEntryId::from_proto(envelope.payload.entry_id);
+        let trash = envelope.payload.use_trash;
 
         this.update(&mut cx, |_, cx| cx.emit(Event::DeletedEntry(entry_id)))?;
 
@@ -8354,7 +8360,7 @@ impl Project {
                 worktree
                     .as_local_mut()
                     .unwrap()
-                    .delete_entry(entry_id, cx)
+                    .delete_entry(entry_id, trash, cx)
                     .ok_or_else(|| anyhow!("invalid entry"))
             })??
             .await?;

crates/project_panel/src/project_panel.rs 🔗

@@ -111,7 +111,13 @@ pub struct Delete {
     pub skip_prompt: bool,
 }
 
-impl_actions!(project_panel, [Delete]);
+#[derive(PartialEq, Clone, Default, Debug, Deserialize)]
+pub struct Trash {
+    #[serde(default)]
+    pub skip_prompt: bool,
+}
+
+impl_actions!(project_panel, [Delete, Trash]);
 
 actions!(
     project_panel,
@@ -880,16 +886,25 @@ impl ProjectPanel {
         }
     }
 
+    fn trash(&mut self, action: &Trash, cx: &mut ViewContext<Self>) {
+        self.remove(true, action.skip_prompt, cx);
+    }
+
     fn delete(&mut self, action: &Delete, cx: &mut ViewContext<Self>) {
+        self.remove(false, action.skip_prompt, cx);
+    }
+
+    fn remove(&mut self, trash: bool, skip_prompt: bool, cx: &mut ViewContext<'_, ProjectPanel>) {
         maybe!({
             let Selection { entry_id, .. } = self.selection?;
             let path = self.project.read(cx).path_for_entry(entry_id, cx)?.path;
             let file_name = path.file_name()?;
 
-            let answer = (!action.skip_prompt).then(|| {
+            let operation = if trash { "Trash" } else { "Delete" };
+            let answer = (!skip_prompt).then(|| {
                 cx.prompt(
                     PromptLevel::Destructive,
-                    &format!("Delete {file_name:?}?"),
+                    &format!("{operation:?} {file_name:?}?",),
                     None,
                     &["Delete", "Cancel"],
                 )
@@ -903,7 +918,7 @@ impl ProjectPanel {
                 }
                 this.update(&mut cx, |this, cx| {
                     this.project
-                        .update(cx, |project, cx| project.delete_entry(entry_id, cx))
+                        .update(cx, |project, cx| project.delete_entry(entry_id, trash, cx))
                         .ok_or_else(|| anyhow!("no such entry"))
                 })??
                 .await
@@ -1808,6 +1823,7 @@ impl Render for ProjectPanel {
                         .on_action(cx.listener(Self::new_directory))
                         .on_action(cx.listener(Self::rename))
                         .on_action(cx.listener(Self::delete))
+                        .on_action(cx.listener(Self::trash))
                         .on_action(cx.listener(Self::cut))
                         .on_action(cx.listener(Self::copy))
                         .on_action(cx.listener(Self::paste))

crates/rpc/proto/zed.proto 🔗

@@ -578,6 +578,7 @@ message CopyProjectEntry {
 message DeleteProjectEntry {
     uint64 project_id = 1;
     uint64 entry_id = 2;
+    bool use_trash = 3;
 }
 
 message ExpandProjectEntry {

crates/worktree/src/worktree.rs 🔗

@@ -1335,6 +1335,7 @@ impl LocalWorktree {
     pub fn delete_entry(
         &self,
         entry_id: ProjectEntryId,
+        trash: bool,
         cx: &mut ModelContext<Worktree>,
     ) -> Option<Task<Result<()>>> {
         let entry = self.entry_for_id(entry_id)?.clone();
@@ -1343,16 +1344,31 @@ impl LocalWorktree {
 
         let delete = cx.background_executor().spawn(async move {
             if entry.is_file() {
-                fs.remove_file(&abs_path?, Default::default()).await?;
+                if trash {
+                    fs.trash_file(&abs_path?, Default::default()).await?;
+                } else {
+                    fs.remove_file(&abs_path?, Default::default()).await?;
+                }
             } else {
-                fs.remove_dir(
-                    &abs_path?,
-                    RemoveOptions {
-                        recursive: true,
-                        ignore_if_not_exists: false,
-                    },
-                )
-                .await?;
+                if trash {
+                    fs.trash_dir(
+                        &abs_path?,
+                        RemoveOptions {
+                            recursive: true,
+                            ignore_if_not_exists: false,
+                        },
+                    )
+                    .await?;
+                } else {
+                    fs.remove_dir(
+                        &abs_path?,
+                        RemoveOptions {
+                            recursive: true,
+                            ignore_if_not_exists: false,
+                        },
+                    )
+                    .await?;
+                }
             }
             anyhow::Ok(entry.path)
         });

crates/worktree/src/worktree_tests.rs 🔗

@@ -1764,7 +1764,7 @@ fn randomly_mutate_worktree(
     match rng.gen_range(0_u32..100) {
         0..=33 if entry.path.as_ref() != Path::new("") => {
             log::info!("deleting entry {:?} ({})", entry.path, entry.id.0);
-            worktree.delete_entry(entry.id, cx).unwrap()
+            worktree.delete_entry(entry.id, false, cx).unwrap()
         }
         ..=66 if entry.path.as_ref() != Path::new("") => {
             let other_entry = snapshot.entries(false).choose(rng).unwrap();