[pacman-dev] [PATCH v2] libmakepkg: add optional argument support to parseopts

Dave Reisner d at falconindy.com
Wed Oct 23 23:52:22 UTC 2019


On Wed, Oct 23, 2019 at 06:57:24PM -0400, Ethan Sommer wrote:
> Adds a "?" suffix that can be used to indicate that an option's argument is
> optional.
> 
> This allows options to have a default behaviour when the user doesn't
> specify one, e.g.: --color=[when] being able to behave like --color=auto
> when only --color is passed
> 
> Signed-off-by: Ethan Sommer <e5ten.arch at gmail.com>
> ---
>  scripts/libmakepkg/util/parseopts.sh.in | 110 +++++++++++++++---------
>  test/scripts/parseopts_test.sh          |  12 ++-
>  2 files changed, 77 insertions(+), 45 deletions(-)
> 
> diff --git a/scripts/libmakepkg/util/parseopts.sh.in b/scripts/libmakepkg/util/parseopts.sh.in
> index c056cb1e..9a215648 100644
> --- a/scripts/libmakepkg/util/parseopts.sh.in
> +++ b/scripts/libmakepkg/util/parseopts.sh.in
> @@ -18,16 +18,17 @@
>  #   along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  #
>  # A getopt_long-like parser which portably supports longopts and
> -# shortopts with some GNU extensions. It does not allow for options
> -# with optional arguments. For both short and long opts, options
> -# requiring an argument should be suffixed with a colon. After the
> -# first argument containing the short opts, any number of valid long
> -# opts may be be passed. The end of the options delimiter must then be
> -# added, followed by the user arguments to the calling program.
> +# shortopts with some GNU extensions. For both short and long opts,
> +# options requiring an argument should be suffixed with a colon, and
> +# options with optional arguments should be suffixed with a question
> +# mark. After the first argument containing the short opts, any number
> +# of valid long opts may be be passed. The end of the options delimiter
> +# must then be added, followed by the user arguments to the calling
> +# program.
>  #
>  # Recommended Usage:
> -#   OPT_SHORT='fb:z'
> -#   OPT_LONG=('foo' 'bar:' 'baz')
> +#   OPT_SHORT='fb:zq?'
> +#   OPT_LONG=('foo' 'bar:' 'baz' 'qux?')
>  #   if ! parseopts "$OPT_SHORT" "${OPT_LONG[@]}" -- "$@"; then
>  #     exit 1
>  #   fi
> @@ -49,29 +50,30 @@ parseopts() {
>  	longoptmatch() {
>  		local o longmatch=()
>  		for o in "${longopts[@]}"; do
> -			if [[ ${o%:} = "$1" ]]; then
> +			if [[ ${o%[:?]} = "$1" ]]; then
>  				longmatch=("$o")
>  				break
>  			fi
> -			[[ ${o%:} = "$1"* ]] && longmatch+=("$o")
> +			[[ ${o%[:?]} = "$1"* ]] && longmatch+=("$o")
>  		done
>  
>  		case ${#longmatch[*]} in
>  			1)
> -				# success, override with opt and return arg req (0 == none, 1 == required)
> -				opt=${longmatch%:}
> -				if [[ $longmatch = *: ]]; then
> -					return 1
> -				else
> -					return 0
> -				fi ;;
> +				# success, override with opt and return arg req (0 == none, 1 == required, 2 == optional)
> +				opt=${longmatch%[:?]}
> +				case $longmatch in
> +					*:)  return 1 ;;
> +					*\?) return 2 ;;
> +					*)   return 0 ;;
> +				esac
> +				;;
>  			0)
>  				# fail, no match found
>  				return 255 ;;
>  			*)
>  				# fail, ambiguous match
>  				printf "${0##*/}: $(gettext "option '%s' is ambiguous; possibilities:")" "--$1"
> -				printf " '%s'" "${longmatch[@]%:}"
> +				printf " '%s'" "${longmatch[@]%[:?]}"
>  				printf '\n'
>  				return 254 ;;
>  		esac >&2
> @@ -87,32 +89,47 @@ parseopts() {
>  				for (( i = 1; i < ${#1}; i++ )); do
>  					opt=${1:i:1}
>  
> -					# option doesn't exist
> -					if [[ $shortopts != *$opt* ]]; then
> -						printf "${0##*/}: $(gettext "invalid option") -- '%s'\n" "$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 "${0##*/}: $(gettext "option requires an argument") -- '%s'\n" "$opt" >&2
> +					case $shortopts in
> +						# option requires optarg
> +						*$opt:*)
> +							# if we're not at the end of the option chunk, the rest is the optarg
> +							if (( i < ${#1} - 1 )); then
> +								OPTRET+=("-$opt" "${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+=("-$opt" "$2")
> +								shift
> +								break
> +							# parse failure
> +							else
> +								printf "${0##*/}: $(gettext "option requires an argument") -- '%s'\n" "$opt" >&2
> +								OPTRET=(--)
> +								return 1
> +							fi
> +							;;
> +						# option's optarg is optional
> +						*$opt\?*)
> +							# if we're not at the end of the option chunk, the rest is the optarg
> +							if (( i < ${#1} - 1 )); then
> +								OPTRET+=("-$opt=${1:i+1}")

This isn't really documented well (well ok, none of parseopts is
sufficiently documented). If you encounter an option with an optional
argument, you're no longer able to capture "--foo" in your subsequent
options processing logic -- you have to now look at --foo|--foo=* and
potentially split the optarg from the option yourself. That's kind of
awkward. The whole point of parseopts is to avoid this sort of parsing
in the frontend.

An alternative approach would be to do something closer to getopts
itself where you maintain OPTIND to keep global state. That would make
for an API that looks more like:

  while parseopts flag "$shortflags" "${longflags[@]}" -- "$@"; do
    case $flag in
      -p|--ponies)
        ponies=$PARSEOPTSARG

        # if it's a flag that has an optional arg, we can unset
        # PARSEOPTSARG to distinguish between not provided and provided
        # but empty string.
        ;;
      # other flag handling...
    esac
  done
  shift $(( PARSEOPTSIND - 1 ))

I suppose it's not critical in what order this happens since the
parseopts code is all internal...

> +								break
> +							# option has no optarg
> +							else
> +								OPTRET+=("-$opt")
> +							fi
> +							;;
> +						# option has no optarg
> +						*$opt*)
> +							OPTRET+=("-$opt")
> +							;;
> +						# option doesn't exist
> +						*)
> +							printf "${0##*/}: $(gettext "invalid option") -- '%s'\n" "$opt" >&2
>  							OPTRET=(--)
>  							return 1
> -						fi
> -					fi
> +							;;
> +					esac
>  				done
>  				;;
>  			--?*=*|--?*) # long option
> @@ -145,6 +162,15 @@ parseopts() {
>  							return 1
>  						fi
>  						;;
> +					2)
> +						# --longopt=optarg
> +						if [[ $1 = *=* ]]; then
> +							OPTRET+=("--$opt=$optarg")
> +						# --longopt
> +						else
> +							OPTRET+=("--$opt")
> +						fi
> +						;;
>  					254)
>  						# ambiguous option -- error was reported for us by longoptmatch()
>  						OPTRET=(--)
> diff --git a/test/scripts/parseopts_test.sh b/test/scripts/parseopts_test.sh
> index 9674c6a6..7d14bf29 100755
> --- a/test/scripts/parseopts_test.sh
> +++ b/test/scripts/parseopts_test.sh
> @@ -16,12 +16,12 @@ if ! type -t parseopts &>/dev/null; then
>  fi
>  
>  # borrow opts from makepkg
> -OPT_SHORT="AcdefFghiLmop:rRsV"
> +OPT_SHORT="AcdefFghiLmop:rRsVb;"
>  OPT_LONG=('allsource' 'asroot' 'ignorearch' 'check' 'clean:' 'cleanall' 'nodeps'
>            'noextract' 'force' 'forcever:' 'geninteg' 'help' 'holdver'
>            'install' 'key:' 'log' 'nocolor' 'nobuild' 'nocheck' 'noprepare' 'nosign' 'pkg:' 'rmdeps'
>            'repackage' 'skipinteg' 'sign' 'source' 'syncdeps' 'version' 'config:'
> -          'noconfirm' 'noprogressbar')
> +          'noconfirm' 'noprogressbar' 'opt;')
>  
>  tap_parse() {
>  	local result=$1 tokencount=$2; shift 2
> @@ -31,7 +31,7 @@ tap_parse() {
>  	unset OPTRET
>  }
>  
> -tap_plan 50
> +tap_plan 54
>  
>  # usage: tap_parse <expected result> <token count> test-params...
>  # a failed tap_parse will match only the end of options marker '--'
> @@ -111,4 +111,10 @@ tap_parse '--force --' 2 --force
>  # exact match on possible stem (opt has optarg)
>  tap_parse '--clean foo --' 3 --clean=foo
>  
> +# long opt with empty, non-empty, and no optional arg
> +tap_parse '--opt= --opt=foo --opt --' 4 --opt= --opt=foo --opt
> +
> +# short opt with and without optional arg, and non-option arg
> +tap_parse '-b=foo -A -b -- foo' 5 -bfoo -Ab foo

This is strictly different from getopts. "-b=foo" would be treated as
having a value of "=foo", not "foo".

> +
>  tap_finish
> -- 
> 2.23.0


More information about the pacman-dev mailing list