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

:z4h:fzf-complete recurse-dirs interfers with -g and -/ argument to _files completion #209

Open
VorpalBlade opened this issue Mar 1, 2022 · 10 comments

Comments

@VorpalBlade
Copy link
Contributor

When using the _files completer (either directly or as a sub-completer to for example _argument), the -g and -/ flags to restrict to a glob or directories only doesn't work if :z4h:fzf-complete recurse-dirs is set to yes.

Strangely enough it DOES work if using the underlying _path_files directly, which seems to bypass the recurse-dirs style.

Example for reproducing:

$ cd ~
$ zstyle :z4h:fzf-complete recurse-dirs yes
$ touch a.foo b.foo
$ compdef '_files -g "*.foo"' blergh
$ blergh <tab> # Notice that it completes lots of things recursively in your home directory that do not end in .foo
$ zstyle :z4h:fzf-complete recurse-dirs no
$ blergh <tab> # Now it only completes *.foo, though of course only in the current directory.

The same can be observed with -/ which should only complete directories. I have not tested the other flags to _files, it is possible they are also bugged.

@VorpalBlade
Copy link
Contributor Author

VorpalBlade commented Mar 1, 2022

With some further investigation it seems that the filter is correctly applied for files in the current directory but not for "recursive results" (that is: files or directories in sub-directories)

Also: having poked around in the code for a bit, I'm going to guess that either something is up with the call to -z4h-find in -z4h-comp-files or with -z4h-find itself.

Also also: as -z4h-find depends on either find or bfs I tried installing bfs to see if it would make a difference. Unless some cache needs to be clearer (by something more than just opening a new shell), it did not make a difference.

There is also a relevant TODO in _path_files in z4h-fzf-complete. As _path_files seems to work I'm slightly confused as to why it would appear to work properly.

@romkatv
Copy link
Owner

romkatv commented Mar 1, 2022

See:

# TODO: respect pats and ignore somehow.
local -a opts files dirs pats ignore
zparseopts -a mopts f=files /=dirs g+:=pats F:=ignore \
P: S: q r: R: W: M+: J+: V+: x+: X+: 1 2 o+: n
(( ${#files} || ! ${#dirs} )) && typeset -gi _z4h_only_dirs=0

It's possible to fix that TODO but it's not trivial.

If you need to complete directories, use _directories instead of _files -/.

@romkatv
Copy link
Owner

romkatv commented Mar 1, 2022

Also also: as -z4h-find depends on either find or bfs I tried installing bfs to see if it would make a difference. Unless some cache needs to be clearer (by something more than just opening a new shell), it did not make a difference.

Installing bfs changes the order of elements in recursive traversal. It becomes BFS instead of DFS -- a pretty big improvement in usability, so I highly recommend installing bfs if you can.

@romkatv
Copy link
Owner

romkatv commented Mar 1, 2022

Unrelated to this issue but might be relevant to you in general: #35 (comment) and #35 (comment)

@VorpalBlade
Copy link
Contributor Author

Installing bfs changes the order of elements in recursive traversal. It becomes BFS instead of DFS -- a pretty big improvement in usability, so I highly recommend installing bfs if you can.

Cool! Are there any other optional dependencies of z4h? Since it is mostly undocumented I assume yes, but it would still be nice to know still.

It's possible to fix that TODO but it's not trivial.

Hm, if and when I get time I might give that a go. I assume the general idea would be to extract the patterns & ignores into variables that are then used in -z4h-comp-files to limit what files are found by passing the relevant flags to -z4h-find and then bfs/find in turn.

If you need to complete directories, use _directories instead of _files -/.

Okay, it isn't documented in man zshcompsys and many of the bundled completions in zsh and zsh-completions already use _files -/. As do many of the completions bundled by various system packages:

$ grep -RF '_files -/' /usr/share/zsh | wc -l
474
$ grep -RF '_directories' /usr/share/zsh | wc -l
308

It would be nice if those worked as is. Also _directories is normally implemented in terms of _files (haven't checked if z4h overrides that):

$ cat  /usr/share/zsh/functions/Completion/Unix/_directories
#compdef dircmp -P -value-,*path,-default-

local expl

_wanted directories expl directory _files -/ "$@" -

@romkatv
Copy link
Owner

romkatv commented Mar 1, 2022

Cool! Are there any other optional dependencies of z4h? Since it is mostly undocumented I assume yes, but it would still be nice to know still.

Other optional dependencies are tmux and git but those are a lot more obvious.

It's possible to fix that TODO but it's not trivial.

Hm, if and when I get time I might give that a go. I assume the general idea would be to extract the patterns & ignores into variables that are then used in -z4h-comp-files to limit what files are found by passing the relevant flags to -z4h-find and then bfs/find in turn.

Applying pats and ignore patterns would need to be done after find/bfs because zsh patterns cannot in general be converted to find command line arguments. There is already a zsh process that goes over find output before passing it on to fzf and that code is very heavily optimized due to its being the bottleneck in recursive completions. Any pattern-based filtering will need to go there and will need to be heavily optimized, too. The code I'm referring to is in fn/-z4h-present-files.

It would be nice if those worked as is.

This is fixed.

@VorpalBlade
Copy link
Contributor Author

[...] zsh patterns cannot in general be converted to find command line arguments [...]

Would it be feasible/worth it trying to detect when it is possible and do a fast-path using find/bfs flags and a slow path when it isn't possible? Looking at the uses in /usr/share/zsh, in many cases -g is simply used to filter on file extensions, and can be converted to either -name or -iname.

There are absolutely some more complex cases though. Also, ignores seem more difficult in general.

@romkatv
Copy link
Owner

romkatv commented Mar 1, 2022

[...] zsh patterns cannot in general be converted to find command line arguments [...]

Would it be feasible/worth it trying to detect when it is possible and do a fast-path using find/bfs flags and a slow path when it isn't possible?

Yes, definitely. It would also be OK to handle common cases only in the fast path. It might even be better (fast completions with too many options can be preferred to slow completing). That's also how I handled _files -/ -- by special-casing some -g arguments.

@VorpalBlade
Copy link
Contributor Author

Another thought about optimisation (this time for ignore patterns): Looking at -z4h-present-files it seems to handle one file/directory result at a time. For handling ignores (as long as it is possible to convert, I haven't really looked into the gory details of this yet) it seems to me like it would be better to handle it earlier, even using something like find ... | grep -Ev "pattern". Some rather crafty anchoring rules might be required. Alternatively awk or sed could be used to split on the last component and then match on that.

However, premature optimisation etc... First implement. Then profile. Then optimise if needed. Not sure if or when I might get around to trying my hands at that.

@romkatv
Copy link
Owner

romkatv commented Mar 2, 2022

Another thought about optimisation (this time for ignore patterns): Looking at -z4h-present-files it seems to handle one file/directory result at a time.

The common case is vectorized. This is the code:

modes=("${(@)modes:/(#m)*/${_z4h_mode_codes[$((MATCH & 64075))]:-$nil}}")
while true; do
i=${modes[(i)@*]}
if (( i == $#modes + 1 )); then
printf '%3$s\0\e[%2$sm%3$q\e[0m%1$s\n' "${(@0)${(@)modes:^files}}"
break
elif (( i != 1 )); then
printf '%3$s\0\e[%2$sm%3$q\e[0m%1$s\n' "${(@0)${(@)modes[1,i-1]:^files}}"
shift $((i-1)) modes files
fi
file=$files[1]
shift modes files
if ! zstat -L -A link +link -- $file; then
printf '%1$s\0\e['$or'm%1$q\e[0m\n' $file
elif ! zstat -A link_mode +mode -- $file; then
printf '%1$s\0\e['$or'm%1$q\e[0m -> \e['$or'm%2$q\e[0m\n' $file $link
elif (( ln_target )); then
printf '%1$s\0\e[%4$sm%1$q\e[0m -> \e[%4$sm%2$q\e[0m%3$s\n' \
$file $link "${(@0)${_z4h_mode_codes[$((link_mode & 64075))]:-$nil}}"
else
printf '%1$s\0\e['$ln'm%1$q\e[0m -> \e[%4$sm%2$q\e[0m%3$s\n' \
$file $link "${(@0)${_z4h_mode_codes[$((link_mode & 64075))]:-$nil}}"
fi
done

It processes all non-symlinks as a vector and only symlinks are processed one by one.

For handling ignores (as long as it is possible to convert, I haven't really looked into the gory details of this yet) it seems to me like it would be better to handle it earlier, even using something like find ... | grep -Ev "pattern".

We are on the same page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants