r-darwish / topgrade

Upgrade everything
GNU General Public License v3.0
3.37k stars 162 forks source link

ZSH zi step runs when zoxide is installed due to alias name collision #935

Closed trmckay closed 2 years ago

trmckay commented 2 years ago

What did you expect to happen?

ZSH "zi" step should only run if zi is installed, not if the alias zi from zoxide is available.

What actually happened?

zi is run, but it fails because it is not the command that the step expects.

Additional details

The error occurs here since it is executing the command after sourcing the user's zshrc.

This is tough because for many of the steps, the zshrc needs to be sourced to pick up plugins, but this increases the likelihood of name collisions like this.

MCOfficer commented 2 years ago

Can we just check for the existance of ~/.zi instead?

trmckay commented 2 years ago

That could work. I'd be happy to draft a PR if this is wanted.

r-darwish commented 2 years ago

@trmckay please go ahead

r-darwish commented 2 years ago

https://github.com/r-darwish/topgrade/blob/08e8b56b75fd273b0bad17dcec5bb7f1a9b289f5/src/steps/zsh.rs#L101-L105

We don't check the existance of zi, but ZPFX instead. @t4i5uKE can you explain the role of this environment variable?

trmckay commented 2 years ago

After looking at zinit, it looks like the ZPFX variable is the prefix for the installation directory. Also, it appears zi also uses this variable for a similar purpose.

trmckay commented 2 years ago

Maybe my usage of zinit over zi is the real issue here. I'm not sure if I completely follow the whole story, but it appears zinit/zi have deleted and revived a few times.

r-darwish commented 2 years ago

So, if ZPFX is set, there supposed to be a zi or .zi directory inside the ZPFX directory?

ss-o commented 2 years ago

Hi :wave:

I just noticed this issue. Due to similar issues, I wrapped zi in a function:

❮▼❯() { zi "$@"; }

which makes zi available as ❮▼❯.


Also noticed some confusion, feel free, and also I would appreciate it if mention me to help with zi / zsh related issues in the future as I am the maintainer of zi and a user of topgrade. :octocat:

Thanks,