Cargo.lock 🔗
@@ -6480,6 +6480,7 @@ dependencies = [
"rope",
"serde",
"serde_json",
+ "slotmap",
"smol",
"tempfile",
"text",
checkpoint: introduce trash id and keep trash in fs
dino created
Change summary
Cargo.lock | 1
crates/fs/Cargo.toml | 1
crates/fs/src/fs.rs | 81 ++++++++++++------------
crates/fs/tests/integration/fs.rs | 67 +++-----------------
crates/project_panel/src/project_panel.rs | 2
crates/worktree/src/worktree.rs | 75 +++-------------------
6 files changed, 66 insertions(+), 161 deletions(-)
Detailed changes
crates/fs/Cargo.toml 🔗
@@ -35,6 +35,7 @@ proto.workspace = true
thiserror.workspace = true
serde.workspace = true
serde_json.workspace = true
+slotmap.workspace = true
smol.workspace = true
tempfile.workspace = true
text.workspace = true
crates/fs/src/fs.rs 🔗
@@ -1,6 +1,7 @@
pub mod fs_watcher;
use parking_lot::Mutex;
+use slotmap::SlotMap;
use std::ffi::OsString;
use std::sync::atomic::{AtomicU8, AtomicUsize, Ordering};
use std::time::Instant;
@@ -118,7 +119,7 @@ pub trait Fs: Send + Sync {
/// 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 item in the system's trash.
- async fn trash(&self, path: &Path, options: RemoveOptions) -> Result<TrashedEntry>;
+ async fn trash(&self, path: &Path, options: RemoveOptions) -> Result<TrashId>;
/// Removes a file from the filesystem.
/// There is no expectation that the file will be preserved in the system
@@ -167,10 +168,7 @@ pub trait Fs: Send + Sync {
/// Restores a given `TrashedEntry`, moving it from the system's trash back
/// to the original path.
- async fn restore(
- &self,
- trashed_entry: TrashedEntry,
- ) -> std::result::Result<PathBuf, TrashRestoreError>;
+ async fn restore(&self, item: TrashId) -> std::result::Result<PathBuf, TrashRestoreError>;
#[cfg(feature = "test-support")]
fn as_fake(&self) -> Arc<FakeFs> {
@@ -184,7 +182,7 @@ pub trait Fs: Send + Sync {
/// 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, Debug)]
-pub struct TrashedEntry {
+struct TrashedEntry {
/// Platform-specific identifier for the file/directory in the trash.
///
/// * Freedesktop – Path to the `.trashinfo` file.
@@ -226,6 +224,11 @@ pub enum TrashRestoreError {
NotFound { path: PathBuf },
#[error("File or directory ({}) already exists at the restore destination.", path.display())]
Collision { path: PathBuf },
+ // This should never occur, the only way to get a TrashId is to undo
+ // consumes the Change::Trashed. We worry about remoting duplicate messages
+ // we do not want to crash the app then which is why this error is there.
+ #[error("The item was already restored")]
+ AlreadyRestored,
#[error("Unknown error ({description})")]
Unknown { description: String },
}
@@ -393,11 +396,15 @@ impl From<MTime> for proto::Timestamp {
}
}
+// TODO!(yara) for protocol get out u64 via keydata().ffi()
+slotmap::new_key_type! { pub struct TrashId; }
+
pub struct RealFs {
bundled_git_binary_path: Option<PathBuf>,
executor: BackgroundExecutor,
next_job_id: Arc<AtomicUsize>,
job_event_subscribers: Arc<Mutex<Vec<JobEventSender>>>,
+ trash: Arc<Mutex<SlotMap<TrashId, TrashedEntry>>>,
is_case_sensitive: AtomicU8,
}
@@ -506,6 +513,7 @@ impl RealFs {
executor,
next_job_id: Arc::new(AtomicUsize::new(0)),
job_event_subscribers: Arc::new(Mutex::new(Vec::new())),
+ trash: Arc::new(Mutex::new(SlotMap::with_key())),
is_case_sensitive: Default::default(),
}
}
@@ -796,7 +804,7 @@ impl Fs for RealFs {
}
}
- async fn trash(&self, path: &Path, _options: RemoveOptions) -> Result<TrashedEntry> {
+ async fn trash(&self, path: &Path, _options: RemoveOptions) -> Result<TrashId> {
// 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.
@@ -811,11 +819,14 @@ impl Fs for RealFs {
.spawn(|| tx.send(trash::delete_with_info(path)))
.expect("The os can spawn threads");
- Ok(rx
+ let entry = rx
.await
.context("Tx dropped or fs.restore panicked")?
.context("Could not trash file or dir")?
- .into())
+ .into();
+
+ let id = self.trash.lock().insert(entry);
+ Ok(id)
}
async fn open_sync(&self, path: &Path) -> Result<Box<dyn io::Read + Send + Sync>> {
@@ -1264,10 +1275,13 @@ impl Fs for RealFs {
res
}
- async fn restore(
- &self,
- trashed_entry: TrashedEntry,
- ) -> std::result::Result<PathBuf, TrashRestoreError> {
+ async fn restore(&self, item: TrashId) -> std::result::Result<PathBuf, TrashRestoreError> {
+ let trashed_entry = self
+ .trash
+ .lock()
+ .remove(item)
+ .ok_or(TrashRestoreError::AlreadyRestored)?;
+
let restored_item_path = trashed_entry.original_parent.join(&trashed_entry.name);
let (tx, rx) = futures::channel::oneshot::channel();
@@ -1316,7 +1330,7 @@ struct FakeFsState {
path_write_counts: std::collections::HashMap<PathBuf, usize>,
moves: std::collections::HashMap<u64, PathBuf>,
job_event_subscribers: Arc<Mutex<Vec<JobEventSender>>>,
- trash: Vec<(TrashedEntry, FakeFsEntry)>,
+ trash: Mutex<SlotMap<TrashId, (TrashedEntry, FakeFsEntry)>>,
}
#[cfg(feature = "test-support")]
@@ -1602,7 +1616,7 @@ impl FakeFs {
path_write_counts: Default::default(),
moves: Default::default(),
job_event_subscribers: Arc::new(Mutex::new(Vec::new())),
- trash: Vec::new(),
+ trash: Mutex::new(SlotMap::with_key()),
})),
});
@@ -2429,16 +2443,6 @@ impl FakeFs {
self.executor.simulate_random_delay()
}
- /// Returns list of all tracked trash entries.
- pub fn trash_entries(&self) -> Vec<TrashedEntry> {
- self.state
- .lock()
- .trash
- .iter()
- .map(|(entry, _)| entry.clone())
- .collect()
- }
-
async fn remove_dir_inner(
&self,
path: &Path,
@@ -2814,7 +2818,7 @@ impl Fs for FakeFs {
self.remove_dir_inner(path, options).await.map(|_| ())
}
- async fn trash(&self, path: &Path, options: RemoveOptions) -> Result<TrashedEntry> {
+ async fn trash(&self, path: &Path, options: RemoveOptions) -> Result<TrashId> {
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();
@@ -2832,9 +2836,14 @@ impl Fs for FakeFs {
original_parent: parent_path.to_path_buf(),
};
- let mut state = self.state.lock();
- state.trash.push((trashed_entry.clone(), fake_entry));
- Ok(trashed_entry)
+ let trash_id = self
+ .state
+ .lock()
+ .trash
+ .lock()
+ .insert((trashed_entry.clone(), fake_entry));
+
+ Ok(trash_id)
}
None => anyhow::bail!("{normalized_path:?} does not exist"),
}
@@ -3092,18 +3101,11 @@ impl Fs for FakeFs {
receiver
}
- async fn restore(&self, trashed_entry: TrashedEntry) -> Result<PathBuf, TrashRestoreError> {
+ async fn restore(&self, trash_id: TrashId) -> Result<PathBuf, TrashRestoreError> {
let mut state = self.state.lock();
- let Some((trashed_entry, fake_entry)) = state
- .trash
- .iter()
- .find(|(entry, _)| *entry == trashed_entry)
- .cloned()
- else {
- return Err(TrashRestoreError::NotFound {
- path: PathBuf::from(trashed_entry.id),
- });
+ let Some((trashed_entry, fake_entry)) = state.trash.lock().remove(trash_id) else {
+ return Err(TrashRestoreError::AlreadyRestored);
};
let path = trashed_entry
@@ -3122,7 +3124,6 @@ impl Fs for FakeFs {
match result {
Ok(_) => {
- state.trash.retain(|(entry, _)| *entry != trashed_entry);
state.emit_event([(path.clone(), Some(PathEventKind::Created))]);
Ok(path)
}
crates/fs/tests/integration/fs.rs 🔗
@@ -1,6 +1,5 @@
use std::{
collections::BTreeSet,
- ffi::OsString,
io::Write,
path::{Path, PathBuf},
time::Duration,
@@ -644,15 +643,11 @@ async fn test_fake_fs_trash(executor: BackgroundExecutor) {
.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(path, Default::default())
+ fs.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![
@@ -662,32 +657,19 @@ async fn test_fake_fs_trash(executor: BackgroundExecutor) {
]
);
- 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);
-
// Trashing a directory.
let path = path!("/root/src").as_ref();
- let trashed_entry = fs
- .trash(
- path,
- RemoveOptions {
- recursive: true,
- ..Default::default()
- },
- )
- .await
- .expect("should be able to trash {path:?}");
+ fs.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_b.txt"))]);
-
- let trash_entries = fs.trash_entries();
- assert_eq!(trash_entries.len(), 2);
- assert_eq!(trash_entries[1].name, "src");
- assert_eq!(trash_entries[1].original_parent, root_path);
}
#[gpui::test]
@@ -705,36 +687,14 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) {
)
.await;
- // Providing a non-existent `TrashedEntry` should result in an error.
- 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,
- name,
- original_parent,
- };
- let result = fs.restore(trashed_entry).await;
- assert!(matches!(result, Err(TrashRestoreError::NotFound { .. })));
-
// Attempt deleting a file, asserting that the filesystem no longer reports
// 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(path, Default::default()).await.unwrap();
- assert_eq!(fs.trash_entries().len(), 1);
- assert_eq!(
- fs.files(),
- vec![
- PathBuf::from(path!("/root/file_c.txt")),
- PathBuf::from(path!("/root/src/file_b.txt"))
- ]
- );
-
fs.restore(trashed_entry).await.unwrap();
- assert_eq!(fs.trash_entries().len(), 0);
assert_eq!(
fs.files(),
vec![
@@ -754,7 +714,6 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) {
let path = path!("/root/src/").as_ref();
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"))]);
fs.restore(trashed_entry).await.unwrap();
@@ -767,14 +726,12 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) {
PathBuf::from(path!("/root/src/file_b.txt"))
]
);
- assert_eq!(fs.trash_entries().len(), 0);
// 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(path, Default::default()).await.unwrap();
- assert_eq!(fs.trash_entries().len(), 1);
assert_eq!(
fs.files(),
vec![
@@ -785,7 +742,6 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) {
fs.write(path, "New File A".as_bytes()).await.unwrap();
- assert_eq!(fs.trash_entries().len(), 1);
assert_eq!(
fs.files(),
vec![
@@ -811,19 +767,16 @@ async fn test_fake_fs_restore(executor: BackgroundExecutor) {
let path = path!("/root/src/").as_ref();
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"))]);
fs.create_dir(path).await.unwrap();
assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]);
- assert_eq!(fs.trash_entries().len(), 2);
let result = fs.restore(trashed_entry).await;
assert!(result.is_err());
assert_eq!(fs.files(), vec![PathBuf::from(path!("/root/file_c.txt"))]);
- assert_eq!(fs.trash_entries().len(), 2);
}
#[gpui::test]
crates/project_panel/src/project_panel.rs 🔗
@@ -2213,6 +2213,8 @@ 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);
crates/worktree/src/worktree.rs 🔗
@@ -8,7 +8,7 @@ use clock::ReplicaId;
use collections::{HashMap, HashSet, VecDeque};
use encoding_rs::Encoding;
use fs::{
- Fs, MTime, PathEvent, PathEventKind, RemoveOptions, TrashedEntry, Watcher, copy_recursive,
+ Fs, MTime, PathEvent, PathEventKind, RemoveOptions, TrashId, Watcher, copy_recursive,
read_dir_items,
};
use futures::{
@@ -849,16 +849,10 @@ impl Worktree {
entry_id: ProjectEntryId,
trash: bool,
cx: &mut Context<Worktree>,
- ) -> Option<Task<Result<Option<TrashedEntry>>>> {
+ ) -> Option<Task<Result<Option<TrashId>>>> {
let task = match self {
- Worktree::Local(this) => {
- dbg!(("LOCAL", trash));
- this.delete_entry(entry_id, trash, cx)
- }
- Worktree::Remote(this) => {
- dbg!(("REMOTE", trash));
- this.delete_entry(entry_id, trash, cx)
- }
+ Worktree::Local(this) => this.delete_entry(entry_id, trash, cx),
+ Worktree::Remote(this) => this.delete_entry(entry_id, trash, cx),
}?;
let entry = match &*self {
@@ -878,13 +872,13 @@ impl Worktree {
}
pub async fn restore_entry(
- trash_entry: TrashedEntry,
+ trash_id: TrashId,
worktree: Entity<Self>,
cx: &mut AsyncApp,
) -> Result<RelPathBuf> {
let is_local = worktree.read_with(cx, |this, _| this.is_local());
if is_local {
- LocalWorktree::restore_entry(trash_entry, worktree, cx).await
+ LocalWorktree::restore_entry(trash_id, worktree, cx).await
} else {
// TODO(dino): Add support for restoring entries in remote worktrees.
Err(anyhow!("Unsupported"))
@@ -899,18 +893,6 @@ impl Worktree {
}
}
- // pub fn rename_entry(
- // &mut self,
- // entry_id: ProjectEntryId,
- // new_path: Arc<RelPath>,
- // cx: &Context<Self>,
- // ) -> Task<Result<CreatedEntry>> {
- // match self {
- // Worktree::Local(this) => this.rename_entry(entry_id, new_path, cx),
- // Worktree::Remote(this) => this.rename_entry(entry_id, new_path, cx),
- // }
- // }
-
pub fn copy_external_entries(
&mut self,
target_directory: Arc<RelPath>,
@@ -1019,13 +1001,13 @@ impl Worktree {
),
)
});
- let trashed_entry = task
+ let trash_id = task
.ok_or_else(|| anyhow::anyhow!("invalid entry"))?
.await?;
Ok(proto::ProjectEntryResponse {
entry: None,
worktree_scan_id: scan_id as u64,
- trashed_entry: trashed_entry.map(|e| proto::TrashedEntry {
+ trashed_entry: trash_id.map(|e| proto::TrashedEntry {
trash_id: e.id.to_string_lossy().to_string(),
file_name: e.name.to_string_lossy().to_string(),
original_parent_path: e.original_parent.to_string_lossy().to_string(),
@@ -1713,13 +1695,13 @@ impl LocalWorktree {
entry_id: ProjectEntryId,
trash: bool,
cx: &Context<Worktree>,
- ) -> Option<Task<Result<Option<TrashedEntry>>>> {
+ ) -> Option<Task<Result<Option<TrashId>>>> {
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 {
- let trashed_entry = match (entry.is_file(), trash) {
+ let trash_id = match (entry.is_file(), trash) {
(true, true) => Some(fs.trash(&abs_path, Default::default()).await?),
(false, true) => Some(
fs.trash(
@@ -1748,7 +1730,7 @@ impl LocalWorktree {
}
};
- anyhow::Ok((trashed_entry, entry.path))
+ anyhow::Ok((trash_id, entry.path))
});
Some(cx.spawn(async move |this, cx| {
@@ -2191,41 +2173,6 @@ impl RemoteWorktree {
}))
}
- // fn rename_entry(
- // &self,
- // entry_id: ProjectEntryId,
- // new_path: impl Into<Arc<RelPath>>,
- // cx: &Context<Worktree>,
- // ) -> Task<Result<CreatedEntry>> {
- // let new_path: Arc<RelPath> = new_path.into();
- // let response = self.client.request(proto::RenameProjectEntry {
- // project_id: self.project_id,
- // entry_id: entry_id.to_proto(),
- // new_worktree_id: new_path.worktree_id,
- // new_path: new_path.as_ref().to_proto(),
- // });
- // cx.spawn(async move |this, cx| {
- // let response = response.await?;
- // match response.entry {
- // Some(entry) => this
- // .update(cx, |this, cx| {
- // this.as_remote_mut().unwrap().insert_entry(
- // entry,
- // response.worktree_scan_id as usize,
- // cx,
- // )
- // })?
- // .await
- // .map(CreatedEntry::Included),
- // None => {
- // let abs_path =
- // this.read_with(cx, |worktree, _| worktree.absolutize(&new_path))?;
- // Ok(CreatedEntry::Excluded { abs_path })
- // }
- // }
- // })
- // }
-
fn copy_external_entries(
&self,
target_directory: Arc<RelPath>,