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

[Dave Reisner]

On Fri, Apr 13, 2012 at 8:39 AM, Allan McRae <allan at archlinux.org> wrote:

> On 13/04/12 22:31, Dave Reisner wrote:
> > 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.
> >
>
> Yeah. I meant something like:
>
> if (( (ADD || DELETE || EDITKEY || ....) && $# == 0 )); then
>
>
Ah, right.  Yep, I can do that... looking back, seems I missed this in a
few places anyways (import, verify, etc)