git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3] contrib/completion: fix zsh completion regression from 59d85a2a05

From
David Aguilar <davvid@gmail.com>
Date
Jun 1, 2021, 20:59 UTC
Message-ID
<CAJDDKr630n7=8FqsSRXpN7fNy9ZB8FOzO=LxvfHFVrMR1=L3_A@mail.gmail.com>
In-Reply-To
<60b6873624c6f_1a702085e@natae.notmuch>

On Tue, Jun 1, 2021 at 12:15 PM Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 51 quoted lines
>
> David Aguilar wrote:
> > A recent change to make git-completion.bash use $__git_cmd_idx
> > Add "git" to the "words" array in _git_zsh_main to guarantee
> > that "git" is at least always in the completion list.
>
> Hm, no. The current code already guarantees "git" is always at the start
> of the completion list. In [1] I suggested to add git *if* $words is
> used instead of $orig_words.
>
> If you add "git" to $orig_words you end up with something like
> "git git mv", so the __git_cmd_idx is definitely not 1.
>
> You should probably try to test yourself:
>
>   words=( git ${words[@]} )
>   echo "$words" >> /tmp/words-log.txt
>
> The problem is that zsh's _arguments eats all the words it finds, so for
> example if you type:
>
>   git mv --force <tab>
>
> $words will be 'mv --force'.
>
> It's better to use $words because in case there's arguments beforehand,
> like:
>
>   git --git-dir=/tmp/test/.git mv --force
>
> $words will be 'mv --force', so we can get the proper index by just
> adding 'git' beforehand.
>
> But it doesn't work for arguments not in _arguments, like:
>
>   git --foo mv --force
>
> Which returns:
>
>   --foo mv --force
>
> And unfortunately upstream's version of the wrapper doesn't understand
> many arguments, like -c, or -C. git-completion does have all of them
>
> It's better to just leave the code as it is and just fix the regression
> by adding __git_cmd_idx=1.
>
> > Helped-by: Felipe Contreras <felipe.contreras@gmail.com>
>
> I mean I kind of wrote 2 of the 3 lines you sent, can I get a
> Suggested-by?
The v4 I just sent is now basically the same as v2 modulo the
Suggested-by: trailer update.
Show 27 quoted lines
>
> > --- a/contrib/completion/git-completion.zsh
> > +++ b/contrib/completion/git-completion.zsh
> > @@ -251,7 +251,7 @@ __git_zsh_main ()
> >               done
> >               ;;
> >       (arg)
> > -             local command="${words[1]}" __git_dir
> > +             local command="${words[1]}" __git_dir __git_cmd_idx=1
>
> This is needed.
>
> >
> >               if (( $+opt_args[--bare] )); then
> >                       __git_dir='.'
> > @@ -261,7 +261,7 @@ __git_zsh_main ()
> >
> >               (( $+opt_args[--help] )) && command='help'
> >
> > -             words=( ${orig_words[@]} )
> > +             words=( git ${orig_words[@]} )
>
> This is wrong. The current code is fine.
>
> Cheers.
>
> [1] https://lore.kernel.org/git/60b3c2d7557bd_be762089a@natae.notmuch/
Thanks for the detailed explanation.
Just so I'm understanding this correctly.. if this was instead..
    words=( git ${words[@]} )

(instead of orig_words like I mistakenly included in v3) would that be an improvement, no-op or would it be worse? It sounds like additional changes are needed to make it properly support options between "git" and the sub-command name, hence the patch is fine as-is in v4, correct?

Hopefully in the future it can be extended to cover eg. "git -c foo.bar -C some-dir <sub-command>" as well. Thanks for your patience.

cheers,
-- 
David
Previous: Felipe ContrerasNext: Felipe Contreras
Message 3 of 4 in “contrib/completion: fix zsh completion regression from 59d85a2a05”
  1. contrib/completion: fix zsh completion regression from 59d85a2a05David Aguilar, Jun 1, 2021
  2. Felipe ContrerasJun 1, 2021
  3. David AguilarJun 1, 2021
  4. Felipe ContrerasJun 1, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.