[pacman-dev] [PATCH 4/8] pacman-key: adopt parseopts for option parsing

[Dave Reisner]

On Thu, Apr 12, 2012 at 11:50 PM, Allan McRae <allan at archlinux.org> wrote:

> On 13/04/12 00:54, Dave Reisner wrote:
> > This requires an ugly amount of reworking of how pacman-key handles
> > options. The change simply to avoid passing keys, files, and directories
> > as arguments to options, but to leave them as arguments to the overall
> > program. This is reasonable since pacman-key limits the user to
> > essentially one operation per invocation (like pacman).
> >
> > Since we now pass around the positional parameters to the various
> > operations, we can add some better sanity checking. Each operation is
> > responsible for testing input and making sure it can operate properly,
> > otherwise it throws an error and exits.
> >
> > The doc is updated to reflect this, and uses similar verbiage as pacman,
> > describing the non-option arguments now passed to pacman-key as targets.
> >
> > Similar to the doc, --help is reorganized to separate operations and
> > options and remove argument tokens from operations.
> >
> > Signed-off-by: Dave Reisner <dreisner at archlinux.org>
> > ---
> > I really hate this patch, but I don't think it makes sense to split it.
>
> Agreed.  One patch is fine.
>
> <snip>
>
> > +     if (( $# == 0 )); then
> > +             error "$(gettext "No keys specified")"
> > +             exit 1
> > +     fi
>
> This is repeated so many times...   How about doing a single check below
> the check that only one operation is specified?
>

Because that would break any option that really can accept 0 arguments,
such as --list-keys or --refresh-keys.



> <snip>
>
> > @@ -413,7 +428,7 @@ list_sigs() {
> >
> >  lsign_keys() {
> >       check_keyids_exist
> > -     printf 'y\ny\n' | LANG=C "${GPG_PACMAN[@]}" --command-fd 0 --quiet
> --batch --lsign-key "${KEYIDS[@]}" 2>/dev/null
> > +     printf '%s\n' y y | LANG=C "${GPG_PACMAN[@]}" --command-fd 0
> --quiet --batch --lsign-key "$@" 2>/dev/null
> >       if (( PIPESTATUS[1] )); then
> >               error "$(gettext "A specified key could not be locally
> signed.")"
> >               exit 1
>
> Is there a good reason beyond the cosmetic for the printf change?
> Because I think it fails if it is only cosmetic...
>
>
It's only cosmetic, but I don't see how it fails... Anyways, it shouldn't
be in the patch, so it's gone from my tree.


> Allan
>
>
>