zed icon indicating copy to clipboard operation
zed copied to clipboard

[UI] Cannot copy paste folders

Open devdumpling opened this issue 1 year ago • 2 comments

Check for existing issues

  • [X] Completed

Describe the bug / provide steps to reproduce it

When clicking copy paste in the editor, I am unable to copy entire folders. The actions go through, but there's no evidence of a paste anywhere.

Copying individual files work as expected, but not folders.

(Doesn't seem to matter whether I do it through hotkeys or the mouse. Also verified this happens on any repo I've opened)

Environment

Zed: v0.119.19 (Zed) OS: macOS 14.2.1 Memory: 16 GiB Architecture: aarch64

If applicable, add mockups / screenshots to help explain present your vision of the feature

https://github.com/zed-industries/zed/assets/2354127/e74f19e8-cdde-463e-b2b5-e86266c20702

If applicable, attach your ~/Library/Logs/Zed/Zed.log file to this issue.

If you only need the most recent lines, you can run the zed: open log command palette action to see the last 1000.

No response

devdumpling avatar Jan 26 '24 14:01 devdumpling

Additional context from my poking around:

The project_panel::Paste action is dispatched but the editor throws an error stating the folder already exists regardless of whether the it actually exists or not, see:

[2024-01-26T17:57:50Z ERROR util] crates/project_panel/src/project_panel.rs:928: "/Users/georgeguvamatanga/Documents/BRAIN_2/.obsidian/testfolder" already exists

No matter what folder I copied, and no matter where I pasted it, I could not get it to work. I verified that in all cases the folder I was trying to copy did not exist via Finder and ls.

georgemunyoro avatar Jan 26 '24 18:01 georgemunyoro

I think this is the issue:

In the copy_recursive function in crates/fs/src/fs.rs:

if metadata.is_dir {
    if !options.overwrite && fs.metadata(target).await.is_ok() { // <-- This line is the problem
        if options.ignore_if_exists {
            return Ok(());
        } else {
            return Err(anyhow!("{target:?} already exists"));
        }
    }

The intended logic of this code, as I understand it, is to see if we can overwrite the target, and if the target is there, decide if we should ignore it. But, there's an issue with how it checks for the target's existence.

When the target directory is non-existent, fs.metadata(target).await is not an Err(_). Instead, it is Ok(None). So .is_ok() will always be true unless some other kind of error is encountered. You can see below that fn.metadata returns Ok(None) when an io::ErrorKind::NotFound is encountered:

    async fn metadata(&self, path: &Path) -> Result<Option<Metadata>> {
        let symlink_metadata = match smol::fs::symlink_metadata(path).await {
            Ok(metadata) => metadata,
            Err(err) => {
                return match (err.kind(), err.raw_os_error()) {
                    (io::ErrorKind::NotFound, _) => Ok(None),  //  <-- See here
                    (io::ErrorKind::Other, Some(libc::ENOTDIR)) => Ok(None),
                    _ => Err(anyhow::Error::new(err)),
                }
            }
        };

Proposed solution

Change the line from:

if !options.overwrite && fs.metadata(target).await.is_ok() {

to:

if !options.overwrite && fs.metadata(target).await.is_ok_and(|m| m.is_some()) {

Making this change locally, I could copy and paste folders. The copy_recursive function isn't used anywhere else so it shouldn't break anything.

@maxbrunsfeld You made the latest change to this code in #1386. Could you kindly confirm if my understanding above is accurate, or utterly wrong 😅 . I'd love to fix this in a PR.

georgemunyoro avatar Jan 26 '24 22:01 georgemunyoro

Seeing this as well

feelingsonice avatar Jan 29 '24 06:01 feelingsonice

Linking this issue in for reference:

  • https://github.com/zed-industries/zed/issues/4983

JosephTLyons avatar Jan 29 '24 07:01 JosephTLyons