On Thu, Apr 12, 2012 at 11:50 PM, Allan McRae <[email protected]> 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 <[email protected]> > > --- > > 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 > > >
