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

[Dave Reisner]

On Fri, Apr 13, 2012 at 12:46 AM, Allan McRae <allan at archlinux.org> 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 <dreisner at archlinux.org>
> > ---
>
> <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
>