fzf icon indicating copy to clipboard operation
fzf copied to clipboard

zsh completion: Add support for per-command completion triggers.

Open rickyz opened this issue 6 years ago • 10 comments

Allow users to configure custom completion triggers on a per-command basis. For example, if FZF_COMPLETION_TRIGGER='**' and FZF_PER_CMD_COMPLETION_TRIGGERS=(ssh ''), then:

ssh triggers fzf completion.

but

ls will not.

Previously, completion for the kill command was special cased to not require a completion trigger. This patch reimplements kill in terms of FZF_PER_CMD_COMPLETION_TRIGGERS as well. Doing this also required modifying _fzf_complete to respect quotes when doing word splitting on its fzf_opts param.

This is a minor behavior change for kill completion. Previously, kill's hardcoded fzf invocation enabled the following options by default:

--height ${FZF_TMUX_HEIGHT:-50%} --min-height 15

and also allowed any options to be overriden by FZF_COMPLETION_OPTS. When reusing _fzf_complete, this will no longer happen (consistent with all of the other built-in completion functions).

rickyz avatar May 21 '18 22:05 rickyz

Thanks for your interest in the project. Actually, I regret making kill completion an exception, and I've considered many times if I should rectify the decision at some point, to always require **, even though it means breaking backward compatibility.

Making the completion system more configurable can benefit some users, but it will inevitably bring more complexity which I, the maintainer of this project and a mere bash user, cannot afford to handle at this point. Also note that when I add a new feature, I have to make sure that it works both on bash and zsh in a consistent manner and there are test cases for regular, and exceptional use cases. And once the new feature is added, I need to take extra caution not to break backward compatibility of the feature and I'll have to deal with questions, issue reports, and feature requests around it. In short, any decision cannot be made lightly.

I'll probably pass on this pull request, but I'll leave the pull request open for now, so that other users can notice the issue. Thanks.

junegunn avatar Jun 01 '18 03:06 junegunn

@junegunn can this be reconsidered? Bash seems to have the option to remove the ** trigger for certain commands and it would be nice to have the same option (albeit with implementation differences) for Zsh as well.

avamsi avatar Aug 13 '21 07:08 avamsi

Thanks a lot Rickyz I was trying to do that for some time :D

edsoncsouza avatar Sep 11 '21 09:09 edsoncsouza

For what it's worth, I would still be interested in merging this if the maintainers are willing. There have been some improvements made to fzf's zsh code since the original pull request, so the patch is now very simple compared to before. As far as I know, none of the caveats in the original pull request description apply to the latest version anymore.

rickyz avatar Sep 11 '21 21:09 rickyz

It might be a silly question but what does this line of code actually do? It appears to be removing the ending '' but on my PC, the trigger variable always equals to '' no matter what FZF_PER_CMD_COMPLETION_TRIGGERS is set to. And the PR is not working.

Thanks!

trigger=${FZF_PER_CMD_COMPLETION_TRIGGERS[$cmd]-${FZF_COMPLETION_TRIGGER-'**'}}

aur3l14no avatar Sep 28 '21 12:09 aur3l14no

@aur3l14no That line says to set trigger to the first non-null value out of $FZF_PER_CMD_COMPLETION_TRIGGERS[$cmd], $FZF_COMPLETION_TRIGGER, or '**'. See https://zsh.sourceforge.io/Doc/Release/Expansion.html#Parameter-Expansion for a description of this zsh syntax.

Can you provide a minimal zshrc that demonstrates your problem? I'm afraid I don't have enough information to explain why this isn't working for you.

rickyz avatar Sep 30 '21 00:09 rickyz

@rickyz Thanks for your explanations. My issue had something to do with zinit and its delayed loading mechanism and I have now solved it. The PR works great. Hope it will be merged soon : P

aur3l14no avatar Oct 07 '21 08:10 aur3l14no

Now that 52594355bfa63ce7d579c7961f4f2fb30b486101 has removed the special case for kill, this has effectively become a one-line change. I also added a simple test case.

It wouldn't be difficult to implement the same feature in bash - the main reasons I have not done so are:

  • Bash 3 (sadly still the version on a default macOS install) does not support associative arrays, so the configuration variable would need to be different.
  • There is already a working method to override the completion trigger on a per-command basis in Bash: https://github.com/junegunn/fzf/wiki/Examples-(completion)#bash-custom-trigger-less-completion

That said, I would be happy to implement bash support if that would help - we would just need to agree on the mechanism for specifying custom triggers.

rickyz avatar Sep 25 '22 21:09 rickyz

(Coming from #2026 again) @junegunn are you still not interested in this (now very small) PR? Is diverging support for Bash and Zsh the concern? That already seems to be the case, looking at https://github.com/junegunn/fzf/wiki/Examples-(completion) and arguably this PR makes the situation a little bit better.

avamsi avatar Jan 04 '23 14:01 avamsi

Upvoting, this does seem like a very useful feature and the changeset is small enough

TaiSHiNet avatar Feb 27 '24 19:02 TaiSHiNet