From: Daniel Shahaf <d.s@daniel.shahaf.name>
To: doron.behar@gmail.com
Cc: zsh-workers@zsh.org
Subject: Re: [PATCH 1/1] _gpg: Use explicit UIDs for state = public keys.
Date: Sat, 26 May 2018 16:25:41 +0000 [thread overview]
Message-ID: <20180526162541.ejfzrpqwl5lwjkzb@tarpaulin.shahaf.local2> (raw)
In-Reply-To: <20180526151628.17983-2-doron.behar@gmail.com>
On Sat, May 26, 2018 at 06:16:28PM +0300, doron.behar@gmail.com wrote:
> From: Doron Behar <doron.behar@gmail.com>
>
> Use the `--with-colons` option and parse the output while IFS=":"
> according to the output format.
> ---
> Completion/Unix/Command/_gpg | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/Completion/Unix/Command/_gpg b/Completion/Unix/Command/_gpg
> index 48a36eff2..71fa7667d 100644
> --- a/Completion/Unix/Command/_gpg
> +++ b/Completion/Unix/Command/_gpg
> @@ -206,8 +206,19 @@ 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
> + OLDIFS="${IFS}"
OLDIFS isn't made local so it leaks.
You could alternatively have set IFS in a precommand assignment (e.g., «IFS=:
/bin/echo foo:bar»), or made it local to a function. Another approach is use
parameter expansion flags such as «${(s.:.)foo}».
> + IFS=":"
> + public_keys=($($words[1] $needed --list-public-keys --list-options no-show-photos --with-colons))
Another parameter leak. (See WARN_CREATE_GLOBAL.)
Why did you remove the use of _call_program?
> + for i in {1..${#public_keys[@]}}; do
> + if [[ ${public_keys[$i]} =~ "fpr" ]] && [[ ${public_keys[$((i + 19))]} =~ "@" ]] ; then
The parameter 'i' leaks.
"fpr" should be looked for as a complete string, not as a substring. Also, it
should be looked for only in the first column, not in every single output
field.
A subscript is always parsed as a math context so you can just do «$foo[i+9]»
without an additional $((…)) inside.
> + # +9 is the uid
> + # +19 is the description
> + uids_and_emails+=(${public_keys[$((i + 9))]}":"${public_keys[$((i + 19))]})
I'm sorry, but that's not forward compatible. The output format spec
(doc/DETAILS, which is referred to from the manpage) states that fields may be
added in the future, so that "19" may not be hardcoded here.
> + i=$((i+20))
This line doesn't have any effect, does it? This isn't an arithmetic for, it's
a list-of-words for, and the next word is equal to $(( pre_assignment_value_of_i + 1 )).
> + fi
> + done
> + _describe uids uids_and_emails
The use of describe doesn't set the 'public-keys' tag that _wanted set. (You
need to pass -t.)
> + IFS="${OLDIFS}"
Thanks for the patch. I agree that it would be better to use --with-colons.
We look forward to a revised patch. :-)
Cheers,
Daniel
next prev parent reply other threads:[~2018-05-26 16:36 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-26 15:16 [PATCH 0/1] *** Small improvement for gpg completion *** doron.behar
2018-05-26 15:16 ` [PATCH 1/1] _gpg: Use explicit UIDs for state = public keys doron.behar
2018-05-26 16:25 ` Daniel Shahaf [this message]
2018-05-29 14:11 ` Doron Behar
2018-05-30 19:04 ` Daniel Shahaf
2018-06-02 15:08 ` Doron Behar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180526162541.ejfzrpqwl5lwjkzb@tarpaulin.shahaf.local2 \
--to=d.s@daniel.shahaf.name \
--cc=doron.behar@gmail.com \
--cc=zsh-workers@zsh.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.vuxu.org/mirror/zsh/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).