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

zsh completions broken in yadm 3.0.0 #292

Closed
sidmadala opened this issue Jan 4, 2021 · 24 comments
Closed

zsh completions broken in yadm 3.0.0 #292

sidmadala opened this issue Jan 4, 2021 · 24 comments
Labels

Comments

@sidmadala
Copy link

Describe the bug

Yadm 2 completions worked very well, but when upgrading to version 3.0.0 through Homebrew, pressing tab yields the following error on the first try followed by silence on any other attempts in the shell. The completions file _yadm still exists in the proper /usr/local/share/zsh/site-functions directory.

Screen Shot 2021-01-03 at 9 19 31 AM

To reproduce

Can this be reproduced with the yadm/testbed docker image: Most likely no since I use MacOS and Homebrew

Steps to reproduce the behavior:

  1. Type command yadm
  2. Press TAB
  3. See error

Expected behavior

I'd expect the completions to show up with a description of each command.

Environment

  • Operating system: [Ubuntu 18.04, yadm/testbed, etc.]
  • Version yadm: yadm 3.0.0
  • Version Git: git version 2.30.0
@sidmadala sidmadala added the bug label Jan 4, 2021
@erijo
Copy link
Collaborator

erijo commented Jan 4, 2021

What version of zsh are you using?

@sidmadala
Copy link
Author

zsh 5.7.1 (x86_64-apple-darwin19.0)

@erijo
Copy link
Collaborator

erijo commented Jan 4, 2021

Can you try this to verify that it isn't something in your setup:

zsh -f
fpath=(/usr/local/share/zsh/site-functions $fpath)
autoload compinit
compinit

and then try to complete yadm commands again?

@sidmadala
Copy link
Author

I do not get the error, but I also do not get any suggestions either. Only folder names appear for autocompletion.

@erijo
Copy link
Collaborator

erijo commented Jan 4, 2021

Then the fpath is probably wrong. Does the path added contain the file _yadm?

@sidmadala
Copy link
Author

sidmadala commented Jan 4, 2021

The first item in my fpath is /usr/local/share/zsh/site-functions which contains the _yadm file.

Screen Shot 2021-01-04 at 2 53 01 PM

And a slightly newer error I'm getting.

Screen Shot 2021-01-04 at 2 54 01 PM

@TheLocehiliosan
Copy link
Member

I'm not a zsh user, and I just attempted to use the completions installed via HomeBrew. I put this in my .zshrc as suggested by HomeBrew:

if type brew &>/dev/null; then
  FPATH=$(brew --prefix)/share/zsh-completions:$FPATH
  autoload -Uz compinit
  compinit -i
fi

And then started a zsh, and typed "yadm ", and I'm met with this error:

yadm _git:12: command not found: __yadm_main

and then afterwards, only get directory completions...

I'm not sure where that comes from yet, but I would guess it comes from the inherited git completions. I see this in /usr/local/share/zsh/site-functions/_git:

_git ()
{
	local _ret=1
	local cur cword prev

	cur=${words[CURRENT]}
	prev=${words[CURRENT-1]}
	let cword=CURRENT-1

	if (( $+functions[__${service}_zsh_main] )); then
		__${service}_zsh_main
	else
		emulate ksh -c __${service}_main
	fi

	let _ret && _default && _ret=0
	return _ret
}

But I'm not sure where ${service} gets defined.

@erijo
Copy link
Collaborator

erijo commented Jan 4, 2021

It seems like the git completion shipped with git differs from the git completion shipped with zsh. The new yadm completion requires the latter.

If you try removing the _git symlink from the /usr/local/share/zsh/site-functions dir, does it work then (after starting a new shell)?

@sidmadala
Copy link
Author

It works if i rename/remove the _git symlink.

@TheLocehiliosan
Copy link
Member

I'm away at the moment, but later I'll dig into where the _git completions in home brew are coming from. I think there may still need to be some mitigation for this situation.

@erijo
Copy link
Collaborator

erijo commented Jan 4, 2021

Yes, I think we should keep this issue open until a proper fix is in place.

The git completions comes from the git package according to the image above (seen in symlink).

@sidmadala sidmadala reopened this Jan 4, 2021
@TheLocehiliosan
Copy link
Member

Again, I'm away, but in my setup, there was also a second git completion. I'll look into it later for sure.

@sidmadala
Copy link
Author

My other git completions come from a zsh plugin called zsh-useres/zsh-completions which packages most of the default tools' completions into one folder. Yadm works fine when this other completion source is the one being used instead of the default _git.

@erijo
Copy link
Collaborator

erijo commented Jan 4, 2021

I've been able to reproduce this issue. I'll have a look and see if I can come up with a solution.

@erijo
Copy link
Collaborator

erijo commented Jan 4, 2021

I've just posted a PR that works for me in my testing. But more testing is of course welcome.

@TheLocehiliosan
Copy link
Member

@sidmadala I have tested @erijo's changes, and they work for me. It would be great to get further confirmation. Is it possible to copy the completions/zsh/_yadm file from the develop branch into /usr/local/share/zsh/site-functions, and then run brew reinstall git, and see if completions are working?

The brew reinstall should put the Git completions back into place if they were removed.

@sidmadala
Copy link
Author

Works for me. Don't know why the spacing is uneven though.

Git:
Screen Shot 2021-01-04 at 9 14 47 PM

Yadm:
Screen Shot 2021-01-04 at 9 13 50 PM

@xenoterracide
Copy link

xenoterracide commented Jan 5, 2021

I didn't get an error, but completions were stalling out zsh so bad I had to kill them so I unloaded them.

1 ❯ zsh --version                                                                                       # ~
zsh 5.8 (x86_64-pc-linux-gnu)
❯ uname -a                                                                                              # ~
Linux freyja 5.10.2-2-MANJARO #1 SMP PREEMPT Tue Dec 22 08:14:42 UTC 2020 x86_64 GNU/Linux

@erijo
Copy link
Collaborator

erijo commented Jan 5, 2021

@xenoterracide: is this also with the fixed completions on the develop branch?

erijo added a commit to erijo/yadm that referenced this issue Jan 5, 2021
Don't rely on internals from the git completion, instead set up the
environment and then simply call _git and let it do it's completion as
it see fit.

See yadm-dev#292.
@erijo
Copy link
Collaborator

erijo commented Jan 5, 2021

@sidmadala: the mixed spacing was a consequent of how the completion was done.

All: I've now done yet another retake on the completion (see my zsh-completion branch) where I've simplified the completion and stopped using internal functions from the git completion. I've only done some simple testing so far, but it seems to work well.

Please try it and see if it works for you as well.

@sidmadala
Copy link
Author

@erijo, looks good to me. Separating the yadm specific commands is a nice touch.

Screen Shot 2021-01-05 at 5 30 47 PM

erijo added a commit to erijo/yadm that referenced this issue Jan 6, 2021
Don't rely on internals from the git completion. Instead set up the
environment and then simply call _git and let it do the completion as
it see fit.

See yadm-dev#292.
@TheLocehiliosan
Copy link
Member

@erijo - Can you create a new PR for the updated zsh completions? (assuming the are working better)

@erijo
Copy link
Collaborator

erijo commented Jan 6, 2021

@TheLocehiliosan: yes, will do. I have a few minor things to fix first.

erijo added a commit to erijo/yadm that referenced this issue Jan 6, 2021
Don't rely on internals from the git completion. Instead set up the
environment and then simply call _git and let it do the completion as
it see fit.

See yadm-dev#292.
erijo added a commit to erijo/yadm that referenced this issue Jan 6, 2021
Don't rely on internals from the git completion. Instead set up the
environment and then simply call _git and let it do the completion as
it see fit.

See yadm-dev#292.
TheLocehiliosan added a commit that referenced this issue Jan 7, 2021
* Improve handling of submodules at upgrade (#284, #285, #293)
* Improve Zsh completions (#292, #298)
* Use stderr for error messages (#297)
@xenoterracide
Copy link

just an FYI I plan on testing this, just haven't gotten around to it yet. no later than this weekend though, maybe tonight. I should probably update the aur PKGBUILD to do it.

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

No branches or pull requests

4 participants