Re: [PATCH] _gpg: Use explicit UIDs for state = public keys.

[Doron Behar <doron.behar@xxxxxxxxx>]

On Sat, Jun 09, 2018 at 06:21:55PM +0000, Daniel Shahaf wrote:
> doron.behar@xxxxxxxxx wrote on Thu, Jun 07, 2018 at 17:48:57 +0300:
> > Use the `--with-colons` option in conjunction with `(f)` and `(@s.:.)`
> > to parse the output.
> > Quote the variables used in `_call_program`.
> > ---
> >  Completion/Unix/Command/_gpg | 71 ++++++++++++++++++++++++++++++++----
> >  1 file changed, 64 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Completion/Unix/Command/_gpg b/Completion/Unix/Command/_gpg
> > index 48a36eff2..a09ba3f9e 100644
> > --- a/Completion/Unix/Command/_gpg
> > +++ b/Completion/Unix/Command/_gpg
> > @@ -206,20 +206,77 @@ fi
> >  
> >  case "$state" in
> >    public-keys)
> > -    _wanted public-keys expl 'public key' \
> > -	compadd ${${(Mo)$(_call_program public-keys $words[1] $needed --list-public-keys --list-options no-show-photos):%<*>}//(<|>)/} && return
> > +    local public_keys=(${(@s.:.)${(f)"$(_call_program public-keys ${(q)words[1]} ${(q)needed} --list-public-keys --list-options no-show-photos --with-colons)"}})
> > +    local -a uids_and_emails
> > +    local i
> > +    for i in {1..${#public_keys[@]}}; do
> > +      if [[ ${public_keys[$i]} == "fpr" ]]; then
> 
> This looks for the string "fpr" in any field, not just in the first column,
> doesn't it?  (Already pointed out earlier)

Correct, when the search for "fpr" is done in this for loop, I can't
tell what element in the array was at the start of a line and which
wasn't.

> 
> > +        i=$((i + 1))
> > +        local j=$i
> > +        while [[ ${public_keys[$j]} != "fpr" ]] && [ $j -lt ${#public_keys[@]} ]; do
> > +          if [[ ${public_keys[$j]} =~ "@" ]]; then
> > +            local email="${public_keys[$j]}"
> > +            local uid="${public_keys[$i]}"
> > +            uids_and_emails+=("${uid}":"${email}")
> 
> Here, colons and backslashes in $uid should be escaped for _describe.  It may be
> easier to use the _describe syntax that takes two array names rather than one.
> 

Doing so actually brings a whole lot better way of handling the
completion (IMO) and I really like it. Try this yourself:

    local public_keys=(${(@s.:.)${(f)"$(_call_program public-keys ${(q)words[1]} ${(q)needed} --list-public-keys --list-options no-show-photos --with-colons)"}})
    local -a uids emails
    local i
    for i in {1..${#public_keys[@]}}; do
      if [[ ${public_keys[$i]} == "fpr" ]]; then
        i=$((i + 1))
        local j=$i
        while [[ ${public_keys[$j]} != "fpr" ]] && [ $j -lt ${#public_keys[@]} ]; do
          if [[ ${public_keys[$j]} =~ "@" ]]; then
            emails+="${public_keys[$j]}"
            uids+="${public_keys[$i]}"
            i=$j
            break
          fi
          j=$((j + 1))
        done
        i=$j
      fi
    done
    _describe -t public-keys 'public key' emails uids

Is that what you meant by using two arrays?

> > +            i=$j
> > +            break
> > +          fi
> > +          j=$((j + 1))
> 
> This assignment to $j seems to be a no-op, isn't it?  The written value
> wouldn't be used by anything.
> 

You are right, I removed `i=$j`.