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

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

Merged
merged 1 commit into from
Sep 28, 2022

Conversation

psprint
Copy link
Contributor

@psprint psprint commented Sep 28, 2022

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

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

The user can use them in 3 ways:

$ cd ZIBIN
$ cd $ZIBIN
$ cd ~ZIBIN
$ cd ZPFX
@alichtman alichtman merged commit 37cd36e into zdharma-continuum:main Sep 28, 2022
@jankatins
Copy link
Contributor

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

@Ryooooooga
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

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

@psprint
Copy link
Contributor Author

psprint commented Sep 29, 2022

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
Copy link
Contributor

@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
Copy link
Contributor Author

psprint commented Sep 29, 2022

@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
Copy link
Contributor

named directories too

@psprint
Copy link
Contributor Author

psprint commented Sep 29, 2022

@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:

  • named dir namespace isn't populated by more than (guessing) 3 entries,
  • it is designed for exactly such an use case – to help with often used and long paths,

so why not use it?

@Ryooooooga
Copy link
Contributor

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

@psprint
Copy link
Contributor Author

psprint commented Sep 29, 2022

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
Copy link
Contributor

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

So why should it be included in zinit itself?

@psprint
Copy link
Contributor Author

psprint commented Sep 29, 2022

So why should it be included in zinit itself?

IMO, because:

  • hitting tab 6 times to get to a plugin dir is at least cumbersome,
  • the named directories are rarely used so no pollution of it,
  • the dirs are designed just for that purpose – to quickly cd to them,
  • the separate plugin wouldn't be a serious plugin – it would have only one line of code (the hash -d … line),
  • it would in reality make the relieving functionality very rarely used, because of limited discoverability of such an one line plugin.

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
Copy link
Contributor

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

@psprint
Copy link
Contributor Author

psprint commented Sep 29, 2022

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
Copy link
Contributor

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
Copy link
Contributor Author

psprint commented Sep 30, 2022

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
Copy link
Contributor

@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
Copy link
Contributor Author

psprint commented Sep 30, 2022

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

No global vars, see #381.

@Gerrit-K
Copy link
Contributor

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
Copy link

github-actions bot commented Nov 7, 2022

🎉 This PR is included in version 3.8.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants