On Fri, Apr 13, 2012 at 8:39 AM, Allan McRae <al...@archlinux.org> wrote:
> On 13/04/12 22:31, Dave Reisner wrote: > > On Thu, Apr 12, 2012 at 11:50 PM, Allan McRae <al...@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 <dreis...@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)
