-
-
Notifications
You must be signed in to change notification settings - Fork 97
feat(fb_actions.trash): add trash action #196
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
base: master
Are you sure you want to change the base?
Conversation
137aabc to
4ed8eb7
Compare
|
Just fixed a small typo in the action docstsring. |
This action attempts to find a common trash executable (currently "trash" or "gio" in that order) and performs a blocking system call via vim.fn.system to trash each file or directory selected in the file browser. Like actions.remove, refuses to trash parent directory or current working directory.
4ed8eb7 to
6ef20dd
Compare
|
Fixed the style guide linter issues |
| if trash_cmd == "gio" then | ||
| cmd = { "gio", "trash", "--", p:absolute() } | ||
| else | ||
| cmd = { trash_cmd, "--", p:absolute() } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
trash from trash-cli does not accept --, so this makes the command always "fail" (although it does still trash the file).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That was a regression now fixed in the most recent version: andreafrancia/trash-cli#303 (comment)
Regardless I believe the -- is unnecessary since we are using absolute paths. Looking back I also think putting all the filenames into a single command would be much faster.
|
any progress on this one? |
|
tbh, I'm very hesitant on merging this. |
|
this really should be merged |
This action attempts to find a common trash executable (currently "trash" or "gio" in that order) and performs a blocking system call via vim.fn.system to trash each file or directory selected in the file browser. Like actions.remove, refuses to trash parent directory or current working directory.
More info:
I really love this plugin! I'm a big fan of avoiding unlinking and using the system trash as much as possible and I think this action would be a valuable asset to the plugin, especially for users who aren't great at Lua or searching GitHub issues but want to use trash for removal. I saw #169 and adapted the
removeaction and the work posted by @paveloom. I see there was some interest in adding it to the plugin if a good PR was written.I think adding a new action is the most appropriate because it completely avoids accidentally breaking
actions.remove. To make it even easier for users there could be an extension option calleduse_trash_for_removalor something which could make the default binding forduseactions.trashinstead ofactions.remove.The
giocommand is a good backup because many Linux systems already have theglib2package installed which provides it.I tested locally with only
trashavailable, onlygioavailable, and neither installed. Worked great for me. I'm running Linux so I don't know how effective this plugin would be on Mac and Windows, but if neither of those executables are there it will fail fast and do nothing with a nice warning message. Please feel free to correct me on anything.Prior art:
https://github.dev/ms-jpq/chadtree/blob/ec3f65d3556d0395d95af9db1f06eed266e00761/chadtree/transitions/delete.py#L89
https://github.dev/nvim-tree/nvim-tree.lua/blob/f8489c992998e1e1b45aec65bdb9615e5cd59a61/lua/nvim-tree/actions/fs/trash.lua#L69
Fixes #169