FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 07-21-2011, 08:40 PM
Dave Reisner
 
Default scripts/library: rewrite parse_options

In addition to being what I feel is a cleaner and faster implementation,
we avoid the use of eval by normalizing option arguments into a global
array which is then set after a successful call to parse_options.

This trims out the idea of having multiple arguments to a single option,
making our parsing algorithm a little more sane. We never took advantage of
this in makepkg (for the one option that feasibly supports it), and I
think we've overlooked a much simpler solution in pacman-key. Since
actions are limited to 1 at a time the leftover positional parameters
become the keys or keyfiles which are acted upon.

Also added is a new test directory test/scripts with a harness for
parse_options, run as part of make check.

Signed-off-by: Dave Reisner <dreisner@archlinux.org>
---
Thoughts? I know Allan wasn't quite sold on this, as the downside is that we
sort of pigeonhole ourselves into using the non-optional parameter as arguments
to our action. This has zero effect on makepkg.

I've also thought to add --option=arg syntax parsing, but I'm not sure we need
this.

Makefile.am | 6 +-
configure.ac | 1 +
scripts/library/parse_options.sh | 195 +++++++++++++++++-------------------
scripts/makepkg.sh.in | 13 +--
scripts/pacman-key.sh.in | 59 ++++++------
test/scripts/Makefile.am | 9 ++
test/scripts/parse_options_test.sh | 103 +++++++++++++++++++
7 files changed, 242 insertions(+), 144 deletions(-)
rewrite scripts/library/parse_options.sh (89%)
create mode 100644 test/scripts/Makefile.am
create mode 100755 test/scripts/parse_options_test.sh

diff --git a/Makefile.am b/Makefile.am
index 286e6ae..f88b6d1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1,4 +1,4 @@
-SUBDIRS = lib/libalpm src/util src/pacman scripts etc test/pacman test/util contrib
+SUBDIRS = lib/libalpm src/util src/pacman scripts etc test/pacman test/scripts test/util contrib
if WANT_DOC
SUBDIRS += doc
endif
@@ -21,12 +21,14 @@ dist_pkgdata_DATA =
proto/ChangeLog.proto

# run the pactest test suite and vercmp tests
-check-local: test/pacman test/util src/pacman src/util
+check-local: test/pacman test/scripts test/util src/pacman src/util
$(PYTHON) $(top_srcdir)/test/pacman/pactest.py --debug=1
--test $(top_srcdir)/test/pacman/tests/*.py
-p $(top_builddir)/src/pacman/pacman
$(SH) $(top_srcdir)/test/util/vercmptest.sh
$(top_builddir)/src/util/vercmp
+ $(BASH_SHELL) $(top_srcdir)/test/scripts/parse_options_test.sh
+ $(top_builddir)/scripts/library/parse_options.sh

# create the pacman DB and cache directories upon install
install-data-local:
diff --git a/configure.ac b/configure.ac
index 6ad5be5..a553176 100644
--- a/configure.ac
+++ b/configure.ac
@@ -381,6 +381,7 @@ doc/Makefile
etc/Makefile
test/pacman/Makefile
test/pacman/tests/Makefile
+test/scripts/Makefile
test/util/Makefile
contrib/Makefile
Makefile
diff --git a/scripts/library/parse_options.sh b/scripts/library/parse_options.sh
dissimilarity index 89%
index 48fd42c..215059f 100644
--- a/scripts/library/parse_options.sh
+++ b/scripts/library/parse_options.sh
@@ -1,105 +1,90 @@
-# getopt like parser
-parse_options() {
- local short_options=$1; shift;
- local long_options=$1; shift;
- local ret=0;
- local unused_options=""
- local i
-
- while [[ -n $1 ]]; do
- if [[ ${1:0:2} = '--' ]]; then
- if [[ -n ${1:2} ]]; then
- local match=""
- for i in ${long_options//,/ }; do
- if [[ ${1:2} = ${i//:} ]]; then
- match=$i
- break
- fi
- done
- if [[ -n $match ]]; then
- local needsargument=0
-
- [[ ${match} = ${1:2}: ]] && needsargument=1
- [[ ${match} = ${1:2}:: && -n $2 && ${2:0:1} != "-" ]] && needsargument=1
-
- if (( ! needsargument )); then
- printf ' %s' "$1"
- else
- if [[ -n $2 ]]; then
- printf ' %s ' "$1"
- shift
- printf "'%q" "$1"
- while [[ -n $2 && ${2:0:1} != "-" ]]; do
- shift
- printf " %q" "$1"
- done
- printf "'"
- else
- printf "@SCRIPTNAME@: $(gettext "option %s requires an argument
")" "'$1'" >&2
- ret=1
- fi
- fi
- else
- echo "@SCRIPTNAME@: $(gettext "unrecognized option") '$1'" >&2
- ret=1
- fi
- else
- shift
- break
- fi
- elif [[ ${1:0:1} = '-' ]]; then
- for ((i=1; i<${#1}; i++)); do
- if [[ $short_options =~ ${1:i:1} ]]; then
- local needsargument=0
-
- [[ $short_options =~ ${1:i:1}: && ! $short_options =~ ${1:i:1}:: ]] && needsargument=1
- [[ $short_options =~ ${1:i:1}:: &&
- ( -n ${1:$i+1} || ( -n $2 && ${2:0:1} != "-" ) ) ]] && needsargument=1
-
- if (( ! needsargument )); then
- printf ' -%s' "${1:i:1}"
- else
- if [[ -n ${1:$i+1} ]]; then
- printf ' -%s ' "${1:i:1}"
- printf "'%q" "${1:$i+1}"
- while [[ -n $2 && ${2:0:1} != "-" ]]; do
- shift
- printf " %q" "$1"
- done
- printf "'"
- else
- if [[ -n $2 ]]; then
- printf ' -%s ' "${1:i:1}"
- shift
- printf "'%q" "$1"
- while [[ -n $2 && ${2:0:1} != "-" ]]; do
- shift
- printf " %q" "$1"
- done
- printf "'"
-
- else
- printf "@SCRIPTNAME@: $(gettext "option %s requires an argument
")" "'-${1:i:1}'" >&2
- ret=1
- fi
- fi
- break
- fi
- else
- echo "@SCRIPTNAME@: $(gettext "unrecognized option") '-${1:i:1}'" >&2
- ret=1
- fi
- done
- else
- unused_options="${unused_options} '$1'"
- fi
- shift
- done
-
- printf " --"
- [[ $unused_options ]] && printf ' %s' "${unused_options[@]}"
- [[ $1 ]] && printf " '%s'" "$@"
- printf "
"
-
- return $ret
-} No newline at end of file
+# getopt-like parser
+parse_options () {
+ local opt= i= shortopts=$1
+ local -a longopts param
+
+ # split the longopt array on commas
+ IFS=, read -r -a longopts <<< "$2"
+ shift 2
+
+ while (( $# )); do
+ case $1 in
+ --) # explicit end of options
+ shift
+ break
+ ;;
+ -[!-]*) # short option
+ for (( i = 1; i < ${#1}; i++ )); do
+ opt=${1:i:1}
+
+ # option doesn't exist
+ if [[ $shortopts != *$opt* ]]; then
+ echo "@SCRIPTNAME@: $(gettext "unrecognized option") '-$opt'" >&2
+ OPTRET=(--)
+ return 1
+ fi
+
+ OPTRET+=("-$opt")
+ # option requires optarg
+ if [[ $shortopts = *$opt:* ]]; then
+ # if we're not at the end of the option chunk, the rest is the optarg
+ if (( i < ${#1} - 1 )); then
+ OPTRET+=("${1:i+1}")
+ break
+
+ # if we're at the end, grab the the next positional, if it exists
+ elif (( i == ${#1} - 1 )) && [[ $2 ]]; then
+ OPTRET+=("$2")
+ shift
+ break
+
+ # parse failure
+ else
+ printf "@SCRIPTNAME@: $(gettext "option %s requires an argument
")" "'-$opt'" >&2
+ OPTRET=(--)
+ return 1
+ fi
+ fi
+
+ done
+ ;;
+ --?*) # long option
+ opt=${1#--}
+ OPTRET+=("$1")
+
+ for longopt in "${longopts[@]}"; do
+ if [[ $longopt =~ ^($opt)(:?)$ ]]; then
+ # option requires optarg
+ if [[ -n ${BASH_REMATCH[2]} ]]; then
+ if [[ -z $2 ]]; then
+ printf "@SCRIPTNAME@: $(gettext "option %s requires an argument
")" "'--$opt'" >&2
+ OPTRET=(--)
+ return 1
+ fi
+ OPTRET+=("$2")
+ shift 2
+ continue 2
+
+ # simple longopt
+ elif [[ -n ${BASH_REMATCH[1]} ]]; then
+ shift
+ continue 2
+ fi
+ fi
+ done
+ echo "@SCRIPTNAME@: $(gettext "unrecognized option") '--$opt'" >&2
+ OPTRET=(--)
+ return 1
+ ;;
+ *) # non-option arg encountered, add it as a parameter
+ param+=("$1")
+ ;;
+ esac
+ shift
+ done
+
+ # add end-of-opt terminator and any leftover positional parameters
+ OPTRET+=('--' "${param[@]}" "$@")
+
+ return 0
+}
diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
index a4e7156..b6ab2ee 100644
--- a/scripts/makepkg.sh.in
+++ b/scripts/makepkg.sh.in
@@ -1755,14 +1755,14 @@ OPT_LONG+=",install,key:,log,nocolor,nobuild,noche ck,nosign,pkg:,rmdeps"
OPT_LONG+=",repackage,skipinteg,sign,source,syncde ps,version,config:"
# Pacman Options
OPT_LONG+=",noconfirm,noprogressbar"
-if ! OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@")"; then
+if ! parse_options "$OPT_SHORT" "$OPT_LONG" "$@"; then
echo; usage; exit 1 # E_INVALID_OPTION;
fi
-eval set -- "$OPT_TEMP"
-unset OPT_SHORT OPT_LONG OPT_TEMP
+set -- "${OPTRET[@]}"
+unset OPT_SHORT OPT_LONG OPTRET

-while true; do
- case "$1" in
+while (( $# )); do
+ case $1 in
# Pacman Options
--noconfirm) PACMAN_OPTS+=" --noconfirm" ;;
--noprogressbar) PACMAN_OPTS+=" --noprogressbar" ;;
@@ -1801,8 +1801,7 @@ while true; do
-h|--help) usage; exit 0 ;; # E_OK
-V|--version) version; exit 0 ;; # E_OK

- --) OPT_IND=0; shift; break;;
- *) usage; exit 1 ;; # E_INVALID_OPTION
+ --) shift; break;;
esac
shift
done
diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in
index 5cb5bd5..d0fb0ef 100644
--- a/scripts/pacman-key.sh.in
+++ b/scripts/pacman-key.sh.in
@@ -255,16 +255,16 @@ reload_keyring() {
}

receive_keys() {
- if [[ -z ${KEYIDS[@]} ]]; then
+ if [[ -z $1 ]]; then
error "$(gettext "You need to specify the keyserver and at least one key identifier")"
exit 1
fi
- ${GPG_PACMAN} --keyserver "$KEYSERVER" --recv-keys "${KEYIDS[@]}"
+ ${GPG_PACMAN} --keyserver "$KEYSERVER" --recv-keys "$@"
}

edit_keys() {
local errors=0;
- for key in ${KEYIDS[@]}; do
+ for key; do
# Verify if the key exists in pacman's keyring
if ! ${GPG_PACMAN} --list-keys "$key" &>/dev/null; then
error "$(gettext "The key identified by %s does not exist")" "$key"
@@ -273,7 +273,7 @@ edit_keys() {
done
(( errors )) && exit 1;

- for key in ${KEYIDS[@]}; do
+ for key; do
${GPG_PACMAN} --edit-key "$key"
done
}
@@ -285,32 +285,33 @@ if ! type gettext &>/dev/null; then
}
fi

-OPT_SHORT="a::d:e:f::hlr:uv:V"
-OPT_LONG="add::,config:,delete:,edit-key:,export::,finger::,gpgdir:"
+declare -a OPTRET
+OPT_SHORT="ade:fhlr:uv:V"
+OPT_LONG="add,config:,delete,edit-key,export,finger,gpgdir:"
OPT_LONG+=",help,init,list,receive:,reload,updated b,verify:,version"
-if ! OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@")"; then
+if ! parse_options "$OPT_SHORT" "$OPT_LONG" "$@"; then
echo; usage; exit 1 # E_INVALID_OPTION;
fi
-eval set -- "$OPT_TEMP"
-unset OPT_SHORT OPT_LONG OPT_TEMP
+set -- "${OPTRET[@]}"
+unset OPT_SHORT OPT_LONG OPTRET

-if [[ $1 == "--" ]]; then
- usage;
- exit 0;
+if [[ -z $1 ]]; then
+ usage
+ exit 0
fi

-while true; do
- case "$1" in
- -a|--add) ADD=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYFILES=($1) ;;
+while (( $# )); do
+ case $1 in
+ -a|--add) ADD=1 ;;
--config) shift; CONFIG=$1 ;;
- -d|--delete) DELETE=1; shift; KEYIDS=($1) ;;
- --edit-key) EDITKEY=1; shift; KEYIDS=($1) ;;
- -e|--export) EXPORT=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;;
- -f|--finger) FINGER=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;;
+ -d|--delete) DELETE=1 ;;
+ --edit-key) EDITKEY=1 ;;
+ -e|--export) EXPORT=1 ;;
+ -f|--finger) FINGER=1 ;;
--gpgdir) shift; PACMAN_KEYRING_DIR=$1 ;;
--init) INIT=1 ;;
-l|--list) LIST=1 ;;
- -r|--receive) RECEIVE=1; shift; KEYSERVER=$1; KEYIDS=("${@:1}") ;;
+ -r|--receive) RECEIVE=1; shift; KEYSERVER=$1 ;;
--reload) RELOAD=1 ;;
-u|--updatedb) UPDATEDB=1 ;;
-v|--verify) VERIFY=1; shift; SIGNATURE=$1 ;;
@@ -318,13 +319,11 @@ while true; do
-h|--help) usage; exit 0 ;;
-V|--version) version; exit 0 ;;

- --) OPT_IND=0; shift; break;;
- *) usage; exit 1 ;;
+ --) shift; break ;;
esac
shift
done

-
if ! type -p gpg >/dev/null; then
error "$(gettext "Cannot find the %s binary required for all %s operations.")" "gpg" "pacman-key"
exit 1
@@ -364,14 +363,14 @@ esac

(( ! INIT )) && check_keyring

-(( ADD )) && ${GPG_PACMAN} --quiet --batch --import "${KEYFILES[@]}"
-(( DELETE )) && ${GPG_PACMAN} --quiet --batch --delete-key --yes "${KEYIDS[@]}"
-(( EDITKEY )) && edit_keys
-(( EXPORT )) && ${GPG_PACMAN} --armor --export "${KEYIDS[@]}"
-(( FINGER )) && ${GPG_PACMAN} --batch --fingerprint "${KEYIDS[@]}"
+(( ADD )) && ${GPG_PACMAN} --quiet --batch --import "$@"
+(( DELETE )) && ${GPG_PACMAN} --quiet --batch --delete-key --yes "$@"
+(( EDITKEY )) && edit_keys "$@"
+(( EXPORT )) && ${GPG_PACMAN} --armor --export "$@"
+(( FINGER )) && ${GPG_PACMAN} --batch --fingerprint "$@"
(( INIT )) && initialize
-(( LIST )) && ${GPG_PACMAN} --batch --list-sigs "${KEYIDS[@]}"
-(( RECEIVE )) && receive_keys
+(( LIST )) && ${GPG_PACMAN} --batch --list-sigs "$@"
+(( RECEIVE )) && receive_keys "$@"
(( RELOAD )) && reload_keyring
(( UPDATEDB )) && ${GPG_PACMAN} --batch --check-trustdb
(( VERIFY )) && ${GPG_PACMAN} --verify $SIGNATURE
diff --git a/test/scripts/Makefile.am b/test/scripts/Makefile.am
new file mode 100644
index 0000000..caa9ab1
--- /dev/null
+++ b/test/scripts/Makefile.am
@@ -0,0 +1,9 @@
+check_SCRIPTS =
+ parse_options_test.sh
+
+noinst_SCRIPTS = $(check_SCRIPTS)
+
+EXTRA_DIST =
+ $(check_SCRIPTS)
+
+# vim:set ts=2 sw=2 noet:
diff --git a/test/scripts/parse_options_test.sh b/test/scripts/parse_options_test.sh
new file mode 100755
index 0000000..85981e1
--- /dev/null
+++ b/test/scripts/parse_options_test.sh
@@ -0,0 +1,103 @@
+#!/bin/bash
+
+declare -i testcount=0 pass=0 fail=0
+
+# source the library function
+if [[ -z $1 || ! -f $1 ]]; then
+ printf "error: path to parse_option library not provided or does not exist
"
+ exit 1
+fi
+. "$1"
+
+if ! type -t parse_options >/dev/null; then
+ printf 'parse_options function not found
'
+ exit 1
+fi
+
+# borrow opts from makepkg
+OPT_SHORT="AcdefFghiLmop:rRsV"
+OPT_LONG="allsource,asroot,ignorearch,check,clean ,nodeps"
+OPT_LONG+=",noextract,force,forcever:,geninteg,he lp,holdver"
+OPT_LONG+=",install,key:,log,nocolor,nobuild,noch eck,nosign,pkg:,rmdeps"
+OPT_LONG+=",repackage,skipinteg,sign,source,syncd eps,version,config:"
+OPT_LONG+=",noconfirm,noprogressbar"
+
+parse() {
+ local result=$1 tokencount=$2; shift 2
+
+ (( ++testcount ))
+ #printf '[TEST %2s]: ' "$testcount"
+ parse_options "$OPT_SHORT" "$OPT_LONG" "$@" 2>/dev/null
+ test_result "$result" "$tokencount" "$*" "${OPTRET[@]}"
+ unset OPTRET
+}
+
+test_result() {
+ local result=$1 tokencount=$2 input=$3; shift 3
+
+ if { [[ $result = "$*" ]] || [[ $2 = NULL && -z $1 ]]; } && (( tokencount == $# )); then
+ (( ++pass ))
+ else
+ printf '[TEST %3s]: FAIL
' "$testcount"
+ printf ' input: %s
' "$input"
+ printf ' output: %s (%s tokens)
' "$*" "$#"
+ printf ' expected: %s (%s tokens)
' "$result" "$tokencount"
+ echo
+ (( ++fail ))
+ fi
+}
+
+summarize() {
+ if (( !fail )); then
+ printf 'All %s tests successful
' "$testcount"
+ exit 0
+ else
+ printf '%s of %s tests failed
' "$fail" "$testcount"
+ exit 1
+ fi
+}
+trap 'summarize' EXIT
+
+printf 'Beginning parse_options tests
'
+
+# usage: parse <expected result> <token count> test-params...
+# a failed parse will match only the end of options marker '--'
+
+# no options
+parse '--' 1
+
+# short options
+parse '-s -r --' 3 -s -r
+
+# short options, no spaces
+parse '-s -r --' 3 -sr
+
+# short opt missing an opt arg
+parse '--' 1 -s -p
+
+# short opt with an opt arg
+parse '-p PKGBUILD -L --' 4 -p PKGBUILD -L
+
+# short opt with an opt arg, no space
+parse '-p PKGBUILD --' 3 -pPKGBUILD
+
+# valid shortopts as a long opt
+parse '--' 1 --sir
+
+# long opt with missing optarg
+parse '--' 1 -sr --pkg
+
+# long opt with optarg
+parse '--pkg foo --' 3 --pkg foo
+
+# explicit end of options with options after
+parse '-s -r -- foo bar baz' 6 -s -r -- foo bar baz
+
+# non-option parameters mixed in with options
+parse '-s -r -- foo baz' 5 -s foo baz -r
+
+# optarg with whitespace
+parse '-p foo bar -s --' 4 -p'foo bar' -s
+
+# non-option parameter with whitespace
+parse '-i -- foo bar' 3 -i 'foo bar'
--
1.7.6
 
Old 08-17-2011, 10:27 PM
Dan McGee
 
Default scripts/library: rewrite parse_options

On Thu, Jul 21, 2011 at 3:40 PM, Dave Reisner <d@falconindy.com> wrote:
> In addition to being what I feel is a cleaner and faster implementation,
> we avoid the use of eval by normalizing option arguments into a global
> array which is then set after a successful call to parse_options.
>
> This trims out the idea of having multiple arguments to a single option,
> making our parsing algorithm a little more sane. We never took advantage of
> this in makepkg (for the one option that feasibly supports it), and I
> think we've overlooked a much simpler solution in pacman-key. Since
> actions are limited to 1 at a time the leftover positional parameters
> become the keys or keyfiles which are acted upon.
>
> Also added is a new test directory test/scripts with a harness for
> parse_options, run as part of make check.
>
> Signed-off-by: Dave Reisner <dreisner@archlinux.org>
> ---
> Thoughts? I know Allan wasn't quite sold on this, as the downside is that we
> sort of pigeonhole ourselves into using the non-optional parameter as arguments
> to our action. This has zero effect on makepkg.
>
> I've also thought to add --option=arg syntax parsing, but I'm not sure we need
> this.

Allan, was deferring to you on this.

> *Makefile.am * * * * * * * * * * * *| * *6 +-
> *configure.ac * * * * * * * * * * * | * *1 +
> *scripts/library/parse_options.sh * | *195 +++++++++++++++++-------------------
> *scripts/makepkg.sh.in * * * * * * *| * 13 +--
> *scripts/pacman-key.sh.in * * * * * | * 59 ++++++------
> *test/scripts/Makefile.am * * * * * | * *9 ++
> *test/scripts/parse_options_test.sh | *103 +++++++++++++++++++
> *7 files changed, 242 insertions(+), 144 deletions(-)
> *rewrite scripts/library/parse_options.sh (89%)
> *create mode 100644 test/scripts/Makefile.am
> *create mode 100755 test/scripts/parse_options_test.sh
>
> diff --git a/Makefile.am b/Makefile.am
> index 286e6ae..f88b6d1 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -1,4 +1,4 @@
> -SUBDIRS = lib/libalpm src/util src/pacman scripts etc test/pacman test/util contrib
> +SUBDIRS = lib/libalpm src/util src/pacman scripts etc test/pacman test/scripts test/util contrib
> *if WANT_DOC
> *SUBDIRS += doc
> *endif
> @@ -21,12 +21,14 @@ dist_pkgdata_DATA =
> * * * *proto/ChangeLog.proto
>
> *# run the pactest test suite and vercmp tests
> -check-local: test/pacman test/util src/pacman src/util
> +check-local: test/pacman test/scripts test/util src/pacman src/util
> * * * *$(PYTHON) $(top_srcdir)/test/pacman/pactest.py --debug=1
> * * * * * * * *--test $(top_srcdir)/test/pacman/tests/*.py
> * * * * * * * *-p $(top_builddir)/src/pacman/pacman
> * * * *$(SH) $(top_srcdir)/test/util/vercmptest.sh
> * * * * * * * *$(top_builddir)/src/util/vercmp
> + * * * $(BASH_SHELL) $(top_srcdir)/test/scripts/parse_options_test.sh
> + * * * * * * * $(top_builddir)/scripts/library/parse_options.sh
>
> *# create the pacman DB and cache directories upon install
> *install-data-local:
> diff --git a/configure.ac b/configure.ac
> index 6ad5be5..a553176 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -381,6 +381,7 @@ doc/Makefile
> *etc/Makefile
> *test/pacman/Makefile
> *test/pacman/tests/Makefile
> +test/scripts/Makefile
> *test/util/Makefile
> *contrib/Makefile
> *Makefile
> diff --git a/scripts/library/parse_options.sh b/scripts/library/parse_options.sh
> dissimilarity index 89%
> index 48fd42c..215059f 100644
> --- a/scripts/library/parse_options.sh
> +++ b/scripts/library/parse_options.sh
> @@ -1,105 +1,90 @@
> -# getopt like parser
> -parse_options() {
> - * * * local short_options=$1; shift;
> - * * * local long_options=$1; shift;
> - * * * local ret=0;
> - * * * local unused_options=""
> - * * * local i
> -
> - * * * while [[ -n $1 ]]; do
> - * * * * * * * if [[ ${1:0:2} = '--' ]]; then
> - * * * * * * * * * * * if [[ -n ${1:2} ]]; then
> - * * * * * * * * * * * * * * * local match=""
> - * * * * * * * * * * * * * * * for i in ${long_options//,/ }; do
> - * * * * * * * * * * * * * * * * * * * if [[ ${1:2} = ${i//:} ]]; then
> - * * * * * * * * * * * * * * * * * * * * * * * match=$i
> - * * * * * * * * * * * * * * * * * * * * * * * break
> - * * * * * * * * * * * * * * * * * * * fi
> - * * * * * * * * * * * * * * * done
> - * * * * * * * * * * * * * * * if [[ -n $match ]]; then
> - * * * * * * * * * * * * * * * * * * * local needsargument=0
> -
> - * * * * * * * * * * * * * * * * * * * [[ ${match} = ${1:2}: ]] && needsargument=1
> - * * * * * * * * * * * * * * * * * * * [[ ${match} = ${1:2}:: && -n $2 && ${2:0:1} != "-" ]] && needsargument=1
> -
> - * * * * * * * * * * * * * * * * * * * if (( ! needsargument )); then
> - * * * * * * * * * * * * * * * * * * * * * * * printf ' %s' "$1"
> - * * * * * * * * * * * * * * * * * * * else
> - * * * * * * * * * * * * * * * * * * * * * * * if [[ -n $2 ]]; then
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * printf ' %s ' "$1"
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * shift
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * printf "'%q" "$1"
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * while [[ -n $2 && ${2:0:1} != "-" ]]; do
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * shift
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * printf " %q" "$1"
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * done
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * printf "'"
> - * * * * * * * * * * * * * * * * * * * * * * * else
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * printf "@SCRIPTNAME@: $(gettext "option %s requires an argument
")" "'$1'" >&2
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * ret=1
> - * * * * * * * * * * * * * * * * * * * * * * * fi
> - * * * * * * * * * * * * * * * * * * * fi
> - * * * * * * * * * * * * * * * else
> - * * * * * * * * * * * * * * * * * * * echo "@SCRIPTNAME@: $(gettext "unrecognized option") '$1'" >&2
> - * * * * * * * * * * * * * * * * * * * ret=1
> - * * * * * * * * * * * * * * * fi
> - * * * * * * * * * * * else
> - * * * * * * * * * * * * * * * shift
> - * * * * * * * * * * * * * * * break
> - * * * * * * * * * * * fi
> - * * * * * * * elif [[ ${1:0:1} = '-' ]]; then
> - * * * * * * * * * * * for ((i=1; i<${#1}; i++)); do
> - * * * * * * * * * * * * * * * if [[ $short_options =~ ${1:i:1} ]]; then
> - * * * * * * * * * * * * * * * * * * * local needsargument=0
> -
> - * * * * * * * * * * * * * * * * * * * [[ $short_options =~ ${1:i:1}: && ! $short_options =~ ${1:i:1}:: ]] && needsargument=1
> - * * * * * * * * * * * * * * * * * * * [[ $short_options =~ ${1:i:1}:: &&
> - * * * * * * * * * * * * * * * * * * * * * * * ( -n ${1:$i+1} || ( -n $2 && ${2:0:1} != "-" ) ) ]] && needsargument=1
> -
> - * * * * * * * * * * * * * * * * * * * if (( ! needsargument )); then
> - * * * * * * * * * * * * * * * * * * * * * * * printf ' -%s' "${1:i:1}"
> - * * * * * * * * * * * * * * * * * * * else
> - * * * * * * * * * * * * * * * * * * * * * * * if [[ -n ${1:$i+1} ]]; then
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * printf ' -%s ' "${1:i:1}"
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * printf "'%q" "${1:$i+1}"
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * while [[ -n $2 && ${2:0:1} != "-" ]]; do
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * shift
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * printf " %q" "$1"
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * done
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * printf "'"
> - * * * * * * * * * * * * * * * * * * * * * * * else
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * if [[ -n $2 ]]; then
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * printf ' -%s ' "${1:i:1}"
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * shift
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * printf "'%q" "$1"
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * while [[ -n $2 && ${2:0:1} != "-" ]]; do
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * shift
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * printf " %q" "$1"
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * done
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * printf "'"
> -
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * else
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * printf "@SCRIPTNAME@: $(gettext "option %s requires an argument
")" "'-${1:i:1}'" >&2
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * ret=1
> - * * * * * * * * * * * * * * * * * * * * * * * * * * * fi
> - * * * * * * * * * * * * * * * * * * * * * * * fi
> - * * * * * * * * * * * * * * * * * * * * * * * break
> - * * * * * * * * * * * * * * * * * * * fi
> - * * * * * * * * * * * * * * * else
> - * * * * * * * * * * * * * * * * * * * echo "@SCRIPTNAME@: $(gettext "unrecognized option") '-${1:i:1}'" >&2
> - * * * * * * * * * * * * * * * * * * * ret=1
> - * * * * * * * * * * * * * * * fi
> - * * * * * * * * * * * done
> - * * * * * * * else
> - * * * * * * * * * * * unused_options="${unused_options} '$1'"
> - * * * * * * * fi
> - * * * * * * * shift
> - * * * done
> -
> - * * * printf " --"
> - * * * [[ $unused_options ]] && printf ' %s' "${unused_options[@]}"
> - * * * [[ $1 ]] && printf " '%s'" "$@"
> - * * * printf "
"
> -
> - * * * return $ret
> -} No newline at end of file
> +# getopt-like parser
> +parse_options () {
> + * * * local opt= i= shortopts=$1
> + * * * local -a longopts param
> +
> + * * * # split the longopt array on commas
> + * * * IFS=, read -r -a longopts <<< "$2"
> + * * * shift 2
> +
> + * * * while (( $# )); do
> + * * * * * * * case $1 in
> + * * * * * * * * * * * --) # explicit end of options
> + * * * * * * * * * * * * * * * shift
> + * * * * * * * * * * * * * * * break
> + * * * * * * * * * * * * * * * ;;
> + * * * * * * * * * * * -[!-]*) # short option
> + * * * * * * * * * * * * * * * for (( i = 1; i < ${#1}; i++ )); do
> + * * * * * * * * * * * * * * * * * * * opt=${1:i:1}
> +
> + * * * * * * * * * * * * * * * * * * * # option doesn't exist
> + * * * * * * * * * * * * * * * * * * * if [[ $shortopts != *$opt* ]]; then
> + * * * * * * * * * * * * * * * * * * * * * * * echo "@SCRIPTNAME@: $(gettext "unrecognized option") '-$opt'" >&2
> + * * * * * * * * * * * * * * * * * * * * * * * OPTRET=(--)
> + * * * * * * * * * * * * * * * * * * * * * * * return 1
> + * * * * * * * * * * * * * * * * * * * fi
> +
> + * * * * * * * * * * * * * * * * * * * OPTRET+=("-$opt")
> + * * * * * * * * * * * * * * * * * * * # option requires optarg
> + * * * * * * * * * * * * * * * * * * * if [[ $shortopts = *$opt:* ]]; then
> + * * * * * * * * * * * * * * * * * * * * * * * # if we're not at the end of the option chunk, the rest is the optarg
> + * * * * * * * * * * * * * * * * * * * * * * * if (( i < ${#1} - 1 )); then
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * OPTRET+=("${1:i+1}")
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * break
> +
> + * * * * * * * * * * * * * * * * * * * * * * * # if we're at the end, grab the the next positional, if it exists
> + * * * * * * * * * * * * * * * * * * * * * * * elif (( i == ${#1} - 1 )) && [[ $2 ]]; then
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * OPTRET+=("$2")
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * shift
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * break
> +
> + * * * * * * * * * * * * * * * * * * * * * * * # parse failure
> + * * * * * * * * * * * * * * * * * * * * * * * else
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * printf "@SCRIPTNAME@: $(gettext "option %s requires an argument
")" "'-$opt'" >&2
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * OPTRET=(--)
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * return 1
> + * * * * * * * * * * * * * * * * * * * * * * * fi
> + * * * * * * * * * * * * * * * * * * * fi
> +
> + * * * * * * * * * * * * * * * done
> + * * * * * * * * * * * * * * * ;;
> + * * * * * * * * * * * --?*) # long option
> + * * * * * * * * * * * * * * * opt=${1#--}
> + * * * * * * * * * * * * * * * OPTRET+=("$1")
> +
> + * * * * * * * * * * * * * * * for longopt in "${longopts[@]}"; do
> + * * * * * * * * * * * * * * * * * * * if [[ $longopt =~ ^($opt)(:?)$ ]]; then
> + * * * * * * * * * * * * * * * * * * * * * * * # option requires optarg
> + * * * * * * * * * * * * * * * * * * * * * * * if [[ -n ${BASH_REMATCH[2]} ]]; then
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * if [[ -z $2 ]]; then
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * printf "@SCRIPTNAME@: $(gettext "option %s requires an argument
")" "'--$opt'" >&2
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * OPTRET=(--)
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * return 1
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * fi
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * OPTRET+=("$2")
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * shift 2
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * continue 2
> +
> + * * * * * * * * * * * * * * * * * * * * * * * # simple longopt
> + * * * * * * * * * * * * * * * * * * * * * * * elif [[ -n ${BASH_REMATCH[1]} ]]; then
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * shift
> + * * * * * * * * * * * * * * * * * * * * * * * * * * * continue 2
> + * * * * * * * * * * * * * * * * * * * * * * * fi
> + * * * * * * * * * * * * * * * * * * * fi
> + * * * * * * * * * * * * * * * done
> + * * * * * * * * * * * * * * * echo "@SCRIPTNAME@: $(gettext "unrecognized option") '--$opt'" >&2
> + * * * * * * * * * * * * * * * OPTRET=(--)
> + * * * * * * * * * * * * * * * return 1
> + * * * * * * * * * * * * * * * ;;
> + * * * * * * * * * * * *) # non-option arg encountered, add it as a parameter
> + * * * * * * * * * * * * * * * param+=("$1")
> + * * * * * * * * * * * * * * * ;;
> + * * * * * * * esac
> + * * * * * * * shift
> + * * * done
> +
> + * * * # add end-of-opt terminator and any leftover positional parameters
> + * * * OPTRET+=('--' "${param[@]}" "$@")
> +
> + * * * return 0
> +}
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index a4e7156..b6ab2ee 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -1755,14 +1755,14 @@ OPT_LONG+=",install,key:,log,nocolor,nobuild,noche ck,nosign,pkg:,rmdeps"
> *OPT_LONG+=",repackage,skipinteg,sign,source,syncd eps,version,config:"
> *# Pacman Options
> *OPT_LONG+=",noconfirm,noprogressbar"
> -if ! OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@")"; then
> +if ! parse_options "$OPT_SHORT" "$OPT_LONG" "$@"; then
> * * * *echo; usage; exit 1 # E_INVALID_OPTION;
> *fi
> -eval set -- "$OPT_TEMP"
> -unset OPT_SHORT OPT_LONG OPT_TEMP
> +set -- "${OPTRET[@]}"
> +unset OPT_SHORT OPT_LONG OPTRET
>
> -while true; do
> - * * * case "$1" in
> +while (( $# )); do
> + * * * case $1 in
> * * * * * * * *# Pacman Options
> * * * * * * * *--noconfirm) * * *PACMAN_OPTS+=" --noconfirm" ;;
> * * * * * * * *--noprogressbar) *PACMAN_OPTS+=" --noprogressbar" ;;
> @@ -1801,8 +1801,7 @@ while true; do
> * * * * * * * *-h|--help) * * * *usage; exit 0 ;; # E_OK
> * * * * * * * *-V|--version) * * version; exit 0 ;; # E_OK
>
> - * * * * * * * --) * * * * * * * OPT_IND=0; shift; break;;
> - * * * * * * * *) * * * * * * * *usage; exit 1 ;; # E_INVALID_OPTION
> + * * * * * * * --) * * * * * * * shift; break;;
> * * * *esac
> * * * *shift
> *done
> diff --git a/scripts/pacman-key.sh.in b/scripts/pacman-key.sh.in
> index 5cb5bd5..d0fb0ef 100644
> --- a/scripts/pacman-key.sh.in
> +++ b/scripts/pacman-key.sh.in
> @@ -255,16 +255,16 @@ reload_keyring() {
> *}
>
> *receive_keys() {
> - * * * if [[ -z ${KEYIDS[@]} ]]; then
> + * * * if [[ -z $1 ]]; then
> * * * * * * * *error "$(gettext "You need to specify the keyserver and at least one key identifier")"
> * * * * * * * *exit 1
> * * * *fi
> - * * * ${GPG_PACMAN} --keyserver "$KEYSERVER" --recv-keys "${KEYIDS[@]}"
> + * * * ${GPG_PACMAN} --keyserver "$KEYSERVER" --recv-keys "$@"
> *}
>
> *edit_keys() {
> * * * *local errors=0;
> - * * * for key in ${KEYIDS[@]}; do
> + * * * for key; do
> * * * * * * * *# Verify if the key exists in pacman's keyring
> * * * * * * * *if ! ${GPG_PACMAN} --list-keys "$key" &>/dev/null; then
> * * * * * * * * * * * *error "$(gettext "The key identified by %s does not exist")" "$key"
> @@ -273,7 +273,7 @@ edit_keys() {
> * * * *done
> * * * *(( errors )) && exit 1;
>
> - * * * for key in ${KEYIDS[@]}; do
> + * * * for key; do
> * * * * * * * *${GPG_PACMAN} --edit-key "$key"
> * * * *done
> *}
> @@ -285,32 +285,33 @@ if ! type gettext &>/dev/null; then
> * * * *}
> *fi
>
> -OPT_SHORT="a::d:e:f::hlr:uv:V"
> -OPT_LONG="add::,config:,delete:,edit-key:,export::,finger::,gpgdir:"
> +declare -a OPTRET
> +OPT_SHORT="ade:fhlr:uv:V"
> +OPT_LONG="add,config:,delete,edit-key,export,finger,gpgdir:"
> *OPT_LONG+=",help,init,list,receive:,reload,update db,verify:,version"
> -if ! OPT_TEMP="$(parse_options $OPT_SHORT $OPT_LONG "$@")"; then
> +if ! parse_options "$OPT_SHORT" "$OPT_LONG" "$@"; then
> * * * *echo; usage; exit 1 # E_INVALID_OPTION;
> *fi
> -eval set -- "$OPT_TEMP"
> -unset OPT_SHORT OPT_LONG OPT_TEMP
> +set -- "${OPTRET[@]}"
> +unset OPT_SHORT OPT_LONG OPTRET
>
> -if [[ $1 == "--" ]]; then
> - * * * usage;
> - * * * exit 0;
> +if [[ -z $1 ]]; then
> + * * * usage
> + * * * exit 0
> *fi
>
> -while true; do
> - * * * case "$1" in
> - * * * * * * * -a|--add) * * * * ADD=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYFILES=($1) ;;
> +while (( $# )); do
> + * * * case $1 in
> + * * * * * * * -a|--add) * * * * ADD=1 ;;
> * * * * * * * *--config) * * * * shift; CONFIG=$1 ;;
> - * * * * * * * -d|--delete) * * *DELETE=1; shift; KEYIDS=($1) ;;
> - * * * * * * * --edit-key) * * * EDITKEY=1; shift; KEYIDS=($1) ;;
> - * * * * * * * -e|--export) * * *EXPORT=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;;
> - * * * * * * * -f|--finger) * * *FINGER=1; [[ -n $2 && ${2:0:1} != "-" ]] && shift && KEYIDS=($1) ;;
> + * * * * * * * -d|--delete) * * *DELETE=1 ;;
> + * * * * * * * --edit-key) * * * EDITKEY=1 ;;
> + * * * * * * * -e|--export) * * *EXPORT=1 ;;
> + * * * * * * * -f|--finger) * * *FINGER=1 ;;
> * * * * * * * *--gpgdir) * * * * shift; PACMAN_KEYRING_DIR=$1 ;;
> * * * * * * * *--init) * * * * * INIT=1 ;;
> * * * * * * * *-l|--list) * * * *LIST=1 ;;
> - * * * * * * * -r|--receive) * * RECEIVE=1; shift; KEYSERVER=$1; KEYIDS=("${@:1}") ;;
> + * * * * * * * -r|--receive) * * RECEIVE=1; shift; KEYSERVER=$1 ;;
> * * * * * * * *--reload) * * * * RELOAD=1 ;;
> * * * * * * * *-u|--updatedb) * *UPDATEDB=1 ;;
> * * * * * * * *-v|--verify) * * *VERIFY=1; shift; SIGNATURE=$1 ;;
> @@ -318,13 +319,11 @@ while true; do
> * * * * * * * *-h|--help) * * * *usage; exit 0 ;;
> * * * * * * * *-V|--version) * * version; exit 0 ;;
>
> - * * * * * * * --) * * * * * * * OPT_IND=0; shift; break;;
> - * * * * * * * *) * * * * * * * *usage; exit 1 ;;
> + * * * * * * * --) * * * * * * * shift; break ;;
> * * * *esac
> * * * *shift
> *done
>
> -
> *if ! type -p gpg >/dev/null; then
> * * error "$(gettext "Cannot find the %s binary required for all %s operations.")" "gpg" "pacman-key"
> * * * *exit 1
> @@ -364,14 +363,14 @@ esac
>
> *(( ! INIT )) && check_keyring
>
> -(( ADD )) && ${GPG_PACMAN} --quiet --batch --import "${KEYFILES[@]}"
> -(( DELETE )) && ${GPG_PACMAN} --quiet --batch --delete-key --yes "${KEYIDS[@]}"
> -(( EDITKEY )) && edit_keys
> -(( EXPORT )) && ${GPG_PACMAN} --armor --export "${KEYIDS[@]}"
> -(( FINGER )) && ${GPG_PACMAN} --batch --fingerprint "${KEYIDS[@]}"
> +(( ADD )) && ${GPG_PACMAN} --quiet --batch --import "$@"
> +(( DELETE )) && ${GPG_PACMAN} --quiet --batch --delete-key --yes "$@"
> +(( EDITKEY )) && edit_keys "$@"
> +(( EXPORT )) && ${GPG_PACMAN} --armor --export "$@"
> +(( FINGER )) && ${GPG_PACMAN} --batch --fingerprint "$@"
> *(( INIT )) && initialize
> -(( LIST )) && ${GPG_PACMAN} --batch --list-sigs "${KEYIDS[@]}"
> -(( RECEIVE )) && receive_keys
> +(( LIST )) && ${GPG_PACMAN} --batch --list-sigs "$@"
> +(( RECEIVE )) && receive_keys "$@"
> *(( RELOAD )) && reload_keyring
> *(( UPDATEDB )) && ${GPG_PACMAN} --batch --check-trustdb
> *(( VERIFY )) && ${GPG_PACMAN} --verify $SIGNATURE
> diff --git a/test/scripts/Makefile.am b/test/scripts/Makefile.am
> new file mode 100644
> index 0000000..caa9ab1
> --- /dev/null
> +++ b/test/scripts/Makefile.am
> @@ -0,0 +1,9 @@
> +check_SCRIPTS =
> + * * * parse_options_test.sh
> +
> +noinst_SCRIPTS = $(check_SCRIPTS)
> +
> +EXTRA_DIST =
> + * * * $(check_SCRIPTS)
> +
> +# vim:set ts=2 sw=2 noet:
> diff --git a/test/scripts/parse_options_test.sh b/test/scripts/parse_options_test.sh
> new file mode 100755
> index 0000000..85981e1
> --- /dev/null
> +++ b/test/scripts/parse_options_test.sh
> @@ -0,0 +1,103 @@
> +#!/bin/bash
> +
> +declare -i testcount=0 pass=0 fail=0
> +
> +# source the library function
> +if [[ -z $1 || ! -f $1 ]]; then
> + *printf "error: path to parse_option library not provided or does not exist
"
> + *exit 1
> +fi
> +. "$1"
> +
> +if ! type -t parse_options >/dev/null; then
> + *printf 'parse_options function not found
'
> + *exit 1
> +fi
> +
> +# borrow opts from makepkg
> +OPT_SHORT="AcdefFghiLmop:rRsV"
> +OPT_LONG="allsource,asroot,ignorearch,check,clean ,nodeps"
> +OPT_LONG+=",noextract,force,forcever:,geninteg,he lp,holdver"
> +OPT_LONG+=",install,key:,log,nocolor,nobuild,noch eck,nosign,pkg:,rmdeps"
> +OPT_LONG+=",repackage,skipinteg,sign,source,syncd eps,version,config:"
> +OPT_LONG+=",noconfirm,noprogressbar"
> +
> +parse() {
> + *local result=$1 tokencount=$2; shift 2
> +
> + *(( ++testcount ))
> + *#printf '[TEST %2s]: ' "$testcount"
> + *parse_options "$OPT_SHORT" "$OPT_LONG" "$@" 2>/dev/null
> + *test_result "$result" "$tokencount" "$*" "${OPTRET[@]}"
> + *unset OPTRET
> +}
> +
> +test_result() {
> + *local result=$1 tokencount=$2 input=$3; shift 3
> +
> + *if { [[ $result = "$*" ]] || [[ $2 = NULL && -z $1 ]]; } && (( tokencount == $# )); then
> + * *(( ++pass ))
> + *else
> + * *printf '[TEST %3s]: FAIL
' "$testcount"
> + * *printf ' * * *input: %s
' "$input"
> + * *printf ' * * output: %s (%s tokens)
' "$*" "$#"
> + * *printf ' * expected: %s (%s tokens)
' "$result" "$tokencount"
> + * *echo
> + * *(( ++fail ))
> + *fi
> +}
> +
> +summarize() {
> + *if (( !fail )); then
> + * *printf 'All %s tests successful
' "$testcount"
> + * *exit 0
> + *else
> + * *printf '%s of %s tests failed
' "$fail" "$testcount"
> + * *exit 1
> + *fi
> +}
> +trap 'summarize' EXIT
> +
> +printf 'Beginning parse_options tests
'
> +
> +# usage: parse <expected result> <token count> test-params...
> +# a failed parse will match only the end of options marker '--'
> +
> +# no options
> +parse '--' 1
> +
> +# short options
> +parse '-s -r --' 3 -s -r
> +
> +# short options, no spaces
> +parse '-s -r --' 3 -sr
> +
> +# short opt missing an opt arg
> +parse '--' 1 -s -p
> +
> +# short opt with an opt arg
> +parse '-p PKGBUILD -L --' 4 -p PKGBUILD -L
> +
> +# short opt with an opt arg, no space
> +parse '-p PKGBUILD --' 3 -pPKGBUILD
> +
> +# valid shortopts as a long opt
> +parse '--' 1 --sir
> +
> +# long opt with missing optarg
> +parse '--' 1 -sr --pkg
> +
> +# long opt with optarg
> +parse '--pkg foo --' 3 --pkg foo
> +
> +# explicit end of options with options after
> +parse '-s -r -- foo bar baz' 6 -s -r -- foo bar baz
> +
> +# non-option parameters mixed in with options
> +parse '-s -r -- foo baz' 5 -s foo baz -r
> +
> +# optarg with whitespace
> +parse '-p foo bar -s --' 4 -p'foo bar' -s
> +
> +# non-option parameter with whitespace
> +parse '-i -- foo bar' 3 -i 'foo bar'
> --
> 1.7.6
>
>
>
 
Old 08-17-2011, 10:57 PM
Allan McRae
 
Default scripts/library: rewrite parse_options

On 18/08/11 08:27, Dan McGee wrote:

On Thu, Jul 21, 2011 at 3:40 PM, Dave Reisner<d@falconindy.com> wrote:

In addition to being what I feel is a cleaner and faster implementation,
we avoid the use of eval by normalizing option arguments into a global
array which is then set after a successful call to parse_options.

This trims out the idea of having multiple arguments to a single option,
making our parsing algorithm a little more sane. We never took advantage of
this in makepkg (for the one option that feasibly supports it), and I
think we've overlooked a much simpler solution in pacman-key. Since
actions are limited to 1 at a time the leftover positional parameters
become the keys or keyfiles which are acted upon.

Also added is a new test directory test/scripts with a harness for
parse_options, run as part of make check.

Signed-off-by: Dave Reisner<dreisner@archlinux.org>
---
Thoughts? I know Allan wasn't quite sold on this, as the downside is that we
sort of pigeonhole ourselves into using the non-optional parameter as arguments
to our action. This has zero effect on makepkg.

I've also thought to add --option=arg syntax parsing, but I'm not sure we need
this.


Allan, was deferring to you on this.



Well, I was deferring to you as Dave is right that I was never sold on
this...


Unless I am missing something, this does have a minor effect on makepkg.
In git "makepkg --pkg foo bar" builds only foo and bar from a split
package. I guess with this patch we would have to quote the package
names in some way (like is needed on the current maint release). So we
would need to revert changes made to the makepkg documentation when the
multiple arguments stuff was added.


But my main issue is that we lose the ability to specify multiple
arguments with optional parameters. We do not use this anywhere at the
moment, so it is not a big loss but that is not to say we will never use
it in the future.


Basically, I am not sure what the actual advantage of rewriting this is...

Allan
 
Old 08-17-2011, 11:34 PM
Dave Reisner
 
Default scripts/library: rewrite parse_options

On Thu, Aug 18, 2011 at 08:57:55AM +1000, Allan McRae wrote:
> On 18/08/11 08:27, Dan McGee wrote:
> >On Thu, Jul 21, 2011 at 3:40 PM, Dave Reisner<d@falconindy.com> wrote:
> >>In addition to being what I feel is a cleaner and faster implementation,
> >>we avoid the use of eval by normalizing option arguments into a global
> >>array which is then set after a successful call to parse_options.
> >>
> >>This trims out the idea of having multiple arguments to a single option,
> >>making our parsing algorithm a little more sane. We never took advantage of
> >>this in makepkg (for the one option that feasibly supports it), and I
> >>think we've overlooked a much simpler solution in pacman-key. Since
> >>actions are limited to 1 at a time the leftover positional parameters
> >>become the keys or keyfiles which are acted upon.
> >>
> >>Also added is a new test directory test/scripts with a harness for
> >>parse_options, run as part of make check.
> >>
> >>Signed-off-by: Dave Reisner<dreisner@archlinux.org>
> >>---
> >>Thoughts? I know Allan wasn't quite sold on this, as the downside is that we
> >>sort of pigeonhole ourselves into using the non-optional parameter as arguments
> >>to our action. This has zero effect on makepkg.
> >>
> >>I've also thought to add --option=arg syntax parsing, but I'm not sure we need
> >>this.
> >
> >Allan, was deferring to you on this.
>
>
> Well, I was deferring to you as Dave is right that I was never sold
> on this...
>
> Unless I am missing something, this does have a minor effect on
> makepkg. In git "makepkg --pkg foo bar" builds only foo and bar
> from a split package.

And, imo, this introduces bizzare unexpected behavior. With the
"standard" getopt{,_long}, passing something such as:

--pkg foo bar

I'd expect bar to be completely unrelated to the flag. Not the case
here, and this behavior isn't really documented clearly at all. We don't
even properly handle arguments with whitespace anymore. Not such a big
deal for makepkg, but I think it's reasonable that pacman-key might some
day need to import a key from a file with a space in the name.

> I guess with this patch we would have to quote the package names in
> some way (like is needed on the current maint release). So we would
> need to revert changes made to the makepkg documentation when the
> multiple arguments stuff was added.

We can (should?) follow pacman here and separate multiple arguments by
commas. I'm going to channel Dan and call out "consistency" here.

> But my main issue is that we lose the ability to specify multiple
> arguments with optional parameters. We do not use this anywhere at
> the moment, so it is not a big loss but that is not to say we will
> never use it in the future.

My problem with this is that you're then forced, in a calling
application, to change the behavior of your parsing of normalized
options. It's a little strange.

> Basically, I am not sure what the actual advantage of rewriting this is...

Faster, cleaner, safer (no eval!!), and more standard. That's what I'm
selling here.

d
 
Old 08-18-2011, 12:22 AM
Dan McGee
 
Default scripts/library: rewrite parse_options

On Wed, Aug 17, 2011 at 7:22 PM, Allan McRae <allan@archlinux.org> wrote:
> On 18/08/11 09:34, Dave Reisner wrote:
>>
>> On Thu, Aug 18, 2011 at 08:57:55AM +1000, Allan McRae wrote:
>>>
>>> On 18/08/11 08:27, Dan McGee wrote:
>>>>
>>>> On Thu, Jul 21, 2011 at 3:40 PM, Dave Reisner<d@falconindy.com> * wrote:
>>>>>
>>>>> In addition to being what I feel is a cleaner and faster
>>>>> implementation,
>>>>> we avoid the use of eval by normalizing option arguments into a global
>>>>> array which is then set after a successful call to parse_options.
>>>>>
>>>>> This trims out the idea of having multiple arguments to a single
>>>>> option,
>>>>> making our parsing algorithm a little more sane. We never took
>>>>> advantage of
>>>>> this in makepkg (for the one option that feasibly supports it), and I
>>>>> think we've overlooked a much simpler solution in pacman-key. Since
>>>>> actions are limited to 1 at a time the leftover positional parameters
>>>>> become the keys or keyfiles which are acted upon.
>>>>>
>>>>> Also added is a new test directory test/scripts with a harness for
>>>>> parse_options, run as part of make check.
>>>>>
>>>>> Signed-off-by: Dave Reisner<dreisner@archlinux.org>
>>>>> ---
>>>>> Thoughts? I know Allan wasn't quite sold on this, as the downside is
>>>>> that we
>>>>> sort of pigeonhole ourselves into using the non-optional parameter as
>>>>> arguments
>>>>> to our action. This has zero effect on makepkg.
>>>>>
>>>>> I've also thought to add --option=arg syntax parsing, but I'm not sure
>>>>> we need
>>>>> this.
>>>>
>>>> Allan, was deferring to you on this.
>>>
>>>
>>> Well, I was deferring to you as Dave is right that I was never sold
>>> on this...
>>>
>>> Unless I am missing something, this does have a minor effect on
>>> makepkg. *In git "makepkg --pkg foo bar" builds only foo and bar
>>> from a split package.
>>
>> And, imo, this introduces bizzare unexpected behavior. With the
>> "standard" getopt{,_long}, passing something such as:
>>
>> * --pkg foo bar
>>
>> I'd expect bar to be completely unrelated to the flag. Not the case
>> here, and this behavior isn't really documented clearly at all. We don't
>> even properly handle arguments with whitespace anymore. Not such a big
>> deal for makepkg, but I think it's reasonable that pacman-key might some
>> day need to import a key from a file with a space in the name.
>
> I thought the whitespace issue was fixed but testing now I see it is not.
> *That is definitely something that needs addressed.
>
>>> I guess with this patch we would have to quote the package names in
>>> some way (like is needed on the current maint release). *So we would
>>> need to revert changes made to the makepkg documentation when the
>>> multiple arguments stuff was added.
>>
>> We can (should?) follow pacman here and separate multiple arguments by
>> commas. I'm going to channel Dan and call out "consistency" here.
>
> You mean like "pacman -S pkg1,pkg2"? * Fairly sure we do not do that! So I
> call consistency... *Where do we use commas in pacman options?

pacman -Syu --ignorepkg foo,bar.baz
 
Old 08-18-2011, 12:22 AM
Allan McRae
 
Default scripts/library: rewrite parse_options

On 18/08/11 09:34, Dave Reisner wrote:

On Thu, Aug 18, 2011 at 08:57:55AM +1000, Allan McRae wrote:

On 18/08/11 08:27, Dan McGee wrote:

On Thu, Jul 21, 2011 at 3:40 PM, Dave Reisner<d@falconindy.com> wrote:

In addition to being what I feel is a cleaner and faster implementation,
we avoid the use of eval by normalizing option arguments into a global
array which is then set after a successful call to parse_options.

This trims out the idea of having multiple arguments to a single option,
making our parsing algorithm a little more sane. We never took advantage of
this in makepkg (for the one option that feasibly supports it), and I
think we've overlooked a much simpler solution in pacman-key. Since
actions are limited to 1 at a time the leftover positional parameters
become the keys or keyfiles which are acted upon.

Also added is a new test directory test/scripts with a harness for
parse_options, run as part of make check.

Signed-off-by: Dave Reisner<dreisner@archlinux.org>
---
Thoughts? I know Allan wasn't quite sold on this, as the downside is that we
sort of pigeonhole ourselves into using the non-optional parameter as arguments
to our action. This has zero effect on makepkg.

I've also thought to add --option=arg syntax parsing, but I'm not sure we need
this.


Allan, was deferring to you on this.



Well, I was deferring to you as Dave is right that I was never sold
on this...

Unless I am missing something, this does have a minor effect on
makepkg. In git "makepkg --pkg foo bar" builds only foo and bar
from a split package.


And, imo, this introduces bizzare unexpected behavior. With the
"standard" getopt{,_long}, passing something such as:

--pkg foo bar

I'd expect bar to be completely unrelated to the flag. Not the case
here, and this behavior isn't really documented clearly at all. We don't
even properly handle arguments with whitespace anymore. Not such a big
deal for makepkg, but I think it's reasonable that pacman-key might some
day need to import a key from a file with a space in the name.


I thought the whitespace issue was fixed but testing now I see it is
not. That is definitely something that needs addressed.



I guess with this patch we would have to quote the package names in
some way (like is needed on the current maint release). So we would
need to revert changes made to the makepkg documentation when the
multiple arguments stuff was added.


We can (should?) follow pacman here and separate multiple arguments by
commas. I'm going to channel Dan and call out "consistency" here.


You mean like "pacman -S pkg1,pkg2"? Fairly sure we do not do that!
So I call consistency... Where do we use commas in pacman options?


And it is going to be just the --pkg option in makepkg that requires
such quoting and commas as far as I can tell. All pacman-key options
will still take a non-comma separated list.


Allan
 
Old 08-18-2011, 12:26 AM
Dave Reisner
 
Default scripts/library: rewrite parse_options

On Thu, Aug 18, 2011 at 10:22:18AM +1000, Allan McRae wrote:
> On 18/08/11 09:34, Dave Reisner wrote:
> >On Thu, Aug 18, 2011 at 08:57:55AM +1000, Allan McRae wrote:
> >>On 18/08/11 08:27, Dan McGee wrote:
> >>>On Thu, Jul 21, 2011 at 3:40 PM, Dave Reisner<d@falconindy.com> wrote:
> >>>>In addition to being what I feel is a cleaner and faster implementation,
> >>>>we avoid the use of eval by normalizing option arguments into a global
> >>>>array which is then set after a successful call to parse_options.
> >>>>
> >>>>This trims out the idea of having multiple arguments to a single option,
> >>>>making our parsing algorithm a little more sane. We never took advantage of
> >>>>this in makepkg (for the one option that feasibly supports it), and I
> >>>>think we've overlooked a much simpler solution in pacman-key. Since
> >>>>actions are limited to 1 at a time the leftover positional parameters
> >>>>become the keys or keyfiles which are acted upon.
> >>>>
> >>>>Also added is a new test directory test/scripts with a harness for
> >>>>parse_options, run as part of make check.
> >>>>
> >>>>Signed-off-by: Dave Reisner<dreisner@archlinux.org>
> >>>>---
> >>>>Thoughts? I know Allan wasn't quite sold on this, as the downside is that we
> >>>>sort of pigeonhole ourselves into using the non-optional parameter as arguments
> >>>>to our action. This has zero effect on makepkg.
> >>>>
> >>>>I've also thought to add --option=arg syntax parsing, but I'm not sure we need
> >>>>this.
> >>>
> >>>Allan, was deferring to you on this.
> >>
> >>
> >>Well, I was deferring to you as Dave is right that I was never sold
> >>on this...
> >>
> >>Unless I am missing something, this does have a minor effect on
> >>makepkg. In git "makepkg --pkg foo bar" builds only foo and bar
> >>from a split package.
> >
> >And, imo, this introduces bizzare unexpected behavior. With the
> >"standard" getopt{,_long}, passing something such as:
> >
> > --pkg foo bar
> >
> >I'd expect bar to be completely unrelated to the flag. Not the case
> >here, and this behavior isn't really documented clearly at all. We don't
> >even properly handle arguments with whitespace anymore. Not such a big
> >deal for makepkg, but I think it's reasonable that pacman-key might some
> >day need to import a key from a file with a space in the name.
>
> I thought the whitespace issue was fixed but testing now I see it is
> not. That is definitely something that needs addressed.
>
> >>I guess with this patch we would have to quote the package names in
> >>some way (like is needed on the current maint release). So we would
> >>need to revert changes made to the makepkg documentation when the
> >>multiple arguments stuff was added.
> >
> >We can (should?) follow pacman here and separate multiple arguments by
> >commas. I'm going to channel Dan and call out "consistency" here.
>
> You mean like "pacman -S pkg1,pkg2"? Fairly sure we do not do
> that! So I call consistency... Where do we use commas in pacman
> options?

No I mean like pacman -Syu --ignore pkg1,pkg2

But, that's a good point about the "flexibility" of the non-option
parameters. Just like I'm proposing for pacman-key, we generalize the
term for pacman and call them "targets". They could be a repo, a
file, a sync/local package... They don't belong to any particular
option, but they're acted upon by the singular specified action.

d
 
Old 08-18-2011, 02:08 AM
Allan McRae
 
Default scripts/library: rewrite parse_options

On 18/08/11 10:26, Dave Reisner wrote:

On Thu, Aug 18, 2011 at 10:22:18AM +1000, Allan McRae wrote:

On 18/08/11 09:34, Dave Reisner wrote:

On Thu, Aug 18, 2011 at 08:57:55AM +1000, Allan McRae wrote:

On 18/08/11 08:27, Dan McGee wrote:

On Thu, Jul 21, 2011 at 3:40 PM, Dave Reisner<d@falconindy.com> wrote:

In addition to being what I feel is a cleaner and faster implementation,
we avoid the use of eval by normalizing option arguments into a global
array which is then set after a successful call to parse_options.

This trims out the idea of having multiple arguments to a single option,
making our parsing algorithm a little more sane. We never took advantage of
this in makepkg (for the one option that feasibly supports it), and I
think we've overlooked a much simpler solution in pacman-key. Since
actions are limited to 1 at a time the leftover positional parameters
become the keys or keyfiles which are acted upon.

Also added is a new test directory test/scripts with a harness for
parse_options, run as part of make check.

Signed-off-by: Dave Reisner<dreisner@archlinux.org>
---
Thoughts? I know Allan wasn't quite sold on this, as the downside is that we
sort of pigeonhole ourselves into using the non-optional parameter as arguments
to our action. This has zero effect on makepkg.

I've also thought to add --option=arg syntax parsing, but I'm not sure we need
this.


Allan, was deferring to you on this.



Well, I was deferring to you as Dave is right that I was never sold
on this...

Unless I am missing something, this does have a minor effect on
makepkg. In git "makepkg --pkg foo bar" builds only foo and bar

>from a split package.

And, imo, this introduces bizzare unexpected behavior. With the
"standard" getopt{,_long}, passing something such as:

--pkg foo bar

I'd expect bar to be completely unrelated to the flag. Not the case
here, and this behavior isn't really documented clearly at all. We don't
even properly handle arguments with whitespace anymore. Not such a big
deal for makepkg, but I think it's reasonable that pacman-key might some
day need to import a key from a file with a space in the name.


I thought the whitespace issue was fixed but testing now I see it is
not. That is definitely something that needs addressed.


I guess with this patch we would have to quote the package names in
some way (like is needed on the current maint release). So we would
need to revert changes made to the makepkg documentation when the
multiple arguments stuff was added.


We can (should?) follow pacman here and separate multiple arguments by
commas. I'm going to channel Dan and call out "consistency" here.


You mean like "pacman -S pkg1,pkg2"? Fairly sure we do not do
that! So I call consistency... Where do we use commas in pacman
options?


No I mean like pacman -Syu --ignore pkg1,pkg2

But, that's a good point about the "flexibility" of the non-option
parameters. Just like I'm proposing for pacman-key, we generalize the
term for pacman and call them "targets". They could be a repo, a
file, a sync/local package... They don't belong to any particular
option, but they're acted upon by the singular specified action.



I'll just add a final note that we are losing the ability to ever do
anything like "pacman-key --add key1 key2 --delete key3" (at least with
that syntax). You might say that is a good thing, but there is a long
standing bug report for universal transactions in pacman (the original
4.0 target!) allowing us to do the analogous "pacman -S pkg1 pkg2 -R pkg3".


I'm happy taking away the ability to do one under the argument of
consistency as long as the other is taken off the table too.


Allan
 

Thread Tools




All times are GMT. The time now is 09:35 AM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org