larkery / zsh-histdb

A slightly better history for zsh
MIT License
1.25k stars 74 forks source link

"which sysopen" doesn't work on zsh 5.7.1 and now zsh-histdb won't load #109

Closed ctranstrum closed 2 years ago

ctranstrum commented 3 years ago

I updated this plugin yesterday after not touching it for over a year, and it looks like there is a breaking change introduced in https://github.com/larkery/zsh-histdb/commit/f7952ad355844e5430d934349e708972b98791cb that seems well-intentioned but doesn't work.

Running on my EC2 instance:

⭑ zsh --version
zsh 5.7.1 (x86_64-koji-linux-gnu)

⭑ which sysopen
/usr/bin/which: no sysopen in ($PATH)

I updated on my Mac as well, and it works as intended:

⭑ zsh --version
zsh 5.8 (x86_64-apple-darwin20.0)

⭑ which sysopen
sysopen: shell built-in command

Any chance we can get this commit reverted or updated so that it does a check that actually works?

ctranstrum commented 3 years ago

Well, this is silly. I just tried it from my Raspberry Pi and it works, even though it's using the same version of zsh as on EC2:

⭑ zsh --version
zsh 5.7.1 (arm-unknown-linux-gnueabihf)

⭑ which sysopen
sysopen: shell built-in command

The difference seems to be how which is working.

The systems it works correctly on:

⭑ which which
which: shell built-in command

And the system where it doesn't:

⭑ which which
which='alias | /usr/bin/which --tty-only --read-alias --show-dot --show-tilde'
        /usr/bin/alias
        /usr/bin/which

Further research determined that the alias is being defined in the file /etc/profile.d/which2.sh, which apparently came as a default on the EC2 instance.

So ... the solution would be to update the line in sqlite-history.zsh from

which sysopen &>/dev/null || return; # guard against zsh older than 5.0.8.

to

builtin which sysopen &>/dev/null || return; # guard against zsh older than 5.0.8.
Hobadee commented 2 years ago

I have had this exact same issue on several fresh Cent/RedHat installs. I just traced it to that same line. I can confirm that changing which to builtin which works for me.

larkery commented 2 years ago

I will merge this request. Happy days.