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

The zsh completion has a brain split #270

Closed
danielshahaf opened this issue Jan 3, 2020 · 8 comments
Closed

The zsh completion has a brain split #270

danielshahaf opened this issue Jan 3, 2020 · 8 comments

Comments

@danielshahaf
Copy link
Contributor

Compare:

https://github.com/RichiH/vcsh/commits/master/_vcsh
https://github.com/zsh-users/zsh/commits/master/Completion/Unix/Command/_vcsh

First of all, they are functionally different. At least vcsh foo add <TAB> works in the former but not in the latter.

The latter has only one commit that isn't in the former: zsh-users/zsh@13fc579#diff-9394f5dc325d8a13da62fcaf4172a892 That commit unfortunately mixes functional and non-functional changes, but I think the important parts of it are two: adding _call_program and setting ret. So I suggest to cherry-pick those two changes and then to remove _vcsh completion from zsh upstream.

@danielshahaf
Copy link
Contributor Author

danielshahaf commented Jan 3, 2020

Specifically, zsh commit zsh-users/zsh@f9851d8 has imported _vcsh as of commit 1d8970f in this repository, verbatim:

% diff -s =(cd vcsh && git cat-file blob 1d8970fdb8d4adbfd5cdecc5d15da9db61efb39d:_vcsh) \
          =(cd zsh && git cat-file blob f9851d817fd6b45e00c35d71f9047f95eca06f42:Completion/Unix/Command/_vcsh) 
Files /tmp/zshwsSvMz and /tmp/zsh1po76w are identical

danielshahaf pushed a commit to danielshahaf/vcsh that referenced this issue Jan 3, 2020
The change to __vcsh_repositories will require users to update zstyle
settings, if anyone has created such zstyle settings.

Ported (by Daniel Shahaf) from the following zsh upstream commit:

    commit 13fc579343b24d298fb8905933b6000d7fcda114
    Author: Oliver Kiddle <opk@zsh.org>
    Date:   Tue Oct 14 23:03:40 2014 +0200

        33467: correct return status on functions and numerous other minor fixes

With this change, our completion is a superset of zsh's _vcsh, so the
latter may be removed.  See issue RichiH#270.
@danielshahaf
Copy link
Contributor Author

Created PR #271. Once it's fixed and _vcsh is removed upstream, this issue may be closed.

alerque pushed a commit that referenced this issue Mar 30, 2021
The change to __vcsh_repositories will require users to update zstyle
settings, if anyone has created such zstyle settings.

Ported (by Daniel Shahaf) from the following zsh upstream commit:

    commit 13fc579343b24d298fb8905933b6000d7fcda114
    Author: Oliver Kiddle <opk@zsh.org>
    Date:   Tue Oct 14 23:03:40 2014 +0200

        33467: correct return status on functions and numerous other minor fixes

With this change, our completion is a superset of zsh's _vcsh, so the
latter may be removed.  See issue #270.

Co-authored-by: Oliver Kiddle <opk@zsh.org>
@alerque
Copy link
Collaborator

alerque commented Mar 30, 2021

I've merged the related PR for this, but I'm not sure what to do about ZSH. Should we submit a PR to nuke the completions from their repository? Or on that brings it to parity with ours? It makes more sense to me as both a developer here and a distro packager to include it in our sources, but if upstream ZSH prefers to keep a copy themselves I guess the least we could do is update it. Is there a protocol they follow for this?

@RichiH
Copy link
Owner

RichiH commented Mar 30, 2021

Historically, doing both was preferred, which is why I did it that way. Unclear if that's still true today, I stopped being active on the Zsh lists a decade ago.

@okapia, what's your opinion?

@okapia
Copy link

okapia commented Mar 30, 2021

Having it in both tends to just result in them diverging and it may not be ideal where projects have different licences. There have been a couple of other cases where we ended up removing the one in zsh.

I tend to think that it is best if upstream projects carry completions. The upstream projects at least know their software best. It allows completions to match the installed version of the software - at least assuming they get updated in tandem with the option parsing code and documentation.

This doesn't seem to be a problem for vcsh but it's also important that packagers install the completions correctly: the worst case is that users need to go digging around to piece together their own completion collection from plugins and frameworks in a few dozen external github repos.

alerque added a commit to alerque/zsh that referenced this issue Mar 30, 2021
See discussion on upstream project here:

RichiH/vcsh#270
zmwangx pushed a commit to bettermirror/zsh that referenced this issue Mar 31, 2021
@danielshahaf
Copy link
Contributor Author

Speaking of upstream projects installing completions, where in zsh's documentation would y'all expect the details of that (e.g., the path to install to) to be documented? I'd like to make sure the information is discoverable.

@alerque
Copy link
Collaborator

alerque commented Mar 31, 2021

Honestly I don't know. I'd expect it to be in a man page somewhere, maybe ZSHCOMPSYS(1), but the ZSH documentation is already a bewildering maze. I'd search the man page for "site-functions", but the only reason I'd do that is I already know the answer and am trying to find the question. I don't know what I would do if I didn't know already.

@alerque alerque closed this as completed Apr 1, 2021
@danielshahaf
Copy link
Contributor Author

@alerque Thanks for the answer (and sorry for the delay; I was waiting to see if anyone else would chime in). I did receive two other answers in private mail, too. I'll take this to the upstream lists. Cheers!

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

No branches or pull requests

4 participants