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

Make sure script doesn't execute if fzf is an alias and the command doesn't actually exist #66

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

3ximus
Copy link
Contributor

@3ximus 3ximus commented Sep 18, 2024

This is a very simple fix for the case where you have an alias for fzf but the fzf command doesn't actually exist

Copy link
Collaborator

@akinomyoga akinomyoga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change suggested by this PR seems to disable the script under the presence of alias fzf regardless of whether the command actually exists. In particular, the script is canceled when both the alias fzf and the command fzf exist, but this seems slightly different from the PR title. Is this intentional?


Note: The following comments are unrelated to the above discussion but just suggest using idioms to check the existence of aliases.

@3ximus
Copy link
Contributor Author

3ximus commented Sep 20, 2024

Indeed @akinomyoga, just came here to correct that because I only noticed it today, I though I had tested correctly but must have made a mistake.

How about something like this? It should not detect aliases

type -P fzf >/dev/null || return

Copy link
Collaborator

@akinomyoga akinomyoga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That works and is much clearer! Thank you!

In this case, I think one wants to prefix command to the fzf invocations (e.g. in the following part for Bash) as command fzf so that it doesn't pick the alias if any.

_fzm_FZF_VERSION=$(fzf --version | awk -F. '{ print $1 * 1e6 + $2 * 1e3 + $3 }')
_fzm_MINIMUM_VERSION=16001
if [[ $_fzm_FZF_VERSION -gt $_fzm_MINIMUM_VERSION ]]; then
FZF_MARKS_COMMAND="fzf --height 40% --reverse"
elif [[ ${FZF_TMUX:-1} -eq 1 ]]; then
FZF_MARKS_COMMAND="fzf-tmux -d${FZF_TMUX_HEIGHT:-40%}"
else
FZF_MARKS_COMMAND="fzf"

@3ximus
Copy link
Contributor Author

3ximus commented Sep 24, 2024

I've added those in!
Cheers

@urbainvaes
Copy link
Owner

Thank you very much @3ximus for the pull request, and thank you very much @akinomyoga for handling everything!

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.

3 participants