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

Allan McRae allan at archlinux.org
Thu Apr 12 23:50:36 EDT 2012

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.


> +	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?


> @@ -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...


More information about the pacman-dev mailing list