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

Remove requirement for $GIT_SUBREPO_ROOT from _git-subrepo zsh completions #183

Open
rbellamy opened this issue May 2, 2016 · 5 comments
Labels

Comments

@rbellamy
Copy link

rbellamy commented May 2, 2016

When the _git-subrepo completion script is dropped directly into /usr/share/zsh/site-functions the completions JustDon'tWork(tm).

Furthermore, the check doesn't actually DO anything except bark and barf if the variable hasn't been set - the completions work just fine without it.

@perlpunk
Copy link
Contributor

perlpunk commented May 2, 2016

@rbellamy regarding your first point:
it works for me.
What I did:

  • comment out the line with enable-completion.sh in the .rc file to make sure the completion isn't loaded from this place
  • sudo cp ~/.../git-subrepo/share/zsh-completion/_git-subrepo /usr/share/zsh/vendor-completions/
  • open a completely new shell

and completion worked. If I leave out the copy, it doesn't work.

Maybe you should check if the path for the completion is correct on your side, and make sure you open a new shell after adding the script.

About the issue topic:
It's true that the completion script works without that variable. I think this is just there to make sure that you sourced the .rc script and have a running git-subrepo command.

Why would you want to remove it?

@rbellamy
Copy link
Author

rbellamy commented May 2, 2016

@perlpunk the assumption made by the completion script is that it has been installed via a git clone, and thus it's important to know where the $GIT_SUBREPO_ROOT is located.

If you copy the completion script, as-is, directly into /usr/zsh/site-functions then the completions will fail, because that variable will not have been set. The documentation says:

In the Z shell (zsh), you can manually enable git-subrepo completion by adding the following line to your ~/.zshrc, before the compinit function is called:

    fpath=('/path/to/git-subrepo/share/zsh-completion' $fpath)

My $fpath is set correctly to load functions in the following order:

  1. ~/.config/zsh/functions
  2. /usr/local/share/zsh/site-functions
  3. /usr/share/zsh/site-functions

Every other completion script in my system, including git, git-flow, curl, journalctl, systemctl, etc, have no requirement to know where their parent executable is installed - why should git-subrepo?

I recognize that the .gitsubrepo file expects to be able to include the git-subrepo commit hash - but again, what if the command has been installed via APT, YUM, or an Arch PKGBUILD? In those cases, there is no git-subrepo clone located on the installed system.

Or to make my point from another angle, to my mind testing whether a command has been installed correctly from within a completion script is asking it to do more than it should - the presumption is that the command is available on the users path, or completion isn't necessary in the first place.

Which brings me to my final argument - why add yet another path variable to the user's environment? $PATH already does this job - if it's really that important to identify whether the command has been installed "properly", then inspect the users path to see if you can find the command.

@perlpunk
Copy link
Contributor

perlpunk commented May 2, 2016

@rbellamy ok, then that's more a general question about $GIT_SUBREPO_ROOT - currently the docs say it's required to source the .rc file.
If that is actually not required to run it, then yeah, it should be removed

@grimmySwe grimmySwe added the Bug label May 4, 2016
@cdlm
Copy link

cdlm commented Mar 15, 2017

👍

ryantrinkle pushed a commit to obsidiansystems/git-subrepo that referenced this issue Aug 21, 2019
…on handler

Fixes ingydotnet#183

I don't know how to comprehensively test this, but I have verified that some basic completions work correctly
@perlpunk
Copy link
Contributor

this was fixed in #476

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