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

Fix bind error messages appearing in non-interactive windows #186

Merged
merged 6 commits into from
Jul 17, 2021

Conversation

kidonng
Copy link
Contributor

@kidonng kidonng commented Jul 5, 2021

This fixes #183, which is a resulting bug from #180. (Note that the offending commit is not authored by me.)

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...

test -n $key_sequences[5] && bind --mode $mode $key_sequences[5] $_fzf_search_vars_command

...essentially becomes:

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

Hi, thanks for discovering the root of the bug! Very clever, I was kinda stuck.
Wouldn't it be better to just add status -i || exit in fzf_configure_bindings?

@PatrickF1
Copy link
Owner

Yeah, fish doesn't even bind any of the preset bindings in non-interactive:

fish -c '                                                                                                       
         echo testing non-interactive mode bindings
         bind --user
     '
testing non-interactive mode bindings

So I think it makes sense for it to just do nothing.

@PatrickF1
Copy link
Owner

Ok I'm just going to go with adding status -i || exit to the configure_fzf_bindings rather than setting _fzf_search_vars_command even when in non-interactive. Reason being

  1. configure_fzf_bindings doesn't need to run in non-interactive anyway
  2. fish doesn't even bind any of the preset bindings in non-interactive
  3. this is the most performance optimal: it reduces the amount of code executed in both conf.d/fzf.fish and configure_fzf_bindings when in non-interactive mode

@PatrickF1 PatrickF1 changed the title Expose $_fzf_search_vars_command in non-interactive sessions Fix bind error messages appearing in non-interactive windows Jul 17, 2021
@PatrickF1 PatrickF1 merged commit a0882d0 into PatrickF1:main Jul 17, 2021
@kidonng
Copy link
Contributor Author

kidonng commented Jul 18, 2021

Sorry for the delay! I was going to reply right after your last comment, but somehow it just got put off.

Wouldn't it be better to just add status -i || exit in fzf_configure_bindings?

Sure that seems reasonable.

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 this pull request may close these issues.

fzf results contain bind commands
2 participants