junegunn / fzf

:cherry_blossom: A command-line fuzzy finder
https://junegunn.github.io/fzf/
MIT License
63.78k stars 2.37k forks source link

Make it possible to disable `Ctrl+T` / `Alt+C` / completions #3678

Closed elibarzilay closed 6 months ago

elibarzilay commented 6 months ago

This makes it possible to skip one of the above key bindings or completions by setting a variable to an empty string. For example,

FZF_CTRL_T_COMMAND= FZF_ALT_C_COMMAND= FZF_COMPLETION_TRIGGER= \
  eval "$(fzf --zsh)"

would leave only the history search. Makes it more convenient then producing the files and changing them myself to not include what I don't want.

This should be working, but for completeness it should also include at least some explaining comment at the top (since there's no docs about these vars?) and something equivalent for bash/fish.

junegunn commented 6 months ago

something equivalent for bash/fish

Can you update those files as well?

elibarzilay commented 6 months ago

something equivalent for bash/fish

Can you update those files as well?

Bash would be easy, but I don't have experience with fish so it'd need to wait until I have time to do it properly...

junegunn commented 6 months ago

Well, I wasn't too happy with it anyway, since it looks a bit random to just hook on one variable per disable-able feature.

Yeah, I kind of felt the same. But the idea of disabling a key binding if its COMMAND variable is intentionally set to an empty string doesn't seem too far-fetched and it's the simplest approach at hand. So I'm going to accept it.

The limitation of this approach though is that it doesn't address the need of some users to bind the feature to a different key. But it's not easy to come up with a solution that works consistently with all 3 shells because they all use different key expressions.

I'll take over this PR from here and finish it up (bash, fish, and documentation). Thanks.

junegunn commented 6 months ago

Merged, thanks!

elibarzilay commented 5 months ago

@junegunn Huge thanks for taking it over!

Some replies that are the least I owe you:

junegunn commented 5 months ago

So what was your motivation behind this? Which feature did you want to opt-out? I decided to make --*sh print both script files because I figured most of the users would want both.

  • Might be nice to replace the *COMMAND by some *KEY so you can both disable it or change the key that is used.

That would be ideal, but I wanted the variables to work consistently across three different shells to minimize the confusion, but it's not easy to achieve that with the approach because all three shells use different key expressions. ^T, \C-t, \ct.

bluz71 commented 5 months ago

I decided to make --*sh print both script files because I figured most of the users would want both.

Maybe I am not "most users" but I certainly wish fine grain control over the scripts. We are not quite there yet (from what I can see).

In the old mechanism there existed separate completion.bash and keybindings.bash scripts, and I only ever sourced the later (keybindings) and never the former (completion). Completion slowed my SHELL startup too much (about 20ms) whilst keybindings only added a couple ms. I don't need ** support.

In the new mechanism eval "$(fzf --bash)" I can not figure out how to not load completion.

Ideally I would like something like:

FZF_COMPLETION= eval "$(fzf --bash)"

I do want Ctrl-r, Ctrl-t, Alt-c but I do not want fuzzy ** completion.

bluz71 commented 5 months ago

I just saw FZF_COMPLETION_TRIGGER= eval "$(fzf --bash)", my bad. However, this has poor startup performance:

% time FZF_COMPLETION_TRIGGER= eval "$(fzf --bash)"
real    0m0.029s

Compared with old-school . $HOMEBREW_PREFIX/opt/fzf/shell/key-bindings.bash:

time . $HOMEBREW_PREFIX/opt/fzf/shell/key-bindings.bash
real    0m0.004s

29ms vs 4ms.

It would be nice to have true opt-out of Bash fzf ** completion.

junegunn commented 5 months ago

@bluz71

I just saw FZF_COMPLETION_TRIGGER= eval "$(fzf --bash)"

That part was not merged.

I can not figure out how to not load completion

eval "$(fzf --bash | sed '/### completion/,$d')"

29ms vs 4ms.

You don't have to use --bash, you can still use the old method if you prefer it. The files are still and will always be included in the package.

bluz71 commented 5 months ago

Thanks @junegunn, eval "$(fzf --bash | sed '/### completion/,$d')" works.

I assume that the the scripts on-disk will eventually go away?

I am satisfied.

bluz71 commented 5 months ago

Oh, the scripts will always be available, if so, then I will stick with those.

All good, sorry for the diversion.

junegunn commented 5 months ago

No problem, thanks for sharing your case.