[BUG] _describe -x adds same completion group header twice

[BUG] _describe -x adds same completion group header twice

[Marlon Richert]

When _describe is called with -x, it adds the same group header twice:

% autoload -Uz compinit; compinit
% zstyle '*' group-name ''
% zstyle '*' format '--%d--'
% compdef _tst tst
% _tst() { local -a arr=( foo bar ); _describe -x 'desc' arr }
% tst \t
--desc--
--desc--
bar  foo

Re: [BUG] _describe -x adds same completion group header twice

[Bart Schaefer]

On Thu, Sep 9, 2021 at 9:50 AM Marlon Richert <marlon.richert@gmail.com> wrote:
>
> When _describe is called with -x, it adds the same group header twice:

I think this is probably the same thing as in the thread starting
workers/48091 (Bug in compdescribe ...) that was partly addressed by
workers/49211.

Re: [BUG] _describe -x adds same completion group header twice

[Marlon Richert]

On Thu, Sep 9, 2021 at 10:04 PM Bart Schaefer <schaefer@brasslantern.com> wrote:
>
> On Thu, Sep 9, 2021 at 9:50 AM Marlon Richert <marlon.richert@gmail.com> wrote:
> >
> > When _describe is called with -x, it adds the same group header twice:
>
> I think this is probably the same thing as in the thread starting
> workers/48091 (Bug in compdescribe ...) that was partly addressed by
> workers/49211.

The problem still exists on master and it looks to me like it isn't in
compdescribe. Here's a diff on the trace output from _describe without
-x (a) and with -x (b):

% diff a b
8c8,14
< +_describe:36> shift 0
---
> +_describe:22> case x (o)
> +_describe:22> case x (O)
> +_describe:22> case x (t)
> +_describe:22> case x (1|2|J|V|x)
> +_describe:33> _jvx12+=( -x )
> +_describe:21> getopts oOt:12JVx _opt
> +_describe:36> shift 1
26c32
< +_describe:68> _next_label values _expl desc
---
> +_describe:68> _next_label -x values _expl desc
48c54
< +_describe:114> compadd -2 -o nosort -J values -X --desc-- -D _a_11 - foo bar
---
> +_describe:114> compadd -2 -o nosort -J values -x --desc-- -D _a_11 - foo bar
57c63
< +_describe:134> compadd -J values -X --desc-- -d _tmpd -a _tmpm
---
> +_describe:134> compadd -J values -x --desc-- -d _tmpd -a _tmpm
60c66
< +_describe:68> _next_label values _expl desc
---
> +_describe:68> _next_label -x values _expl desc
%

The problem here looks to me that compadd -x *always* adds a header,
even when used with -D, whereas compadd -X does *not* add a header
when used with -D (since the operation doesn't add any completions).
And because compadd treats -2 -o nosort -J values -x --desc-- as a
different namespace from -J values -x --desc--, the header ends up
being added twice.

Which of these two should we consider at fault: _describe or compadd?

It would be trivial to fix in _describe, but I feel like it would be
better to fix in compadd, so this problem cannot possibly occur again.
I find it hard to believe it is ever anyone's intention to add a
visible header when calling compadd -D (or -O or -A).

Re: [BUG] _describe -x adds same completion group header twice

[Oliver Kiddle]

On 10 Sep, Marlon Richert wrote:
>
> The problem here looks to me that compadd -x *always* adds a header,
> even when used with -D, whereas compadd -X does *not* add a header

> Which of these two should we consider at fault: _describe or compadd?
>
> It would be trivial to fix in _describe, but I feel like it would be
> better to fix in compadd, so this problem cannot possibly occur again.
> I find it hard to believe it is ever anyone's intention to add a
> visible header when calling compadd -D (or -O or -A).

Yes, I agree. The problem is in compadd. With -D, -O or -A, it shouldn't
do anything besides setting the arrays. 

The following is a minimal change that appears to do the job. It may be
possible to skip rather more of the code within compadd when any of
those options are set but that isn't trivial.

Oliver

diff --git a/Src/Zle/compcore.c b/Src/Zle/compcore.c
index 131e86825..63136854e 100644
--- a/Src/Zle/compcore.c
+++ b/Src/Zle/compcore.c
@@ -2120,7 +2120,7 @@ addmatches(Cadata dat, char **argv)
             curexpl->always = !!dat->mesg;
             curexpl->count = curexpl->fcount = 0;
             curexpl->str = dupstring(dat->mesg ? dat->mesg : dat->exp);
-            if (dat->mesg)
+            if (dat->mesg && !dat->dpar && !dat->opar && !dat->apar)
                 addexpl(1);
         } else
             curexpl = NULL;