yardnsm / tmux-1password

:key: Access your 1Password login items within tmux!
MIT License
252 stars 24 forks source link

Plugin not using $FZF_DEFAULT_OPTS #17

Closed delucca closed 4 years ago

delucca commented 4 years ago

😰 Describe the bug

I've noticed that the plugin only uses $FZF_DEFAULT_OPTS environment variables if it is a string.

🧐 How to reproduce

For example, if I use the following:

export FZF_DEFAULT_OPTS="\
  --ansi \
  --cycle \
  --layout reverse \
  --height 20 \
  --preview '(highlight -O ansi -l {} 2> /dev/null || bat --style=numbers --color=always {} || cat {} || tree -C {}) 2> /dev/null | head -200' \
  --preview-window right:70%:noborder \
  --bind='?:toggle-preview' \
  --bind='tab:down'"

It shows my items in a reverse list with FZF. But, if I change to the following:

export FZF_DEFAULT_OPTS=(
  --ansi
  --cycle
  --layout='reverse'
  --height='20'
  --preview="'(highlight -O ansi -l {} 2> /dev/null || bat --style=numbers --color=always {} || cat {} || tree -C {}) 2> /dev/null | head -200'"
  --preview-window='right:70%:noborder'
  --bind='?:toggle-preview'
  --bind='tab:down'
)

It just stop working, showing a non-reverse list ignoring my other options.

🎉 Expected behavior

Using () on $FZF_DEFAULT_OPTS should work accordingly as if it were a string.

yardnsm commented 4 years ago

The latter one doesn't work at all with fzf for me 😕 Also, In fzf's documentation all of the example uses $FZF_DEFAULT_OPTS as a string.

This plugin uses fzf as usual and should follow the $FZF_DEFAULT_OPTS, so this issue might be related specifically to fzf.

delucca commented 4 years ago

I don't think it is related to FZF, because all other resources I use with FZF works as expected with the second example.

In order for the second example to work, you might need declare it inside your zshenv file, with the env set to zsh

yardnsm commented 4 years ago

I'm using fzf 0.20.0 (the latest) and zsh 5.7.1, and that does not seems to work :/

delucca commented 4 years ago

It appears that FZF doesn't really support this syntax. I've moved to a common string syntax :) Closing it for now