-
-
Notifications
You must be signed in to change notification settings - Fork 93
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
feat(fb_actions.trash): add trash action #196
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
remove
action 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_removal
or something which could make the default binding ford
useactions.trash
instead ofactions.remove
.The
gio
command is a good backup because many Linux systems already have theglib2
package installed which provides it.I tested locally with only
trash
available, onlygio
available, 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