search

marlonrichert / zsh-autocomplete

🤖 Real-time type-ahead completion for Zsh. Asynchronous find-as-you-type autocompletion.
MIT License
5.38k stars 148 forks source link

fzf trigger sequence (**) does not work as expected (even using control + space) #51

Closed paw-lu closed 4 years ago

paw-lu commented 4 years ago

This is some really cool work. Thanks for sharing it!

Describe the bug

The fzf search menu no longer seems to pop up when the trigger sequence is used when zsh-autocomplete is enabled, even when using the new shortcut—control␣.

To Reproduce

Steps to reproduce the behavior:

% zsh -df

% [ -f ~/.fzf.zsh ] && source ~/.fzf.zsh

% cat dir/** [TAB]
>
3/3
> dir/file1.txt
> dir/file2.txt
> dir/file3.txt

% source ~/dir/dir/zsh-autocomplete.plugin.zsh

% cat dir/** [CTRL][SPACE]
file1.txt    file2.txt    file3.txt

Of course, this fixes itself if I run zstyle ':autocomplete:tab:*' completion fzf. tab is now controlled by fzf, and the trigger sequence ** works as expected. But then normal autocomplete seems broken at that point.

Expected behavior

I thought control␣ (with zsh-autocomplete) would trigger the kind of behaviour fzf would trigger by hitting tab normally (without zsh-autocomplete).

Desktop

marlonrichert commented 4 years ago

I thought control␣ (with zsh-autocomplete) would trigger the kind of behaviour fzf would trigger by hitting tab normally (without zsh-autocomplete).

Well, there's the crux of the matter: I never promised any such thing. 😉

This is completely intentional. Since you now have one key binding for normal completion and one key binding for fzf completion, you really don't need the "trigger" sequence anymore. You can now choose on the spot which kind of completion style you want.

However, I could make it so that zstyle ':autocomplete:tab:*' completion fzf provides a sort "smart" behavior that was suggested here by @ztNFny.

Let's discuss: How should this really behave? There's more than one way to go about this and there is no obvious "right" one. 🙂

paw-lu commented 4 years ago

Well, there's the crux of the matter: I never promised any such thing. 😉

Misunderstanding on my part! 😅 So—if I'm understanding correctly—control+␣ is essentially the same as control+T (the default keyboard shortcut fzf creates)?

I think the main issue on my end is that ** triggers some context-specific actions. For example: when places after a path, fzf will only search in that path.

# Without zsh-autocomplete
❯ cd dir/** [TAB]
>
3/3
> dir/file1.txt
> dir/file2.txt
> dir/file3.txt
# With zsh-autocomplete
❯ cd dir/[CTRL] [SPACE]
>
3/3
> dir/file1.txt
> dir/file2.txt
> dir/file3.txt
> folder/file1.txt
> folder/file2.txt
...

Similarly, there are a few other smart completion features, like a custom fuzzy completion API.

If feasible and maintainable, a smart behavior for zstyle ':autocomplete:tab:*' completion fzf would probably be my ideal use case.

My ideal API is a bit different from the one outlined. As far as I know, as outlined in the documentation, there are only a few cases where hitting tab activates fzf.

From my understanding, only the kill command completely takes over the tab keypress, the rest explicitly lookout for the trigger sequence (**) in this case.

For the smart behavior for zstyle ':autocomplete:tab:*' completion fzf, it seems like it would be ideal to whitelist kill and the trigger sequence(**), and then I think fzf would act as expected.

Thanks for getting back to me and hearing out my thoughts!

ztNFny commented 4 years ago

fzf has 4 different completion methods: [TAB] fzf-completion [Ctrl]+[T] fzf-file-widget [Alt]+[C] fzf-cd-widget [Ctrl]+[R] fzf-history-widget

I think (not sure) what you end up with with this plugins [Ctrl]+[Space] is fzf-file-widget and that's not behaving as I want it to. So what I did for now is to add "bindkey '^ ' fzf-completion" to my .zshrc which rebinds [Ctrl]+[Space] to fzf's [Tab]-action.

I'm testing this primarily will "kill"-command right now and there it gives me an acceptable behaviour.

ztNFny commented 4 years ago

For the smart behavior for zstyle ':autocomplete:tab:*' completion fzf, it seems like it would be ideal to whitelist kill and the trigger sequence(**), and then I think fzf would act as expected.

That'd be a nice solution imho, but the "ugly" part is that it requires to copy all the special rules from fzf and make sure to keep them updated (not sure how often they change)

marlonrichert commented 4 years ago

So—if I'm understanding correctly—control+␣ is essentially the same as control+T (the default keyboard shortcut fzf creates)?

No, it's not. It's actually ⌥␣fzf-cd-widget plus alias expansion plus insert longest common expansion prefix plus ⇥fzf-completion rolled into one. 🙂 If you're interested, you can find the code over here: https://github.com/marlonrichert/zsh-autocomplete/blob/dev/zsh-autocomplete.zsh#L710

It works as follows:

  1. If the command line is empty, it works as ⌥␣fzf-cd-widget.
  2. If it's on an alias, it expands it.
  3. If it's on a glob expression and there’s a list of expansions visible, it inserts the longest common prefix. (This part could use some refinement, since it can get stuck under certain circumstances when there's too many matches. I'm still figuring out how to work around this.)
  4. Otherwise, it purges the current shell word of all glob expressions and calls ⇥fzf-completion (which I've set to fall back on ⇤ when it fails).
marlonrichert commented 4 years ago

@paw-lu @ztNFny Can you please checkout the dev branch and try out the latest autocomplete behavior? I've made some enhancements and I'm wondering if this in any way changes your fzf integration needs.

paw-lu commented 4 years ago

@marlonrichert Thanks for the explanation on fzf-cd-widget—it really cleared things up!

I tried out dev. Thanks a lot for the work!

👍

fzf-specific tab features seem to be working as expected

👎

Pressing tab under "normal" expected circumstances leads to unexpected behavior.

So it seems that fzf works fine, but at the expense of the normally expected tab behavior.

marlonrichert commented 4 years ago
  • Directories trigger fzf instead of "normal" autocomplete or zsh-autocomplete.

zsh-autocomplete's tab completion never calls fzf-completion. It is called from control-space completion only.

On your command line, can you please run bindkey '^I'? If the output is "^I" fzf-completion, then there's something wrong with your setup. 🙂

  • Arguments no longer complete upon pressing tab

That has nothing to do with zsh-autocomplete. For whatever reason, brew's completion code is just ridiculously slow. You'll find that without zsh-autocomplete, completion for brew is even more unusable. Please complain at https://github.com/Homebrew/brew/issues/7749

In general, with zsh-autocomplete, if there are no completions listed, then pressing ⇥ will not do anything, because the list shows what it can complete at that moment in time. You either have to wait a moment or type more, until you get a list of completions. But once that list is visible, then tab completion should always be instantaneous (at least, on the dev branch right now).

marlonrichert commented 4 years ago

@paw-lu I did some testing and actually, brew completion is slow only when caching isn't use —and zsh-autocomplete turns on caching. In a clean install, brew completion is slow. However, if you add zsh-autocomplete and use brew commands a couple of times to let it build up the completion cache, then brew completion becomes faster. If brew completion is slow for you all the time, then I fear you really have not installed zsh-autocomplete correctly.

How exactly have you installed zsh-autocomplete?

paw-lu commented 4 years ago

I can confirm that you are correct about

❯ bindkey '^I'
"^I" fzf-completion

Is there an issue with that? That seems to be one of the default shortcuts that are bound when [ -f ~/.fzf.zsh ] && source ~/.fzf.zsh is run. It's what I would expect to be bound to ^+I.

I want to clarify about the Homebrew example. That was just a random command I chose and was not specific to homebrew. This happens across all commands. As far as I can tell, it is not "slow"—completion just isn't happening when hitting tab.

Just to clear things up I recorded my screen. First I show how directory and argument autocomplete normally work, then I activate zsh-autocomplete, then I try those two again. You can also see one way I "install" zsh-autocomplete in the recording.

marlonrichert commented 4 years ago

I can confirm that you are correct about

❯ bindkey '^I'
"^I" fzf-completion

Is there an issue with that?

Yes, there definitely is a major issue with that. It means that zsh-autocomplete has not been loaded correctly. Could you please share your .zshrc file with me, so I can try to figure out what has gone wrong?

paw-lu commented 4 years ago

Yeah no problem! 😄

I'm able to replicate this with an absolute minimum .zshrc. This is using oh-my-zsh.

# Minimum .zshrc via oh-my-zsh
plugins=(
    zsh-autocomplete
)
[ -f ~/.fzf.zsh ] && source ~/.fzf.zsh
zstyle ':autocomplete:tab:*' completion fzf

Obviously, it follows that we can reproduce this issue with a completely blank .zshrc.

%  [ -f ~/.fzf.zsh ] && source ~/.fzf.zsh
% zstyle ':autocomplete:tab:*' completion fzf
% source zsh-autocomplete.zsh 
% bindkey '^I'
"^I" fzf-completion

Just for my understanding—I thought the goal here is to keep fzf running as expected and to keep zsh-autocomplete from "interfering" with its features. So I would naively think that fzf's default shortcuts should be surviving.

marlonrichert commented 4 years ago

zstyle ':autocomplete:tab:*' completion fzf

Ah, right, I completely forgot about that. 🤦🏽‍♂️ That explains the bindkey output.

Thanks! Now I know what to tweak.

marlonrichert commented 4 years ago

@paw-lu @ztNFny Alright, I pushed in a new update on the dev branch. Can you please try it out?

paw-lu commented 4 years ago

Gave it a quick look. As far as I can tell this seems good!

👍

tab features seem to be working as expected

marlonrichert commented 4 years ago

@ztNFny How do you feel about the current behavior on the dev branch?

paw-lu commented 4 years ago

I'm willing to close this @marlonrichert is everyone is comfortable with the changes.

ztNFny commented 4 years ago

I'll try to take a look tomorrow/Sunday. Busy at work right now.

ztNFny commented 4 years ago

@marlonrichert sorry, still hadn't had the chance, work is killing me atm

ztNFny commented 4 years ago

just had a quick look at dev, more hopefully on the weekend:

paw-lu commented 4 years ago

What's the new magic space behavior?

Paulo S. Costa

On Wed, Jun 24, 2020, 1:22 PM ztNFny notifications@github.com wrote:

just had a quick look at dev, more hopefully on the weekend:

  • kill [tab] looks good
  • so does FZF_COMPLETION_TRIGGER
  • the new default "magic" space behaviour is killing me, don't think that's a good default option imho. especially the alias seems uncalled for.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/marlonrichert/zsh-autocomplete/issues/51#issuecomment-649050403, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHFIKRQ2CC3GQUEYGG2JTNTRYJOBXANCNFSM4NZ7EMUA .

ztNFny commented 4 years ago

See README.MD in dev. Basically it expands pretty much anything. History, suggestions, alias (this i find the most annoying, the point of aliases is to not gave the original command), spellcheck. There's a config option but the default is to have it all on.

marlonrichert commented 4 years ago

Auto-expanding aliases is related to #60. I’ve read that it actually works like that in Fish. But I do agree that it’s not very nice when you have very long aliases. I’ll have to tweak that a bit still.

marlonrichert commented 4 years ago

But anyway, if the fzf-related behavior looks god to you, then I’ll merge that part soon to master.

ztNFny commented 4 years ago

@marlonrichert fish doesn't insert aliases.

marlonrichert commented 4 years ago

OK, good to know. I just read it somewhere. ¯\_(ツ)_/¯

marlonrichert commented 4 years ago

Ah, I just discovered that what I'm doing to address #60 is completely unnecessary. I'll remove it soon.

marlonrichert commented 4 years ago

Merged to master.