Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMD + Delete in the File Tree #7228

Closed
1 task done
shellscape opened this issue Feb 1, 2024 · 13 comments · Fixed by #8163 or #10875
Closed
1 task done

CMD + Delete in the File Tree #7228

shellscape opened this issue Feb 1, 2024 · 13 comments · Fixed by #8163 or #10875
Labels
enhancement [core label] good first issue Issue suitable for first-time contributors keymap / key binding Feedback for keyboard shortcuts, key mapping, etc project panel Feedback for files tree view

Comments

@shellscape
Copy link

shellscape commented Feb 1, 2024

Check for existing issues

  • Completed

Describe the feature

Within Atom and VS Code, it's possible to delete files without being prompted by using the CMD + Delete key combination. Emphasis on without being prompted. This is a pretty clutch feature that's key to productivity and speed of use within the editor.

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

FWIW in VS Code this sends something to the trash. A key combo which hard deletes, like rm -rf does`, without recovery would also be useful. It's usually 50/50 when I want to delete something to get it out of the way with the chance of needing it later, or wanting to nuke it from orbit.

The current delete on the file tree items does not send items to trash. Something that could probably use some discussion.

@shellscape shellscape added admin read Pending admin review enhancement [core label] triage Maintainer needs to classify the issue labels Feb 1, 2024
@JosephTLyons JosephTLyons added project panel Feedback for files tree view keymap / key binding Feedback for keyboard shortcuts, key mapping, etc and removed triage Maintainer needs to classify the issue admin read Pending admin review labels Feb 1, 2024
SomeoneToIgnore pushed a commit that referenced this issue Feb 22, 2024
…el. (#8163)

Fixes #7228

Release Notes:

- Added a `cmd-backspace` keybinding to delete files in the project panel ([7228](#7228))
@shellscape
Copy link
Author

@huacnlee @JosephTLyons I'm not sure if there's a regression in 0.125.0 or if the PR to resolve this misunderstood the issue.

Currently in v0.125.0 when I hit backspace/delete on a file in the tree, it does nothing where it used to prompt for delete. I now have to use CMD+delete. That's not ideal behavior.

The original intent was to match VSCode where:

  • using just the delete key will prompt for a delete with "Are you sure?"
  • using CMD+delete key will delete without a prompt

In both cases, the files should go to the Trash

@shellscape
Copy link
Author

@SomeoneToIgnore tagging you here since you merged, and this is possibly a regression.

@JosephTLyons
Copy link
Collaborator

@shellscape - I'll let Kirill respond about the changes to the deletion in the file tree. As for sending files to the trash can, I think we should probably be doing that:

@huacnlee
Copy link
Contributor

Actually I was do that PR was want remove confirm dialog when we triggered CMD-Backspace.

But then I reverted that to keep it. Because I don't sure that why Zed team make like that. So I just let Delete and CMD-Backspace can work. Keep the confirm dialog.

@shellscape
Copy link
Author

I see. On my end, using just delete no longer does anything.

@SomeoneToIgnore
Copy link
Contributor

Here's a video that I've recorded on the fresh Zed version with both bindings.

Screen.Recording.2024-02-29.at.21.31.33.mov

In the bottom left corner of the video, keybindings are shown:

  • backspace: ⌫
  • delete: ⌦
  • cmd: ⌘
  • escape: ⎋

Current settings state this

"delete": "project_panel::Delete",
"cmd-backspace": "project_panel::Delete",

and first, the video ensures that simple backspace does not trigger the deletion modal, then ensures that both cmd-backspace and delete work.

Then the video disables the binding for delete and tries the same: now to show, that both backspace and delete do not cause the modal to appear, only cmd-backspace does.


I see. On my end, using just delete no longer does anything.

So, both delete and cmd-backspace work.
Can you check your keybindings and ensure it's not the plain backspace that does nothing?

The PRs intent was to restore the missing (at that moment) delete binding, but apparently both the PR creator and myself were reading through this issue badly and did notice the dialogue-less emphasis, so let's reopen the issue for now.

@SomeoneToIgnore
Copy link
Contributor

This looks simple to fix:

Project panel responds to deletion action and opens the prompt here:

let answer = cx.prompt(
PromptLevel::Info,
&format!("Delete {file_name:?}?"),
None,
&["Delete", "Cancel"],
);

One could extend that Delete struct to accept prompt: boolean parameter and rework the action declaration similar to buffer_search::Delete:

#[derive(PartialEq, Clone, Deserialize)]
pub struct Deploy {
pub focus: bool,
}
impl_actions!(buffer_search, [Deploy]);

and then change the default keybindings (what was edited by the PR that tried to close this issue) to specify something like

"cmd-e": [
    "buffer_search::Deploy",
    {
        "focus": false
    }
],

but for the new case.

The most important part is to ensure that the old version, "delete": "project_panel::Delete", does still work and does the old, with confirmation query, deletion.
That is done via ensuring that the new field in project_panel::Delete will get a proper default value.

@SomeoneToIgnore SomeoneToIgnore added the good first issue Issue suitable for first-time contributors label Feb 29, 2024
@shellscape
Copy link
Author

Can you check your keybindings and ensure it's not the plain backspace that does nothing?

I've never modified them, but I do have this in the default bindings file:

      "delete": "project_panel::Delete",
      "cmd-backspace": "project_panel::Delete",

Although as stated above, the delete action does nothing on my machine. Glad to see there's a good fix possible!

mikayla-maki pushed a commit that referenced this issue Mar 18, 2024
…9452)

Completes #7228.
Adds back Backspace as the main delete key binding and makes Linux
bindings consistent with macOS

Release Notes:
- ⌘-Delete/⌘-Backspace will now suppress deletion confirmation prompts
in project panel
([#7228](#7228)).
@ConradIrwin
Copy link
Member

This shipped to 0.128.0-pre and will hit stable next week

@shellscape
Copy link
Author

@ConradIrwin appreciate getting this in! unfortunately this didn't fix the regression where delete/backspace (without the CMD key modifier) does bupkis. It "should" prompt for delete, but it doesn't.

@ConradIrwin ConradIrwin reopened this Mar 21, 2024
@mayfieldiv
Copy link
Contributor

@ConradIrwin appreciate getting this in! unfortunately this didn't fix the regression where delete/backspace (without the CMD key modifier) does bupkis. It "should" prompt for delete, but it doesn't.

I think this is finally fixed by #9590, which is in 0.129.0-pre

@shellscape
Copy link
Author

@mayfieldiv @ConradIrwin y'all still don't have this quite right. Delete without the CMD modifier sends a file(s) to the trash with VSCode, and that's the desirable behavior. CMD+Delete is straight to the ether, no trash, do not collect $200 dollars.

@JosephTLyons
Copy link
Collaborator

@mayfieldiv @ConradIrwin y'all still don't have this quite right. Delete without the CMD modifier sends a file(s) to the trash with VSCode, and that's the desirable behavior. CMD+Delete is straight to the ether, no trash, do not collect $200 dollars.

Sounds like these things can be consolidated:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement [core label] good first issue Issue suitable for first-time contributors keymap / key binding Feedback for keyboard shortcuts, key mapping, etc project panel Feedback for files tree view
Projects
None yet
6 participants