On 25/10/19 11:48 am, Ethan Sommer wrote:

Previously parseopts checked if there was an argument by checking that the string was non-empty, resulting in empty arguments being incorrectly considered non-existent. This change makes parseopts check if arguments exist at all, rather than checking that they are non-empty

Signed-off-by: Ethan Sommer

When parsing options, is there any case where an empty argument is functionally different from a missing one? I.e. what is the justification for considering empty arguments?

--- scripts/libmakepkg/util/parseopts.sh.in | 8 ++++---- test/scripts/parseopts_test.sh | 5 ++++- 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/scripts/libmakepkg/util/parseopts.sh.in b/scripts/libmakepkg/util/parseopts.sh.in index c056cb1e..8fb862d8 100644 --- a/scripts/libmakepkg/util/parseopts.sh.in +++ b/scripts/libmakepkg/util/parseopts.sh.in @@ -102,7 +102,7 @@ parseopts() { 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 + elif (( i == ${#1} - 1 && $# > 1 )); then OPTRET+=("$2") shift break @@ -121,7 +121,7 @@ parseopts() { case $? in 0) # parse failure - if [[ $optarg ]]; then + if [[ $1 = *=* ]]; then printf "${0##*/}: $(gettext "option '%s' does not allow an argument")\n" "--$opt" >&2 OPTRET=(--) return 1 @@ -132,10 +132,10 @@ parseopts() { ;; 1) # --longopt=optarg - if [[ $optarg ]]; then + if [[ $1 = *=* ]]; then OPTRET+=("--$opt" "$optarg") # --longopt optarg - elif [[ $2 ]]; then + elif (( $# > 1 )); then OPTRET+=("--$opt" "$2" ) shift # parse failure diff --git a/test/scripts/parseopts_test.sh b/test/scripts/parseopts_test.sh index 9674c6a6..1d76f1ad 100755 --- a/test/scripts/parseopts_test.sh +++ b/test/scripts/parseopts_test.sh @@ -31,7 +31,7 @@ tap_parse() { unset OPTRET }

-tap_plan 50 +tap_plan 52

# usage: tap_parse <expected result> <token count> test-params... # a failed tap_parse will match only the end of options marker '--' @@ -111,4 +111,7 @@ tap_parse '--force --' 2 --force # exact match on possible stem (opt has optarg) tap_parse '--clean foo --' 3 --clean=foo

+# all possible ways to specify empty optargs +tap_parse '--key --pkg -p --' 7 --key '' --pkg='' -p '' + tap_finish

On 25/10/19 11:48 am, Ethan Sommer wrote:

Previously parseopts checked if there was an argument by checking that the string was non-empty, resulting in empty arguments being incorrectly considered non-existent. This change makes parseopts check if arguments exist at all, rather than checking that they are non-empty

Signed-off-by: Ethan Sommer

I'm pushing the optional argument patch to master. Can you rebase this on top of it. Thanks, Allan

--- scripts/libmakepkg/util/parseopts.sh.in | 8 ++++---- test/scripts/parseopts_test.sh | 5 ++++- 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/scripts/libmakepkg/util/parseopts.sh.in b/scripts/libmakepkg/util/parseopts.sh.in index c056cb1e..8fb862d8 100644 --- a/scripts/libmakepkg/util/parseopts.sh.in +++ b/scripts/libmakepkg/util/parseopts.sh.in @@ -102,7 +102,7 @@ parseopts() { 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 + elif (( i == ${#1} - 1 && $# > 1 )); then OPTRET+=("$2") shift break @@ -121,7 +121,7 @@ parseopts() { case $? in 0) # parse failure - if [[ $optarg ]]; then + if [[ $1 = *=* ]]; then printf "${0##*/}: $(gettext "option '%s' does not allow an argument")\n" "--$opt" >&2 OPTRET=(--) return 1 @@ -132,10 +132,10 @@ parseopts() { ;; 1) # --longopt=optarg - if [[ $optarg ]]; then + if [[ $1 = *=* ]]; then OPTRET+=("--$opt" "$optarg") # --longopt optarg - elif [[ $2 ]]; then + elif (( $# > 1 )); then OPTRET+=("--$opt" "$2" ) shift # parse failure diff --git a/test/scripts/parseopts_test.sh b/test/scripts/parseopts_test.sh index 9674c6a6..1d76f1ad 100755 --- a/test/scripts/parseopts_test.sh +++ b/test/scripts/parseopts_test.sh @@ -31,7 +31,7 @@ tap_parse() { unset OPTRET }

-tap_plan 50 +tap_plan 52

# usage: tap_parse <expected result> <token count> test-params... # a failed tap_parse will match only the end of options marker '--' @@ -111,4 +111,7 @@ tap_parse '--force --' 2 --force # exact match on possible stem (opt has optarg) tap_parse '--clean foo --' 3 --clean=foo

+# all possible ways to specify empty optargs +tap_parse '--key --pkg -p --' 7 --key '' --pkg='' -p '' + tap_finish