Re: [pacman-dev] [PATCH 1/8] scripts/library: introduce parseopts

On Fri, Apr 13, 2012 at 12:46 AM, Allan McRae wrote:

On 13/04/12 00:54, Dave Reisner wrote:

...

This will replace our current options parser used in pacman-key and makepkg. It follows heuristics closer to that of GNU getopt long (and thus pacman itself), with the exception that it does not allow for options with optional arguments. Due to the way this parser will be used, this sort of functionality will not be needed.

Instead of relying on eval+set, options are normalized into an array, OPTRET, which callers should expect to be populated after returning from parseopts. This avoids problems with quotes and spaces in arguments, assuming that the user quotes properly when passing into the application.

A new test harness for parseopts is added in test/scripts.

Signed-off-by: Dave Reisner ---

<snip>

...

+ + get_argreq() { + local o re="^($1)(:?:?)$"

You only want one :? in the regex.

Ah yes.. a vestigial from when I was toying with optional arguments.

...

+ for o in "${longopts[@]}"; do + # found option, return number of colons (0 = none, 1 = required) + [[ $o =~ $re ]] && return ${#BASH_REMATCH[2]} + done + # failure + return 255 + }

<snip>

...

+test_result() { + local result=$1 tokencount=$2 input=$3; shift 3 + + if { [[ $result = "$*" ]] || [[ $2 = NULL && -z $1 ]]; } && (( tokencount == $# )); then

What is the "|| [[ $2 = NULL && -z $1 ]];" part for? I can guess, but it seems unneeded.

I have no idea what it's for. This test toy (and some of the parseopts code) dates back to July-ish of last year. Removing the entire middle check seems innocuous, so its gone.

With the minor changes I pointed out in the last few emails, I ack this patchset. My biggest complaint is I prefer the name "parse_options" over "parseopts"!

Allan