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

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

Closed
paw-lu opened this issue Jun 10, 2020 · 28 comments
Labels
bug Something isn't working

Comments

@paw-lu
Copy link

paw-lu commented Jun 10, 2020

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

  • OS + distribution: macOS 10.15.5
  • Zsh version: 5.7.1
  • Plugin version: 33d9812
@paw-lu paw-lu added the bug Something isn't working label Jun 10, 2020
@marlonrichert
Copy link
Owner

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
Copy link
Author

paw-lu commented Jun 10, 2020

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-autocompletecd dir/** [TAB]
>
3/3
> dir/file1.txt
> dir/file2.txt
> dir/file3.txt
# With zsh-autocompletecd 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.

  • ❯ kill {args} [TAB]
  • ❯ {some pattern}** [TAB]

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
Copy link

ztNFny commented Jun 10, 2020

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
Copy link

ztNFny commented Jun 10, 2020

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 added a commit that referenced this issue Jun 11, 2020
marlonrichert added a commit that referenced this issue Jun 11, 2020
marlonrichert added a commit that referenced this issue Jun 12, 2020
marlonrichert added a commit that referenced this issue Jun 15, 2020
marlonrichert added a commit that referenced this issue Jun 15, 2020
@marlonrichert
Copy link
Owner

marlonrichert commented Jun 15, 2020

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
Copy link
Owner

@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
Copy link
Author

paw-lu commented Jun 15, 2020

@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

  • kill [TAB] works as expected now (with fzf taking over)
  • Trigger sequences work (**) with fzf

👎

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

  • Directories trigger fzf instead of "normal" autocomplete or zsh-autocomplete.

    $ ls
    dir    folder    container
    
    # Without zsh-autocomplete
    $ cd con[TAB]
    
    $ cd container
    
    # With zsh-autcomplete
    $ cd con[TAB]
    >
    3/3
    dir
    folder
    container
  • Arguments no longer complete upon pressing tab

    # Without zsh-autocomplete
    $ brew ins[TAB]
    
    $ brew install
    
    # With zsh-autocomplete
    $ brew inst[TAB]
    install   install a formula
    install-bundler-gems
    
    $ brew inst
    install   install a formula
    install-bundler-gems

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

@marlonrichert
Copy link
Owner

marlonrichert commented Jun 16, 2020

  • 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 Homebrew/brew#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
Copy link
Owner

marlonrichert commented Jun 16, 2020

@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
Copy link
Author

paw-lu commented Jun 16, 2020

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
Copy link
Owner

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?

marlonrichert added a commit that referenced this issue Jun 16, 2020
@paw-lu
Copy link
Author

paw-lu commented Jun 16, 2020

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
Copy link
Owner

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 added a commit that referenced this issue Jun 16, 2020
@marlonrichert
Copy link
Owner

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

@paw-lu
Copy link
Author

paw-lu commented Jun 16, 2020

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

👍

tab features seem to be working as expected

  • kill [TAB] works as expected (with fzf taking over)
  • Trigger sequences work (**) with fzf
  • Directories autocomplete with tab
  • Arguments autocomplete with tab

marlonrichert added a commit that referenced this issue Jun 17, 2020
@marlonrichert
Copy link
Owner

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

marlonrichert added a commit that referenced this issue Jun 18, 2020
@paw-lu
Copy link
Author

paw-lu commented Jun 19, 2020

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

@ztNFny
Copy link

ztNFny commented Jun 19, 2020

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

@ztNFny
Copy link

ztNFny commented Jun 24, 2020

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

@ztNFny
Copy link

ztNFny commented Jun 24, 2020

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.

@paw-lu
Copy link
Author

paw-lu commented Jun 25, 2020 via email

@ztNFny
Copy link

ztNFny commented Jun 25, 2020

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
Copy link
Owner

marlonrichert commented Jun 25, 2020

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
Copy link
Owner

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

@ztNFny
Copy link

ztNFny commented Jun 25, 2020

@marlonrichert fish doesn't insert aliases.

@marlonrichert
Copy link
Owner

marlonrichert commented Jun 25, 2020

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

@marlonrichert
Copy link
Owner

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

marlonrichert added a commit that referenced this issue Jun 26, 2020
* Address issue #51.
* Improve `menu-select` logic.
* Add a setting to disabled `fzf` integration. Address issue #59.
@marlonrichert
Copy link
Owner

Merged to master.

marlonrichert added a commit that referenced this issue Jun 26, 2020
* Address issue #51.
* Improve `menu-select` logic.
* Add a setting to disable `fzf` integration. Address issue #59.
marlonrichert added a commit that referenced this issue Jun 26, 2020
* Address issue #51.
* Improve `menu-select` logic.
* Add a setting to disable `fzf` integration. Address issue #59.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants