Skip to content

fs: Return trashed file location#52012

Merged
dinocosta merged 15 commits intomainfrom
5039-fs-update
Apr 9, 2026
Merged

fs: Return trashed file location#52012
dinocosta merged 15 commits intomainfrom
5039-fs-update

Conversation

@dinocosta
Copy link
Copy Markdown
Member

@dinocosta dinocosta commented Mar 20, 2026

Context

These changes are meant to eventually be used by the Project Panel's Undo/Redo system, which was started on #47091 .

In order to be able to undo file trashing and file creation operations, we'll want to be able to track where the file is trashed to such that, reverting these operations is simply a matter of moving the file from the OS' trash back to the original location.

Note that this Pull Request will remain in Draft until the corresponding Pull Request in the our trash-rs fork, zed-industries/trash-rs#1 and zed-industries/trash-rs#2, are reviewed and merged.

How to Review

Since most of the work is handled by the trash-rs crate, the most relevant changes in this Pull Request are:

  • Add fs::Fs::TrashedEntry struct, which allows us to track the original file path and the new path in the OS' trash
  • Update the fs::Fs::trash_dir and fs::Fs::trash_file signatures to now return Result<TrashedEntry> instead of Result<()>
    • The options argument was removed because it was never used by implementations other than the default one, and with this change to the signature type, we no longer have a default implementation, so the options argument would no longer make sense
  • Update fs::RealFs::trash_dir and fs::RealFs::trash_file implementations to simply delegate to trash-rs and convert the result to a TrashedEntry
  • Add fs::FakeFs::trash so we can simulate the OS' trash during tests that touch the filesystem
  • Add fs::FakeFs::trash_file implementation to leverage fs::FakeFs::trash
  • Add fs::FakeFs::trash_dir implementation to leverage fs::FakeFs::trash

Self-Review Checklist

  • I've reviewed my own diff for quality, security, and reliability
  • Unsafe blocks (if any) have justifying comments
  • The content is consistent with the UI/UX checklist
  • Tests cover the new/changed behavior
  • Performance impact has been considered and is acceptable

Release Notes:

  • N/A

- Remove `unsafe` block in `telemetry.rs` now that the updated
  `objc2-foundation` no longer requires it for
  `operatingSystemVersionString()`
- Add explicit type annotation on `.collect()` in `path_key.rs` to fix a
  type inference overflow caused by the `objc2-foundation` update
Replace per-platform `trash_file` implementations (macOS/Linux/Windows)
with a single call to `trash::delete`.
* Replace per-platform `trash_dir` implementations with a single call to
  `trash_file.
* Add `util/test-support` feature to `fs` crate so we can actually run
  the `fs` test suite using `cargo t -p fs`.
* Introduce a `fs::TrashedEntry` struct to track the location of trashed
  files and directories in the system's trash.
* Update the return type for both `Fs.trash_file` and `Fs.trash_dir` to
  `Result<TrashedEntry>` instead of `Result<()>`.
* Update `RealFs.trash_file` and `RealFs.trash_dir` to leverage the
  `trash::delete_with_info` function instead of `trash::delete`, so that
  there's actually a `trash::TrashItem` to work with and that can be
  converted into a `fs::TrashedEntry`.
* Add a `FakeFsState.trash` vector so that we can keep track of the
  trashed entries. While this might be irrelevant for now, it's meant to
  be leveraged when adding supporting for restoring trashed entries.
* In order to avoid duplication, the `FakeFs.remove_file` and
  `FakeFs.remove_dir` implementations have been extracted to a
  `FakeFs.remove_file_inner` and `FakeFs.remove_dir_inner` methods,
  respectively, so that they can be reused by `FakeFs.trash_file` and
  `FakeFs.trash_dir`, while also adding the ability to track trashed
  entries in the `FakeFsState`.
  * This was done because, before these changes, the default
    `Fs.trash_file` and `Fs.trash_dir` implementations were just
    forwarding the call to `Fs.remove_file` and `Fs.remove_dir`,
    respectively.
Update both `FakeFs.trash_file` and `FakeFs.trash_dir` to avoid emitting
the `PathEventKind::Removed` event, as both implementations already
leverage the `FakeFs.remove_file_inner` and `FakeFs.remove_dir_inner`,
respectively, which already emits this event, so there's no need to emit
it again.
@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label Mar 20, 2026
@zed-community-bot zed-community-bot Bot added the staff Pull requests authored by a current member of Zed staff label Mar 20, 2026
@dinocosta dinocosta assigned dinocosta and unassigned dinocosta Mar 20, 2026
@dinocosta dinocosta linked an issue Mar 20, 2026 that may be closed by this pull request
Comment thread crates/fs/src/fs.rs Outdated
Explain why we decided to create our own type instead of just rolling
with `trash::TrashItem`, as well as ensuring that both
`TrashedEntry::name` and `TrashedEntry::original_parent` have a doc
comment.
@dinocosta dinocosta marked this pull request as ready for review March 25, 2026 19:29
@zed-codeowner-coordinator zed-codeowner-coordinator Bot requested review from a team, Veykril and as-cii and removed request for a team March 25, 2026 19:29
@dinocosta dinocosta requested review from yara-blue and removed request for Veykril and as-cii March 25, 2026 19:29
Remove `objc` and `cocoa` dependencies from `fs` crate, as they're no
longer required.
Remove the `options` argument from both `Fs::trash_dir` and
`Fs::trash_file` trait methods. Before the change to the trait method
return type, these options were only used by the default implementation,
as any other implementation of these methods as always using the default
options (`Default::default`) which sets all fields to `false`.

Since we no longer have any default implementation, it doesn't make
sense to continue accepting the `options` argument, as now it is truly
unused.

The only place where it made a difference was in the `FakeFs`
implementation, where it is passed to `FakeFs::remove_file_inner`, but
even then, we can simply have `recursive: true` in the
`FakeFs::trash_dir` implementation.
Comment thread crates/fs/Cargo.toml Outdated
Update the `trash` dependency repository from my personal fork to Zed's
own fork, as that is now updated with the necessary changes to allow
both tracking the trashed file's location as well as restore support for
macOS.
@zelenenka zelenenka removed the Size M label Apr 9, 2026
@dinocosta dinocosta merged commit b6562e8 into main Apr 9, 2026
30 checks passed
@dinocosta dinocosta deleted the 5039-fs-update branch April 9, 2026 16:42
piper-of-dawn pushed a commit to piper-of-dawn/zed that referenced this pull request Apr 25, 2026
Update both `Fs::trash_dir` and `Fs::trash_file` to now return the
location of the trashed directory or file, as well as adding the
`trash-rs` create dependency and updating the `RealFs` implementation
for these methods to simply leverage `trash::delete_with_info`.

* Add `fs::Fs::TrashedEntry` struct, which allows us to track the
  original file path and the new path in the OS' trash
* Update the `fs::Fs::trash_dir` and `fs::Fs::trash_file` signatures to
  now return `Result<TrashedEntry>` instead of `Result<()>`
* The `options` argument was removed because it was never used by
  implementations other than the default one, and with this change to
  the signature type, we no longer have a default implementation, so the
  `options` argument would no longer make sense
* Update `fs::RealFs::trash_dir` and `fs::RealFs::trash_file`
  implementations to simply delegate to `trash-rs` and convert the
  result to a `TrashedEntry`
* Add `fs::FakeFs::trash` so we can simulate the OS' trash during tests
  that touch the filesystem
* Add `fs::FakeFs::trash_file` implementation to leverage
  `fs::FakeFs::trash`
* Add `fs::FakeFs::trash_dir` implementation to leverage
  `fs::FakeFs::trash`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement staff Pull requests authored by a current member of Zed staff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot undo actions in the project panel

3 participants