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

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

[doron.behar]

From: Doron Behar <doron.behar@gmail.com>

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
+        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}")
+            i=$j
+            break
+          fi
+          j=$((j + 1))
+        done
+        i=$j
+      fi
+    done
+    _describe -t public-keys 'public key' uids_and_emails
   ;;
   secret-keys)
-    _wanted secret-keys expl 'secret key' compadd \
-	${${(Mo)$(_call_program secret-keys $words[1] $needed --list-secret-keys --list-options no-show-photos):%<*>}//(<|>)/} && return
+    local secret_keys=(${(@s.:.)${(f)"$(_call_program secret-keys ${(q)words[1]} ${(q)needed} --list-secret-keys --list-options no-show-photos --with-colons)"}})
+    local -a uids_and_emails
+    local i
+    for i in {1..${#secret_keys[@]}}; do
+      if [[ ${secret_keys[$i]} == "fpr" ]]; then
+        i=$((i + 1))
+        local j=$i
+        while [[ ${secret_keys[$j]} != "fpr" ]] && [ $j -lt ${#secret_keys[@]} ]; do
+          if [[ ${secret_keys[$j]} =~ "@" ]]; then
+            local email="${secret_keys[$j]}"
+            local uid="${secret_keys[$i]}"
+            uids_and_emails+=("${uid}":"${email}")
+            i=$j
+            break
+          fi
+          j=$((j + 1))
+        done
+        i=$j
+      fi
+    done
+    _describe -t secret-keys 'secret key' uids_and_emails
   ;;
   ciphers)
     _wanted ciphers expl cipher compadd \
-        ${${(s.,.)${(M)${(f)${"$(_call_program ciphers $words[1] $needed --version)"}//,$'\n' #/, }:#Cipher*}#*:}# } && return
+        ${${(s.,.)${(M)${(f)${"$(_call_program ciphers ${(q)words[1]} ${(q)needed} --version)"}//,$'\n' #/, }:#Cipher*}#*:}# } && return
   ;;
   (public-keyids)
-    _wanted public-keys expl 'public keyid' \
-      compadd ${(M)${${(f)"$(_call_program public-keyids $words[1] $needed --list-public-keys --list-options no-show-photos)"}## #}:#[0-9A-F](#c40)} && 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
+        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}")
+            i=$j
+            break
+          fi
+          j=$((j + 1))
+        done
+        i=$j
+      fi
+    done
+    _describe -t public-keyids 'public keyids' uids_and_emails
   ;;
   (option-list)
     _sequence _wanted options expl option \
-- 
2.17.1

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

[Daniel Shahaf]

doron.behar@gmail.com 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)

> +        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.

> +            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.

>

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

[Doron Behar]

On Sat, Jun 09, 2018 at 06:21:55PM +0000, Daniel Shahaf wrote:
> doron.behar@gmail.com 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`.

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

[Daniel Shahaf]

Doron Behar wrote on Sat, 09 Jun 2018 21:59 +0300:
>     _describe -t public-keys 'public key' emails uids
> 
> Is that what you meant by using two arrays?

Yes.

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

[Doron Behar]

On Sat, Jun 09, 2018 at 07:23:25PM +0000, Daniel Shahaf wrote:
> Doron Behar wrote on Sat, 09 Jun 2018 21:59 +0300:
> >     _describe -t public-keys 'public key' emails uids
> > 
> > Is that what you meant by using two arrays?
> 
> Yes.

Then I guess we are ready for a final patch.

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

[Daniel Shahaf]

Doron Behar wrote on Thu, Jun 28, 2018 at 16:43:10 +0300:
> Hey guys, just reminding you there is still a patch here waiting for you
> to merge it.

Merged.

I apologise for the delay; it didn't register with me I was the only one who
reviewed earlier iterations.  Mea culpa.

Daniel

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

[Doron Behar]

Hey guys, just reminding you there is still a patch here waiting for you
to merge it.

On Sat, Jun 23, 2018 at 08:26:37PM +0300, Doron Behar wrote:
> Hey guys, is this patch O.K? Can we merge it please? I've been waiting a
> while for a response now, it'll be great to get some feedback.
> 
> On Mon, Jun 18, 2018 at 10:47:48PM +0300, doron.behar@gmail.com wrote:
> > From: Doron Behar <doron.behar@gmail.com>
> > 
> > Use the `--with-colons` option and parse the output according to the
> > format specified in the documentation.
> > ---
> >  Completion/Unix/Command/_gpg | 62 ++++++++++++++++++++++++++++++++----
> >  1 file changed, 55 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Completion/Unix/Command/_gpg b/Completion/Unix/Command/_gpg
> > index 48a36eff2..b35693d1f 100644
> > --- a/Completion/Unix/Command/_gpg
> > +++ b/Completion/Unix/Command/_gpg
> > @@ -206,20 +206,68 @@ 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_lines=(${(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 j parts current_uid
> > +    for (( i = 1; i < ${#public_keys_lines[@]}; ++i )); do
> > +      parts=("${(@s.:.)public_keys_lines[$i]}")
> > +      if [[ ${parts[1]} == "fpr" ]]; then
> > +        current_uid="${parts[10]}"
> > +        i=$((i + 1))
> > +        parts=("${(@s.:.)public_keys_lines[$i]}")
> > +        while [[ ${parts[1]} == "uid" ]]; do
> > +          uids+=("${current_uid}")
> > +          emails+=("${parts[10]}")
> > +          i=$((i + 1))
> > +          parts=("${(@s.:.)public_keys_lines[$i]}")
> > +        done
> > +      fi
> > +    done
> > +    _describe -t public-keys 'public key' emails uids
> >    ;;
> >    secret-keys)
> > -    _wanted secret-keys expl 'secret key' compadd \
> > -	${${(Mo)$(_call_program secret-keys $words[1] $needed --list-secret-keys --list-options no-show-photos):%<*>}//(<|>)/} && return
> > +    local secret_keys_lines=(${(f)"$(_call_program secret-keys ${(q)words[1]} ${(q)needed} --list-secret-keys --list-options no-show-photos --with-colons)"})
> > +    local -a uids emails
> > +    local i j parts current_uid
> > +    for (( i = 1; i < ${#secret_keys_lines[@]}; ++i )); do
> > +      parts=("${(@s.:.)secret_keys_lines[$i]}")
> > +      if [[ ${parts[1]} == "fpr" ]]; then
> > +        current_uid="${parts[10]}"
> > +        i=$((i + 1))
> > +        parts=("${(@s.:.)secret_keys_lines[$i]}")
> > +        while [[ ${parts[1]} == "uid" ]]; do
> > +          uids+=("${current_uid}")
> > +          emails+=("${parts[10]}")
> > +          i=$((i + 1))
> > +          parts=("${(@s.:.)secret_keys_lines[$i]}")
> > +        done
> > +      fi
> > +    done
> > +    _describe -t secret-keys 'secret key' emails uids
> >    ;;
> >    ciphers)
> >      _wanted ciphers expl cipher compadd \
> > -        ${${(s.,.)${(M)${(f)${"$(_call_program ciphers $words[1] $needed --version)"}//,$'\n' #/, }:#Cipher*}#*:}# } && return
> > +        ${${(s.,.)${(M)${(f)${"$(_call_program ciphers ${(q)words[1]} ${(q)needed} --version)"}//,$'\n' #/, }:#Cipher*}#*:}# } && return
> >    ;;
> >    (public-keyids)
> > -    _wanted public-keys expl 'public keyid' \
> > -      compadd ${(M)${${(f)"$(_call_program public-keyids $words[1] $needed --list-public-keys --list-options no-show-photos)"}## #}:#[0-9A-F](#c40)} && return
> > +    local public_keys_lines=(${(f)"$(_call_program public-keyids ${(q)words[1]} ${(q)needed} --list-public-keys --list-options no-show-photos --with-colons)"})
> > +    local -a uids emails
> > +    local i j parts current_uid
> > +    for (( i = 1; i < ${#public_keys_lines[@]}; ++i )); do
> > +      parts=("${(@s.:.)public_keys_lines[$i]}")
> > +      if [[ ${parts[1]} == "fpr" ]]; then
> > +        current_uid="${parts[10]}"
> > +        i=$((i + 1))
> > +        parts=("${(@s.:.)public_keys_lines[$i]}")
> > +        while [[ ${parts[1]} == "uid" ]]; do
> > +          uids+=("${current_uid}")
> > +          emails+=("${parts[10]}")
> > +          i=$((i + 1))
> > +          parts=("${(@s.:.)public_keys_lines[$i]}")
> > +        done
> > +      fi
> > +    done
> > +    _describe -t public-keyids 'public key' emails uids
> >    ;;
> >    (option-list)
> >      _sequence _wanted options expl option \
> > -- 
> > 2.17.1
> > 

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

[Doron Behar]

Hey guys, is this patch O.K? Can we merge it please? I've been waiting a
while for a response now, it'll be great to get some feedback.

On Mon, Jun 18, 2018 at 10:47:48PM +0300, doron.behar@gmail.com wrote:
> From: Doron Behar <doron.behar@gmail.com>
> 
> Use the `--with-colons` option and parse the output according to the
> format specified in the documentation.
> ---
>  Completion/Unix/Command/_gpg | 62 ++++++++++++++++++++++++++++++++----
>  1 file changed, 55 insertions(+), 7 deletions(-)
> 
> diff --git a/Completion/Unix/Command/_gpg b/Completion/Unix/Command/_gpg
> index 48a36eff2..b35693d1f 100644
> --- a/Completion/Unix/Command/_gpg
> +++ b/Completion/Unix/Command/_gpg
> @@ -206,20 +206,68 @@ 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_lines=(${(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 j parts current_uid
> +    for (( i = 1; i < ${#public_keys_lines[@]}; ++i )); do
> +      parts=("${(@s.:.)public_keys_lines[$i]}")
> +      if [[ ${parts[1]} == "fpr" ]]; then
> +        current_uid="${parts[10]}"
> +        i=$((i + 1))
> +        parts=("${(@s.:.)public_keys_lines[$i]}")
> +        while [[ ${parts[1]} == "uid" ]]; do
> +          uids+=("${current_uid}")
> +          emails+=("${parts[10]}")
> +          i=$((i + 1))
> +          parts=("${(@s.:.)public_keys_lines[$i]}")
> +        done
> +      fi
> +    done
> +    _describe -t public-keys 'public key' emails uids
>    ;;
>    secret-keys)
> -    _wanted secret-keys expl 'secret key' compadd \
> -	${${(Mo)$(_call_program secret-keys $words[1] $needed --list-secret-keys --list-options no-show-photos):%<*>}//(<|>)/} && return
> +    local secret_keys_lines=(${(f)"$(_call_program secret-keys ${(q)words[1]} ${(q)needed} --list-secret-keys --list-options no-show-photos --with-colons)"})
> +    local -a uids emails
> +    local i j parts current_uid
> +    for (( i = 1; i < ${#secret_keys_lines[@]}; ++i )); do
> +      parts=("${(@s.:.)secret_keys_lines[$i]}")
> +      if [[ ${parts[1]} == "fpr" ]]; then
> +        current_uid="${parts[10]}"
> +        i=$((i + 1))
> +        parts=("${(@s.:.)secret_keys_lines[$i]}")
> +        while [[ ${parts[1]} == "uid" ]]; do
> +          uids+=("${current_uid}")
> +          emails+=("${parts[10]}")
> +          i=$((i + 1))
> +          parts=("${(@s.:.)secret_keys_lines[$i]}")
> +        done
> +      fi
> +    done
> +    _describe -t secret-keys 'secret key' emails uids
>    ;;
>    ciphers)
>      _wanted ciphers expl cipher compadd \
> -        ${${(s.,.)${(M)${(f)${"$(_call_program ciphers $words[1] $needed --version)"}//,$'\n' #/, }:#Cipher*}#*:}# } && return
> +        ${${(s.,.)${(M)${(f)${"$(_call_program ciphers ${(q)words[1]} ${(q)needed} --version)"}//,$'\n' #/, }:#Cipher*}#*:}# } && return
>    ;;
>    (public-keyids)
> -    _wanted public-keys expl 'public keyid' \
> -      compadd ${(M)${${(f)"$(_call_program public-keyids $words[1] $needed --list-public-keys --list-options no-show-photos)"}## #}:#[0-9A-F](#c40)} && return
> +    local public_keys_lines=(${(f)"$(_call_program public-keyids ${(q)words[1]} ${(q)needed} --list-public-keys --list-options no-show-photos --with-colons)"})
> +    local -a uids emails
> +    local i j parts current_uid
> +    for (( i = 1; i < ${#public_keys_lines[@]}; ++i )); do
> +      parts=("${(@s.:.)public_keys_lines[$i]}")
> +      if [[ ${parts[1]} == "fpr" ]]; then
> +        current_uid="${parts[10]}"
> +        i=$((i + 1))
> +        parts=("${(@s.:.)public_keys_lines[$i]}")
> +        while [[ ${parts[1]} == "uid" ]]; do
> +          uids+=("${current_uid}")
> +          emails+=("${parts[10]}")
> +          i=$((i + 1))
> +          parts=("${(@s.:.)public_keys_lines[$i]}")
> +        done
> +      fi
> +    done
> +    _describe -t public-keyids 'public key' emails uids
>    ;;
>    (option-list)
>      _sequence _wanted options expl option \
> -- 
> 2.17.1
> 

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

[doron.behar]

From: Doron Behar <doron.behar@gmail.com>

Use the `--with-colons` option and parse the output according to the
format specified in the documentation.
---
 Completion/Unix/Command/_gpg | 62 ++++++++++++++++++++++++++++++++----
 1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/Completion/Unix/Command/_gpg b/Completion/Unix/Command/_gpg
index 48a36eff2..b35693d1f 100644
--- a/Completion/Unix/Command/_gpg
+++ b/Completion/Unix/Command/_gpg
@@ -206,20 +206,68 @@ 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_lines=(${(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 j parts current_uid
+    for (( i = 1; i < ${#public_keys_lines[@]}; ++i )); do
+      parts=("${(@s.:.)public_keys_lines[$i]}")
+      if [[ ${parts[1]} == "fpr" ]]; then
+        current_uid="${parts[10]}"
+        i=$((i + 1))
+        parts=("${(@s.:.)public_keys_lines[$i]}")
+        while [[ ${parts[1]} == "uid" ]]; do
+          uids+=("${current_uid}")
+          emails+=("${parts[10]}")
+          i=$((i + 1))
+          parts=("${(@s.:.)public_keys_lines[$i]}")
+        done
+      fi
+    done
+    _describe -t public-keys 'public key' emails uids
   ;;
   secret-keys)
-    _wanted secret-keys expl 'secret key' compadd \
-	${${(Mo)$(_call_program secret-keys $words[1] $needed --list-secret-keys --list-options no-show-photos):%<*>}//(<|>)/} && return
+    local secret_keys_lines=(${(f)"$(_call_program secret-keys ${(q)words[1]} ${(q)needed} --list-secret-keys --list-options no-show-photos --with-colons)"})
+    local -a uids emails
+    local i j parts current_uid
+    for (( i = 1; i < ${#secret_keys_lines[@]}; ++i )); do
+      parts=("${(@s.:.)secret_keys_lines[$i]}")
+      if [[ ${parts[1]} == "fpr" ]]; then
+        current_uid="${parts[10]}"
+        i=$((i + 1))
+        parts=("${(@s.:.)secret_keys_lines[$i]}")
+        while [[ ${parts[1]} == "uid" ]]; do
+          uids+=("${current_uid}")
+          emails+=("${parts[10]}")
+          i=$((i + 1))
+          parts=("${(@s.:.)secret_keys_lines[$i]}")
+        done
+      fi
+    done
+    _describe -t secret-keys 'secret key' emails uids
   ;;
   ciphers)
     _wanted ciphers expl cipher compadd \
-        ${${(s.,.)${(M)${(f)${"$(_call_program ciphers $words[1] $needed --version)"}//,$'\n' #/, }:#Cipher*}#*:}# } && return
+        ${${(s.,.)${(M)${(f)${"$(_call_program ciphers ${(q)words[1]} ${(q)needed} --version)"}//,$'\n' #/, }:#Cipher*}#*:}# } && return
   ;;
   (public-keyids)
-    _wanted public-keys expl 'public keyid' \
-      compadd ${(M)${${(f)"$(_call_program public-keyids $words[1] $needed --list-public-keys --list-options no-show-photos)"}## #}:#[0-9A-F](#c40)} && return
+    local public_keys_lines=(${(f)"$(_call_program public-keyids ${(q)words[1]} ${(q)needed} --list-public-keys --list-options no-show-photos --with-colons)"})
+    local -a uids emails
+    local i j parts current_uid
+    for (( i = 1; i < ${#public_keys_lines[@]}; ++i )); do
+      parts=("${(@s.:.)public_keys_lines[$i]}")
+      if [[ ${parts[1]} == "fpr" ]]; then
+        current_uid="${parts[10]}"
+        i=$((i + 1))
+        parts=("${(@s.:.)public_keys_lines[$i]}")
+        while [[ ${parts[1]} == "uid" ]]; do
+          uids+=("${current_uid}")
+          emails+=("${parts[10]}")
+          i=$((i + 1))
+          parts=("${(@s.:.)public_keys_lines[$i]}")
+        done
+      fi
+    done
+    _describe -t public-keyids 'public key' emails uids
   ;;
   (option-list)
     _sequence _wanted options expl option \
-- 
2.17.1