Re: [PATCH] test-lib: allow short options to be stacked

From: Jeff King <peff@peff.net>
Date: Mar 21, 2020, 06:26 UTC
Message-ID: <20200321062611.GA1441446@coredump.intra.peff.net>
In-Reply-To: <48c28683412e3e0803d4c7189a6d66daddcdc580.1584759277.git.matheus.bernardino@usp.br>

On Sat, Mar 21, 2020 at 12:07:05AM -0300, Matheus Tavares wrote:

Show 11 quoted lines

> When debugging a test (or a set of tests), it's common to execute it
> with some combination of short options, such as:
> 
> 	$ ./txxx-testname.sh -d -x -i
> 
> In cases like this, CLIs usually allow the short options to be stacked
> in a single argument, for convenience and agility. Let's add this
> feature to test-lib, allowing the above command to be run as:
> 
> 	$ ./txxx-testname.sh -dxi
> 	(or any other permutation, e.g. '-ixd')

Yay. I've grown accustomed to the lack of this feature in the test scripts, but I'll be happy to have it. :)

Most getopt implementations I've seen call this "bundling" rather than "stacking" (I don't care too much either way, but Junio mentioned being confused at the name).

Show 9 quoted lines

> +	case "$opt" in
> +	--*)
> +		parse_option "$opt" ;;
> +	-?*)
> +		# stacked short options must be fed separately to parse_option
> +		for c in $(echo "${opt#-}" | sed 's/./& /g')
> +		do
> +			parse_option "-$c"
> +		done

I wondered if we could do this without the extra process. This works:

  opt=${opt#-}
  while test -n "$opt"
  do
	extra=${opt#?}
	this=${opt%$extra}
	opt=$extra
	parse_option "-$this"
  done

It's a little convoluted. I'm not sure if saving a process per unbundled short option is worth it.

What happens to bundled short options with arguments? I think "-r" is the only one. We don't allow "stuck" short options like "-r5", so we don't have to worry about feeding non-option bits to parse_option(). It looks like we'd only examine $store_arg_to outside of the short-option loop, so we'd treat:

  ./t1234-foo.sh -vrix 5

the same as:

  ./t1234-foo.sh -v -r 5 -i -x

which seems reasonable. But:

  ./t1234-foo.sh -rr 5 6

would get garbled. We'd come out of "-rr" parsing with $store_arg_to set, but only grab the first argument. I think I could live with that, considering this is just the test scripts. Fixing it would require store_arg_to becoming a space-separated list.

-Peff