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: #2094 #3465

Merged
merged 1 commit into from
Oct 14, 2023
Merged

fix: #2094 #3465

merged 1 commit into from
Oct 14, 2023

Conversation

LangLangBart
Copy link
Contributor

@LangLangBart LangLangBart commented Oct 8, 2023

Description

@romkatv
Copy link
Contributor

romkatv commented Oct 8, 2023

  • Despite the original reporter claiming to have a more recent version of zsh, it's recommended they
    verify their version again. @fluency03

The bulletproof way to verify this would be to add typeset -p ZSH_VERSION directly to /home/{my-user-name}/.fzf/shell/key-bindings.zsh, preferably just before the emulate line.

@junegunn
Copy link
Owner

Thanks. Are we trying to support zsh versions older than 5.0.2? zsh 5.0.2 predates fzf so I'm not sure if this is the right goal.

@romkatv
Copy link
Contributor

romkatv commented Oct 12, 2023

If you want to enforce zsh version >= 5, you can do it like this:

if [[ $ZSH_VERSION != <5->.* ]]; then
  print -ru2 -- "Your version of zsh is too old: $ZSH_VERSION < 5.0"
  return 1
fi

I'm not saying you should enforce zsh version >= 5 although in my open source zsh code I do that. The number of users on zsh < 5 is exceedingly tiny.

@LangLangBart
Copy link
Contributor Author

LangLangBart commented Oct 12, 2023

Thanks. Are we trying to support zsh versions older than 5.0.2? zsh 5.0.2 predates fzf so I'm not sure if this is the right goal.

It's your call to decide, as everything else appears to function properly.


I've never encountered this zsh syntax, but it works:

<5->.*

This syntax seems more common:

if (( ${ZSH_VERSION%%.*} < 5 )); then
  echo "Your zsh version is too old: $ZSH_VERSION < 5.0"
  return 1
fi

@romkatv
Copy link
Contributor

romkatv commented Oct 12, 2023

Your check is also fine but the echo statement is worse with its couple of tiny bugs. Not a big deal but you might just as well use the correct code if you decide to reject old zsh.

@LangLangBart
Copy link
Contributor Author

Your check is also fine but the echo statement is worse with its couple of tiny bugs. Not a big deal but you might just as well use the correct code if you decide to reject old zsh.

I will certainly take your advice and use the correct code. Thank you again for your help! I will proceed once the maintainer has made a decision on how to handle the issue.

@junegunn
Copy link
Owner

I don't think we should put in extra effort to support zsh 4, but this patch is simple and short enough, so I'll merge it. Thanks.

@junegunn junegunn merged commit 3e1735b into junegunn:master Oct 14, 2023
5 checks passed
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.

/home/{user}/.fzf/shell/key-bindings.zsh:emulate:35: unknown argument -o
3 participants