Closed 3ximus closed 4 months ago
Well, on the flip side, one could argue that fzf-tmux should not use an alias because there is no way to know if it is compatible with fzf-tmux, so in that sense using the original binary is considered to be a safer choice. This is why you see so many command something
s in the shell integration scripts.
I suggest that you set up $FZF_DEFAULT_OPTS
instead.
I get your point but I think I didn't make my issue clear enough.
With command -v fzf
it can produce output that's not a valid command (in the case it is an alias). So putting it like this: $(command -v fzf)
will generate and error if fzf is an alias.
Using which
however works better for this use case as it will detect the binary in the path instead of printing out the alias for that name, and I don't think it creates any side effects.
I understand the use for command to access the original binary but in that specific case you're trying to get the binary path, so command -v
will not always produce a path and will error out the subshell.
The reason why I didn't use $FZF_DEFAULT_OPTS
is that it is used in all instances of fzf, including inside vim which I would prefer not to affect. I bumped into this problem since I'm loading my aliases on shell commands inside vim and it's getting into fzf-tmux script.
If you prefer to keep it the way it is I've already made the changes in my fork, and really I just though of letting you know that it can cause issues with this specific use case.
Thanks for your hard work :heart: Have a good day!
So putting it like this:
$(command -v fzf)
will generate and error if fzf is an alias.
I see the point. which
seems like a safer choice.
However, I don't understand why you have aliases in a non-interactive session that runs fzf-tmux script. Because aliases are meant to be used only in interactive sessions and are usually not loaded on a non-interactive session.
$ alias | wc -l
38
$ bash -c 'alias | wc -l'
0
$ bash -ci 'alias | wc -l'
38
Any idea why?
The reason why I didn't use
$FZF_DEFAULT_OPTS
is that it is used in all instances of fzf, including inside vim which I would prefer not to affect.
FYI, you can override the variable in your Vim configuration file. e.g. let $FZF_DEFAULT_OPTS = '--no-color'
However, I don't understand why you have aliases in a non-interactive session that runs fzf-tmux script. Because aliases are meant to be used only in interactive sessions and are usually not loaded on a non-interactive session
I actually don't load aliases on all non-interactive shells, only within vim using (let $BASH_ENV="~/.bash/aliases.sh"
). It let's me use aliases on vim command line. Say I want to urlencode
a line and I have an alias for it I can just type :.!urlencode
. Comes in handy some times...
FYI, you can override the variable in your Vim configuration file. e.g. let $FZF_DEFAULT_OPTS = '--no-color'
Oh that's a good point, I might use it instead of aliasing fzf.
I see. Anyway, whatever the case, fzf-tmux should not use alias fzf=...
output.
Should this project rely on the external which
package?
I'm not sure what your stand is on POSIX compatibility, but since which
is still an external package (that is just preinstalled by the most common distributions), shouldn't there be a fallback to command -v
?
fzf="$(command which fzf)" || fzf="$(command -v fzf 2> /dev/null)" || fzf="$(dirname "$0")/fzf"
PS: The changes of 90d7e38909bc704afea96e23acf93ba05ef9f0af work fine on my system. I just spotted the change and remembered a similar problem in another project where a user hadn't which
installed.
Checklist
man fzf
)Output of
fzf --version
0.42.0 (debian)
OS
Shell
Problem / Steps to reproduce
My problem is that the check for the fzf executable doesn't take into account if the
fzf
name is aliased. This is the current check: https://github.com/junegunn/fzf/blob/f97d2754134607b24849fc4a2062dbfcaafddd6a/bin/fzf-tmux#L10 Which iffzf
is alias would set fzf to the value:alias fzf='fzf --reverse
for example instead of getting the actual executable. My suggestion would be to usewhich fzf
instead ofcommand -v fzf 2>/dev/null
. I can make a PR but I wanted to ask first if this is there for a reason, or if there are any side effects from changing it.To reproduce the error: