sxyazi / yazi

💥 Blazing fast terminal file manager written in Rust, based on async I/O.
https://yazi-rs.github.io
MIT License
11.55k stars 269 forks source link

Show filename in prompt when deleting a file #827

Open cbr9 opened 3 months ago

cbr9 commented 3 months ago

Please describe the problem you're trying to solve

Hello, and thank you for creating yazi! Today I discovered it and ditched lf in favor of yazi :)

However, I have a request. I would like to see the name of the file I am about to delete, if it's just one, or even potentially a list of them if several are selected.

The point would be to avoid accidental deletions of important files

Would you be willing to contribute this feature?

Describe the solution you'd like

I'm not sure if delete_title can be a multiline string, or how that's rendered, but an idea would be to expose an extra variable, similar to lf's $fx or $fs. Then the prompt could be something like:

Delete the following files? [...list of files] (y/N)

Additional context

No response

sxyazi commented 3 months ago

The current deletion prompt is an input box, which cannot include multi-line file list information in the title. I plan to add a separate confirm component to achieve this, it looks like:

CleanShot 2024-03-20 at 00 23 20

"Are you sure you want to move this file to the trash?" can be replaced with multi-line text. (Screenshot from neo-tree, a file manager plugin in neovim)

But I haven't had enough time to complete it, would you like to give it a try? - If you're interested, I'll provide more details on how to implement it ;)

cbr9 commented 3 months ago

Sure, if I get some free time I can play around with it for a bit

sxyazi commented 3 months ago

Thank you, here are some implementation details:

  1. Copy /yazi-core/src/select/ as /yazi-core/src/confirm/, which is a relatively simple module and can be a good starting point.

  2. Add pub confirm: Confirm below pub input: https://github.com/sxyazi/yazi/blob/955e8f59a9461f0fd184315b36ce55fd7077d1ac/yazi-fm/src/context.rs#L5-L14

  3. Add self.cx.confirm.visible below self.cx.input.visible in Root for rendering the Confirm component: https://github.com/sxyazi/yazi/blob/955e8f59a9461f0fd184315b36ce55fd7077d1ac/yazi-fm/src/root.rs#L33-L36

  4. Add a new Confirm in Layer to indicate the layer level of this component, below Input: https://github.com/sxyazi/yazi/blob/955e8f59a9461f0fd184315b36ce55fd7077d1ac/yazi-shared/src/layer.rs#L6-L16

  5. Add a new [confirm] section in keymap.toml to define key bindings (confirm, cancel) for Confirm component:

    [confirm]
    keymap = [
        { on = [ "<Esc>" ], run = "close", desc = "Cancel the confirm" },
        { on = [ "n" ],     run = "close", desc = "Cancel the confirm" },
        { on = [ "y" ],     run = "close --submit", desc = "Submit the confirm" },
    ]
thelamb commented 2 weeks ago

This looked like a nice good first issue, so I decided to take a stab at it

I'm not familiar with Ratatui so it currently renders like this ;)

afbeelding

@sxyazi I'd appreciate feedback on the implementation so far. I think I only need to fix the rendering. I will take some time to play with Ratatui since I hope to contribute more to yazi as a way to learn Rust.

I will first try to tackle the layout and then render the message as a scrollable area like

afbeelding
sxyazi commented 2 weeks ago

Excellent! I'm really happy to see that you're interested in this FR and willing to take it on. Thank you so much!

The code looks good overall, very clean. I think the remaining work is just some fine-tuning of the UI and I've sketched out a layout:

screenshot-000441

There's a prompt at the top, the middle is a list of files, and at the bottom there are two buttons for confirming or canceling.

thelamb commented 2 weeks ago

I'm glad it looks good overall!

Last night I got the layout to this

afbeelding

I'm considering whether to render the list of files as a List (like Select), or to use a Paragraph that is scrollable. I'm leaning towards the Paragraph, and continue to have ConfirmCfg only store only message: String, and not a list of items.

Then any function in ConfirmCfg, like the delete/trash is responsible for converting the input to a single String.

sxyazi commented 2 weeks ago

I'm considering whether to render the list of files as a List (like Select), or to use a Paragraph that is scrollable.

We can store it internally as a ratatui::text::Text, i.e. message: Text, and render it as a scrollable Paragraph.

This will allow for greater flexibility so that it can be used elsewhere, like when we provide it as a Lua API similar to ya.input().

thelamb commented 2 weeks ago

Just a quick update: I'm happy with the current layout:

afbeelding

I'm still considering whether to introduce a horizontal scrollbar or if line-wrapping is sufficient. The vertical scrollbar is only shown if needed. The last 'big' task would be to add mouse support for clicking the buttons. I only had a brief look at the mouse handling logic and wasn't sure how to do it because most seems to be handled in Lua. I'll continue to look into this once I wrap up the rest.

The lua binding is also working well (I did not find where I can update the docs though)

Finally, I'm currently replacing the Input popup with Confirm in 2 other places:

I'm struggling a bit with the 'quit' scenario. I like the solution of using select! to check whether the tasks are completing while the popup is showing. However, the Confirm popup no longer implements the 'realtime' logic of Input (InputProxy::show returns UnboundedReceiver where ConfirmProxy::show (async) returns Result). I can't 'just' use this Future since it's moved in the Select

afbeelding

So I'm trying to see if ConfirmProxy::show() should return the oneshot::Receiver, but this doesn't have a recv() function like UnboundedReceiver, and I don't think it conceptually makes sense for the Confirm popup to have the same 'realtime' logic.

I clearly need to get into tokio/async logic more. I'll continue on this but I'd appreciate any suggestions to unblock this task :)

sxyazi commented 2 weeks ago

Thanks for the update!

I'm still considering whether to introduce a horizontal scrollbar or if line-wrapping is sufficient The last 'big' task would be to add mouse support for clicking the buttons

We can not add it for now and leave it for future expansion

I'll continue on this but I'd appreciate any suggestions to unblock this task

Could you submit it as a PR? I think I can get it fixed there quickly :)