Skip to content

fs: Add support for restoring trashed files#52014

Merged
dinocosta merged 25 commits into
mainfrom
5039-fs-restore
Apr 9, 2026
Merged

fs: Add support for restoring trashed files#52014
dinocosta merged 25 commits into
mainfrom
5039-fs-restore

Conversation

@dinocosta
Copy link
Copy Markdown
Member

Context

With #52012 adding support for tracking the trashed file's location, we can now easily leverage that information in order to restore the file back to its previous location, as fs::TrashedEntry contains all the necessary information to perform this.

As such, the changes in this Pull Request update the fs::Fs trait with a new method, restore, which attempts to restore the trashed file back to its original location.

Note that this Pull Request will remain in Draft until the corresponding base Pull Request, #52012, is merged.

How to Review

  1. Review the fs::Fs::restore method signature
  2. Review the fs::Fs::TrashRestoreError enum, listing all errors we currently identify when attempting to restore a trashed file
  3. Review the fs::Fs::restore implementations in both fs::RealFs and fs::FakeFs
    • In the case of fs::RealFs it just delegates to the trash-rs implementation
    • In the case of fs::FakeFs it leverages the fs::FakeFsState struct in order to simulate an actual move from the trash to the original location

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

dinocosta added 12 commits March 1, 2026 23:02
- 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.
* Introduce a new `restore` method to the `Fs` trait, meant to be used
  to restore a trashed entry (directory or file).
* Implement `Fs.restore` for `RealFs` by simply leveraging
  `trash::restore_all`
  * This also meant introducing `TrashedEntry.into_trash_item` so we can
    easily convert the `TrashedEntry` to a `trash::TrashItem`
* Implement `Fs.restore` for `FakeFs` to actually leverage the existing
  `FakeFsState`, so we can actually simulate entries being trashed and
  restored
  * This will come in handy when adding undo/redo support for the
    Project Panel, so that we can actually test how the panel behaves
    when a collision happens, for example
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.
* Introduce `fs::TrashRestoreError` so we don't depend on the
  `trash::Error` enum
* Update all `Fs.restore` implementations to leverage this new error
  enum
* Update existing `FakeFs.restore` test to match on
  `fs::TrashRestoreError` rather than `trash::Error`
* Remove `objc` and `cocoa` dependencies from `fs` crate, as they're no
  longer required
* Update the `test_fake_fs_restore` test in order to assert that the
  file contents are not changed when there's a restore collision
@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
Comment thread crates/fs/Cargo.toml 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.
Remove `objc` and `cocoa` dependencies from `fs` crate, as they're no
longer required.
@dinocosta dinocosta marked this pull request as ready for review March 25, 2026 19:43
@dinocosta dinocosta self-assigned this Mar 25, 2026
@dinocosta dinocosta requested a review from yara-blue March 25, 2026 19:43
@zed-codeowner-coordinator zed-codeowner-coordinator Bot requested a review from a team March 25, 2026 19:43
@zed-codeowner-coordinator zed-codeowner-coordinator Bot requested review from as-cii and removed request for a team March 25, 2026 19:43
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.
Copy link
Copy Markdown
Member

@yara-blue yara-blue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming the actual testing of the trash impl happens in trash-rs this is great :)

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.
The `FakeFs::trash_file` and `FakeFs::trash_file` implementations no
longer accept options, so the `test_fake_fs_restore` test has been
updated accordingly.
Copy link
Copy Markdown
Member

@yara-blue yara-blue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call on removing the RemoveOptions, cleans things up nicely :).

@zelenenka zelenenka removed the Size M label Apr 9, 2026
Base automatically changed from 5039-fs-update to main April 9, 2026 16:42
@dinocosta dinocosta merged commit fd285c8 into main Apr 9, 2026
31 checks passed
@dinocosta dinocosta deleted the 5039-fs-restore branch April 9, 2026 17:01
piper-of-dawn pushed a commit to piper-of-dawn/zed that referenced this pull request Apr 25, 2026
Introduce a new `fs::Fs::restore` method which, given a
`fs::TrashedEntry` should attempt to restore the file or directory back
to its original path.
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.

3 participants