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 results contain bind commands #183

Closed
037g opened this issue Jul 3, 2021 · 17 comments · Fixed by #186
Closed

fzf results contain bind commands #183

037g opened this issue Jul 3, 2021 · 17 comments · Fixed by #186

Comments

@037g
Copy link

037g commented Jul 3, 2021

Describe the bug

adding fzf_configure_bindings --directory=\cf to config.fish adds additional lines of text to all fzf results:

bind --preset \\cv fish_clipboard_paste
bind --preset -M insert \\cv fish_clipboard_paste

Steps to Reproduce

Screenshots

example1

example2

image

Environment

Versions installed:

  • Fish: [e.g. 3.2.2] Fish Shell 3.3.0
  • fzf.fish: [e.g. 7.0] check
  • plugin manager: [e.g. fisher 4.3.0] check
  • terminal: [e.g. iTerm] ITerm 3.48
  • OS: [e.g. macOS 10.15.7] macOS BigSur 11.4

Additional context

Anything else?

@PatrickF1
Copy link
Owner

PatrickF1 commented Jul 3, 2021

Hmm that is very mysterious indeed. I double checked the code for fzf_configure_bindings and don't see any place where such output would escape.

  1. Does that output also show up when you first start a new fish shell?
  2. can you append 2>/dev/null to your invocation of fzf_configure_bindings in config.fish and see if it still happens?
  3. can you give me the output of bind --user please? (and also double check that they are what you expect)

@kidonng
Copy link
Contributor

kidonng commented Jul 3, 2021

These lines only appear in __fish_shared_key_bindings. Try remove fish_vi_key_bindings from your Fish config and see if the results are different.

Also, are you using tide?

@syphar
Copy link

syphar commented Jul 5, 2021

I also started seeing this message after some upgrades, fish and this plugin is up-to-date.

To answer some of the questions:

can you append 2>/dev/null to your invocation of fzf_configure_bindings in config.fish and see if it still happens?

the output when starting the shell still appears.

These lines only appear in __fish_shared_key_bindings. Try remove fish_vi_key_bindings from your Fish config and see if the results are different.

now I'm seeing:

bind: No binding found for sequence '\cv'
bind: No binding found for sequence '\cv'
bind: No binding found for sequence '\cv'
bind: No binding found for sequence '\cv'

can you give me the output of bind --user please? (and also double check that they are what you expect)

here is my output (with some custom bindings):

bind -M insert \cp vv
bind -M insert \ct ftags
bind -M insert \ce insert_heroku_app
bind -M insert \co cdp
bind -M insert \cq fg_
bind -M insert \cc kill-whole-line repaint
bind \cf _fzf_search_directory
bind \e\cl _fzf_search_git_log
bind \e\cs _fzf_search_git_status
bind \cr _fzf_search_history
bind \cv '_fzf_search_variables (set --show | psub) (set --names | psub)'
bind -M insert \cf _fzf_search_directory
bind -M insert \e\cl _fzf_search_git_log
bind -M insert \e\cs _fzf_search_git_status
bind -M insert \cr _fzf_search_history
bind -M insert \cv '_fzf_search_variables (set --show | psub) (set --names | psub)'

Seeing this, could this be a conflict between fzf_configure_bindings and fish_vi_key_bindings both trying to bind \cv?

@kidonng
Copy link
Contributor

kidonng commented Jul 5, 2021

Try running set -U fish_key_bindings fish_vi_key_bindings once instead of executing fish_vi_key_bindings in your config.fish.

@syphar
Copy link

syphar commented Jul 5, 2021

Try running set -U fish_key_bindings fish_vi_key_bindings once instead of executing fish_vi_key_bindings in your config.fish.

Then I'm seeing bind: No binding found for sequence '\cv', except when I also remove fzf_configure_bindings --directory=\cf.

@kidonng
Copy link
Contributor

kidonng commented Jul 5, 2021

At you using tide or anything that runs in background?

Put status -i || exit before your key bindings and try again.

@syphar
Copy link

syphar commented Jul 5, 2021

At you using tide or anything that runs in background?

yep, tide

Put status -i || exit before your key bindings and try again.

no error then. Interesting is that I still have my mappings.

@037g
Copy link
Author

037g commented Jul 5, 2021

I see a pull request to fix this. Thank you.

I am not using tide. Just starship.rs.

@PatrickF1

  1. Does that output also show up when you first start a new fish shell? no, only when some command calls the interactive tool

  2. can you append 2>/dev/null to your invocation of fzf_configure_bindings in config.fish and see if it still happens?
    Did not make a difference

  3. can you give me the output of bind --user please? (and also double check that they are what you expect)

Output of:

bind --user
bind -M insert '(' _pisces_insert_left\ \\\(\ \\\)
bind -M insert ')' _pisces_insert_right\ \\\)
bind -M insert '[' _pisces_insert_left\ \\\[\ \\\]
bind -M insert ']' _pisces_insert_right\ \\\]
bind -M insert '{' _pisces_insert_left\ \\\{\ \\\}
bind -M insert '}' _pisces_insert_right\ \\\}
bind -M insert '"' _pisces_insert_identical\ \\\"
bind -M insert \' _pisces_insert_identical\ \\\'
bind -M insert -k backspace _pisces_backspace
bind -M insert \x7f _pisces_backspace
bind -M insert \t _pisces_complete
bind -M insert . _puffer_fish_expand_dots
bind -M insert ! _puffer_fish_expand_bang
bind \cf _fzf_search_directory
bind \e\cl _fzf_search_git_log
bind \e\cs _fzf_search_git_status
bind \cr _fzf_search_history
bind \cv '_fzf_search_variables (set --show | psub) (set --names | psub)'
bind -M insert \cf _fzf_search_directory
bind -M insert \e\cl _fzf_search_git_log
bind -M insert \e\cs _fzf_search_git_status
bind -M insert \cr _fzf_search_history
bind -M insert \cv '_fzf_search_variables (set --show | psub) (set --names | psub)'

Other fish shell plugins.

omf l
Plugins
aws		fish-spec	omf

fisher list
jorgebucaran/fisher
edc/bass
laughedelic/pisces
jethrokuan/z
evanlucas/fish-kubectl-completions
markcial/upto
nickeb96/puffer-fish
h-matsuo/fish-color-scheme-switcher
PatrickF1/fzf.fish
patrickf1/colored_man_pages.fish

@037g
Copy link
Author

037g commented Jul 5, 2021

These lines only appear in __fish_shared_key_bindings. Try remove fish_vi_key_bindings from your Fish config and see if the results are different.

Also, are you using tide?

Removing vi bindings fixes the problem.

@Kabouik
Copy link

Kabouik commented Jul 11, 2021

I'm observing the same with no bindings set in my Fish config:

# Remove greeting
set fish_greeting

# fzf.fish keybindings (https://github.com/PatrickF1/fzf.fish)
set fzf_fd_opts --hidden --exclude=.git
fzf_configure_bindings --directory=\cF --git_status=\cS --git_log=\cG --history=\cR --variables=\cV

Screenshot_20210711_001
(Excuse the dpi, it's from a 5.9" phone.)

However, I'm not using Fish >3.2.0 yet because the package on Debian Sid currently is 3.1.3 only, don't know if this specific problem is related.
I am using Fish 3.3.1.

I'm realizing as I post this message that the config to show hidden directories/files with <c-f> doesn't work, but this is probably not related.

@kidonng
Copy link
Contributor

kidonng commented Jul 11, 2021

@Kabouik maybe use lowercase i.e. \cf instead of \cF.

@Kabouik
Copy link

Kabouik commented Jul 12, 2021 via email

@PatrickF1
Copy link
Owner

Hi @Kabouik! Sorry for the late reply. Can you try adding > /dev/null 2>&1 to your fzf_configure_bindings command and see if it still happens? It'll help me isolate the problem.

@PatrickF1
Copy link
Owner

PatrickF1 commented Jul 17, 2021

Ok actually, you don't need to do that. I thought about it some more and I know what's wrong. It's because you are passing the flag --variables=\cV, which uses $_fzf_search_vars_command. But _fzf_search_vars_command is no longer being set in conf.d/fzf.fish if the shell is interactive. Turns out @kidonng already realized this was a bug a while ago and tried to fix it in #186 but that hasn't been merged yet. Let me merge it and then you can run fisher update

PatrickF1 pushed a commit that referenced this issue Jul 17, 2021
This fixes #183, which is a resulting bug from #180.

## Explanation of the issue

#180 makes `$_fzf_search_vars_command` unavailable for non-interactive sessions. However, `fzf_configure_bindings` depends on that variable being available. If the user puts `fzf_configure_bindings` in their `config.fish`, `fzf_configure_bindings` gets executed in non-interactive sessions e.g. fzf preview windows or by tide, and the following line in the function...

https://github.com/PatrickF1/fzf.fish/blob/17d54b576919ee77644779db2dcae56d372a8830/functions/fzf_configure_bindings.fish#L33

...essentially becomes: 

```fish
test -n $key_sequences[5] && bind --mode $mode $key_sequences[5]
```

which results in error message like `bind --preset \cv fish_clipboard_paste` or `bind: No binding found for sequence '\cv'`.

## Solution
Don't execute the entirety of `fzf_configure_bindings` if not in interactive mode. And for good measure, quote `_fzf_search_vars_command` so that even if it does get executed, the bind command will run successfully.
@PatrickF1
Copy link
Owner

Sorry for everyone who had this issue...now I know why fzf.fish lost 5+ stars in the past 2 weeks :(

@Kabouik
Copy link

Kabouik commented Jul 18, 2021 via email

@PatrickF1
Copy link
Owner

Thanks @Kabouik!

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

Successfully merging a pull request may close this issue.

5 participants