zdharma-continuum / zinit

🌻 Flexible and fast ZSH plugin manager
MIT License
2.95k stars 126 forks source link

Add a few global aliases and vars like ZIBIN, ZIPLUGS and others. #378

Closed psprint closed 1 year ago

psprint commented 1 year ago

I've started a new PR for the global aliases because I haven't been using a separate branch in #371.

The user can use them in 3 ways:

$ cd ZIBIN $ cd $ZIBIN $ cd ~ZIBIN $ cd ZPFX

Description

Motivation and Context

Related Issue(s)

Usage examples

How Has This Been Tested?

Types of changes

Checklist:

jankatins commented 1 year ago

Would probably also be nice to add some hints that this is possible into the docs...?

Ryooooooga commented 1 year ago

I dislike global alias due to contamination of the global namespace and do not want to include them in my .zshrc.

Also, I don't feel that they need to be global alias, as they seem to be simply a short hand for environment variables. Therefore, I think it would be better to remove or opt-in to them.

alichtman commented 1 year ago

Yeah, that's a good point. They're also simple enough that you can add these aliases on your own. I'd be alright with reverting this diff and making a section in the wiki for this. Thoughts, @psprint?

pschmitt commented 1 year ago

Please revert this. This should be a separate plugin, not part of zinit.

psprint commented 1 year ago

The plugin would consist of only 2 lines and would limit discoverability of such an useful hints.. how many times one felt boring and repelled by tab completing ~/.local/share/zinit/... path? It doesn't feel right...

The global aliases are limited use because ZIDIR/plugins doesn't work. So I guess that they can be removed. However ~ZIDIR/plugins does work and is very useful so I would leave the ZI... global variables. What do you think?

Also, the named directories are not completed so a zstyle could be provided for it?

Update: when using hash -d instead of global vars the ~ZI<tab> does complete. I've submitted a PR #381 that removes global aliases and uses hash -d for named directories.

Ryooooooga commented 1 year ago

@psprint Are environment variables also really needed in the zinit core? Since zinit is designed to not consume environment variables, it seems to me that adding short hands just because they are convenient is against the design philosophy.

If it is really necessary, it should be an independent plugin.

psprint commented 1 year ago

@Ryooooooga: The variables have been removed in #381. It's quite an argument that Zinit has been designed to not use variables if possible. I've added them because it's really cumbersome to constantly banging tab in order to get to ~/.local/share/zinit/…. However, the proper way of handling this is the hash -d way, as in #381. No namespace has been polluted, except for the one that should be (→ named dirs namespace).

Ryooooooga commented 1 year ago

named directories too

psprint commented 1 year ago

@Ryooooooga Why? Isn't it a convenient way of adding shorthands, that isn't normally polluted? Doesn't it hurt every time one want's to get to a plugin dir like fzf, to bang on tab 6 times, traversing the path after each / in cd ~/.local/share/zinit/plugins/fzf/? A wonderful replacement to do this in 2 max and possibly also 0 times (when entering without completion for its shortness) in ~ZIPLUGS/fzf? I think that it's a cool feature.

What exactly is the objection, is it namespace pollution? If yes, then I think that it's isn't a problem, because:

so why not use it?

Ryooooooga commented 1 year ago

This should be a separate plugin, not part of zinit.

psprint commented 1 year ago

I disagree. It's the same as zi alias that's set up by zinit. The plugin would provide a single line of code, and limit discoverability of the feature meaning a much pain for the users having to keep banging tab. If alias is set, then so should be named dirs.

Ryooooooga commented 1 year ago

This should be a separate plugin, not part of zinit.

So why should it be included in zinit itself?

psprint commented 1 year ago

So why should it be included in zinit itself?

IMO, because:

I wonder what's the actual problem here? Because I think that much more users would be grateful for protecting against constant 6 tab hitting, than some personal issue with Zinit providing its own directories on its own…

Ryooooooga commented 1 year ago

If zinit is going to be oh-my-zsh with lots of "something useful" in it, there is no reason to use zinit.

psprint commented 1 year ago

This is a relief also for developers, not only to users. It's a single line of code, nothing bloated as oh-my-zsh. Where did I write "something useful"?

Gerrit-K commented 1 year ago

I think that much more users would be grateful for protecting against constant 6 tab hitting [...] This is a relief also for developers, not only to users.

Are you sure that it's not rather a relief for developers than for users? I've been using (and just using, not developing) zinit for over two years now and almost never feel the urge to cd into some internal zinit directory, not even the plugin directory. The only times I needed this was when zinit (or my zinit configuration) didn't work as expected, but once I fixed it, I never look back at it. I just want my zsh environment to work and not worry about it.

As a developer (of other tools), I can relate to @Ryooooooga: I consider polluting the global namespace bad practice and would like to challenge every reason to do that. Surely, there are sometimes good reasons, but I don't see one here. If you want happy users that adopt your tool quickly, it's best to focus on their needs, not yours.

psprint commented 1 year ago

Ok, I give up. If so much objection to adding 1 line of code to use unused namespace of named directories that protects against tab banging, then I have to bail out.

PS. Let me just try to leave you with this beautiful screenshot of the comfort that this 1 line of code gives:

2022-09-29-092742_1892x117_scrot

Gerrit-K commented 1 year ago

@psprint sudo rm -rf / is also just 1 line of code, that can solve a lot of issues ;) /s Of course that's a big exaggeration and I didn't want to come across too aggressive. I can understand that this single line would be very helpful for at least you and a couple of contributors, but I would argue that many other of the (perhaps >1.3k) users, it might just be another useless global variable (of which I already have a lot).

I'd suggest to not fully disregard this, but rather keep it as a suggestion and let the user base decide (via emoji votes) whether they want to have this too :) And in the meantime (as I've understood), you're free to set it for yourself in your own .zshrc, which sounds like a small price to pay for that apparently big advantage :)

psprint commented 1 year ago

it might just be another useless global variable (of which I already have a lot).

No global vars, see #381.

Gerrit-K commented 1 year ago

Oh, sorry, I guess I didn't get the whole picture then. I just curiously opened this thread when I noted the merged revert, wanting to know what's going on and if there was a serious incident (luckily there wasn't). When I skimmed over the comments I just thought that the perspective of an actual user might be helpful, but I think I've said more than enough, so I'll zone out again and let you and the devs carry on :)

github-actions[bot] commented 1 year ago

:tada: This PR is included in version 3.8.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: