Fix checking whether fzf is already in PATH

[tsiflimagas]

Description

The check in fzf plugin for whether it's sourced already was inverted. Let's correct it.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My code follows the code style of this project.
• If my change requires a change to the documentation, I have updated the documentation accordingly.
• I have read the CONTRIBUTING document.
• If I have added a new file, I also added it to clean_files.txt and formatted it using lint_clean_files.sh.
• I have added tests to cover my changes, and all the new and existing tests pass.

[davidpfarrell]

Greetings!

So having researched this a little bit, the answer is actually more nuanced than this PR.

There are literally dozens of ways to install fzf.

But there's really only one way to install it where it won't automatically appear on the PATH. That is by installing it through git clone $REPO ~/.fzf

For those who install via git clone, the ~/.fzf.bash script may contain the logic to add the binary to the PATH - For everyone else, the script won't contain that logic since it's already on the PATH.

As such, we can't use the presence (or lack of presence) of the command on the PATH as an indicator to check for the ~/.fzf.bash script, since it might be the script's job to add the command to the PATH, and being on the PATH doesn't necessarily mean the script has already been configured.

BUT we don't really need to do the remaining steps if the command is ultimately NOT on the path.

So I believe the correct answer here is to move the _command_exists fzf || return check to happen AFTER the ~/.fzf.bash sourcing but BEFORE The remaining configuration.

ie. move it to :

# ...

# For installation via git clone, the command may not be on the PATH until *after* the configuration script runs
if [ -r ~/.fzf.bash ] ; then
  source ~/.fzf.bash
elif [ -r "${XDG_CONFIG_HOME:-$HOME/.config}"/fzf/fzf.bash ] ; then
  source "${XDG_CONFIG_HOME:-$HOME/.config}"/fzf/fzf.bash
fi

# No need to continue if the command is not present
_command_exists fzf || return

if [ -z ${FZF_DEFAULT_COMMAND+x}  ] && _command_exists fd ; then
  export FZF_DEFAULT_COMMAND='fd --type f'
fi

# ...

/SIDE NOTE (not related to this PR)

Additionally, the about doc should probably b"configure" (vs "load"):

- about-plugin 'load fzf, if you are using it'
+ about-plugin 'configure fzf, if you are using it'

AND the project's homepage should be in the docs somewhere, preferably visible to the user when they inquire about the plugin.

[tsiflimagas]

Hi @davidpfarrell, thanks for your comment! I guess I acted frivolously on this and didn't see what I did didn't make any sense. Also, I can address your last point, if you want, since it's a small change and it could be squashed into this PR. I think informing the user about the project's homepage could be done the way it's done for zoxide.

[davidpfarrell]

Hi!

I'm approving this quick fix - If you wanted to take some time for a more-thorough revamp of the plugin, that would involve adding logging around the various steps and maybe updating the about text ...

Not required to make this change useful so I'll leave it up to you.

Thanks for the contribution !