Re: [PATCH] compdescribe fix for unsorted groups (workers/34814) (was: Re: completion: git: --fixup: problem with _describe -2V and duplicate commit subjects)

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

Daniel Shahaf wrote on Tue, May 19, 2015 at 20:44:46 +0000:
> Daniel Shahaf wrote on Thu, May 14, 2015 at 14:36:27 +0000:
> > The first attached patch seems to address this.  However,
> > I encountered an odd problem with it, which I have not been able to
> > narrow down.  [... 'git checkout 1<TAB>' ...]  I'm not sure whether
> > this indicates a bug in the first patch or an independent problem.
> 
> The 'git checkout <TAB>' issue is an independent problem.
> 
> Daniel Hahler has determined that the garbled output is due to _git's
> calling __git_commit_objects twice (cf 35216).  The attached minimal
> example demonstrates the problem.
> 
> So, in summary:
> 
> 1. The first patch in 35127 fixes the problem introduced by 34671,
> reported in 34768, and analysed in 34814.  The minimal example at the
> top of 35127 produces garbled output without 35127 and correct output
> with 35127.
> 
> 2. The second patch in 35127 triggered garbled output in 'git checkout
> <TAB>' because of a preexisting problem in _git, which 35216 fixes.
> An example of the garbled output is in 35169, surrounded by triple
> square brackets.
> 

With both 35127 and 35169 applied, I get the :descriptions message
twice:

    > recent commit object name
    > recent commit object name
    cf83173  HEAD     - debug (12 minutes ago)
    ...

The duplicate goes away if I revert
ed3e5f521d1243981831d7e084225b03e205ae38 (35209), i.e., add back the -2.

I verified that __git_recent_commits is called only once, so 35169 is
not at fault here.  I'm not sure what the actual problem is: whether
35209 should be reverted (the -2 added back), or 35127#1 should be
smarter about how compdescribe calls compadd (cf empty matches discussed
in 34814).  I'll investigate later.

Cheers,

Daniel

> 3. I propose the following documentation patch:
> 
> diff --git a/Doc/Zsh/compsys.yo b/Doc/Zsh/compsys.yo
> index a081ea3..5043e7b 100644
> --- a/Doc/Zsh/compsys.yo
> +++ b/Doc/Zsh/compsys.yo
> @@ -4197,6 +4197,9 @@ description will appear together in the list.
>  
>  tt(_describe) uses the tt(_all_labels) function to generate the matches, so
>  it does not need to appear inside a loop over tag labels.
> +
> +tt(_describe) should not be called more than once in a single call to
> +a completion function.
>  )
>  findex(_description)
>  item(tt(_description) [ tt(-x) ] [ tt(-12VJ) ] var(tag) var(name) var(descr) [ var(spec) ... ])(
> 
> Unless objections, I'll commit 35127#1 (the first of the two patches
> attached to that email).
> 
> Daniel

> a=(dalton:bar charlie:bar bob:foo alice:foo)
> _ftwice() { _describe -2 -V -x person a; _describe -2 -V -x person a }; compdef _ftwice ftwice
> % ftwice <TAB>
> > person
> charlie
> alice
> dalton
> bob
> -- bar
> -- foo
> charlie
> alice
> dalton
> bob
> -- bar
> -- foo