andreafrancia / trash-cli

Command line interface to the freedesktop.org trashcan.
GNU General Public License v2.0
3.53k stars 177 forks source link

ZSH completion very slow #335

Closed pinusc closed 3 months ago

pinusc commented 4 months ago

Describe the bug On my system, file completion is usually very slow for trash-put (and likely other commands).

trash-cli version 0.23.11.10

Are you using the latest version of trash-cli? Yes

Have you tried if the bug is present in the latest version of trash-cli? Yes

Operating system:

Expected behavior The completion for files should be snappy and instantaneous.

I have actually figured out where the issue is, but I am not sure of the easiest way to fix it.

In the completion file (obtained with ./trash-put --print-completion zsh), the following function:

_shtab_trash_put_options=(
  "(- : *)"{-h,--help}"[show this help message and exit]"
  "(- : *)--print-completion[print shell completion script]:print_completion:(bash zsh tcsh)"
  {-d,--directory}"[ignored (for GNU rm compatibility)]"
  {-f,--force}"[silently ignore nonexistent files]"
  {-i,--interactive}"[prompt before every removal]"
  {-r,-R,--recursive}"[ignored (for GNU rm compatibility)]"
  "--trash-dir[use TRASHDIR as trash folder]:trashdir:(${$(trash-list --trash-dirs)#parent_*:})"
  "*"{-v,--verbose}"[explain what is being done]"
  "(- : *)--version[show program\'s version number and exit]"
  "(*)::files:_trash_files"
)

calls trash-list --trash-dirs at evaluation time, because the line "--trash-dir[use TRASHDIR as trash folder]:trashdir:(${$(trash-list --trash-dirs)#parent_*:})" is enclosed in double quotes instead of single quotes. Changing the generated file to single quotes, (and removing the final :, I think) fixes the issue. Now trash-list --trash-dirs is only called when completing the arguments to --trash-dir , and it won't slow down completions otherwise.

~However, the fix won't be quite so easy, since the completion is being generated with shtab, and the relevant line is added manually (trashcli/shell_completion.py):~

~TRASH_DIRS.update({"zsh": "(${$(trash-list --trash-dirs)#parent_*:})"})~

~I think the easiest way is to move the logic to its own _trash_dir helper function, just as it's already being done for _trash_files. Then the relevant line above becomes "--trash-dir[use TRASHDIR as trash folder]:trashdir:_trash_dir, generated in python shtab with TRASH_DIRS.update({"zsh": "_trash_dir"}) (I think).~

~I would implement this myself, but I have not figured out how to write such a _trash_dir function, since the zsh compdef documentation is very dense and confusing... To anyone that knows the system, this should be pretty easy!~

Edit: although shtab doesn't give us control over using ' vs ", I realized I could just escape the variable/command expansion. It's a tiny fix, and I already submitted a PR for it.

This bug probably doesn't affect many people... but since there's network mounts on my system, and they sometimes fail, the call to trash-list --trash-dirs, which lists all system mounts, hangs. Nevertheless, it would still be more correct not to run that command at eval time but only when it is needed.

MarDiehl commented 4 months ago

same here.

pinusc commented 4 months ago

@MarDiehl I have now submitted a PR that will fix this. While it gets merged and then the updated completion files make it to whatever distribution you're using... feel free to use these updated completion files:

You can place them in /usr/share/zsh/site-functions, but it's probably better to place them somewhere in your home directory (e.g. ~/.zcompletions, taking care to have fpath=(~/.completions $fpath) in your .zshrc; this also ensures that they override any system-installed version).

andreafrancia commented 3 months ago

The PR was included manually, I think this issue should be solved.