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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 21, 2020, 20:07 UTC
Message-ID: <xmqqtv2h8oac.fsf@gitster.c.googlers.com>
In-Reply-To: <7a6a8197dcd58e8690892d03cb904dd1eec5d7c1.1584818457.git.matheus.bernardino@usp.br>

Matheus Tavares <matheus.bernardino@usp.br> writes:

Show 6 quoted lines

> - Added a check for bundles containing more than one "option that
>   requires args" (e.g. '-rr'), in which case we error-out. We could
>   interpret '-rr 1 2' as 'run tests 1 _and_ 2', but the unbundled
>   format, '-r 1 -r 2', is not currently interpreted like that (the last
>   just overrides the previous). So, for simplicity, let's only forbid
>   such bundles for now.

Makes sense.  I think this is the best we can do at this moment.

Show 5 quoted lines

> +opt_required_arg=
> +# $1: option string
> +# $2: name of the var where the arg will be stored
> +mark_option_requires_arg ()
> +{

"{" on the same line, just like you did for parse_option below.

Show 16 quoted lines

> +	if test -n "$opt_required_arg"
>  	then
> +		echo "error: options that require args cannot be bundled" \
> +			"together: '$opt_required_arg' and '$1'" >&2
> +		exit 1
>  	fi
> +	opt_required_arg=$1
> +	store_arg_to=$2
> +}
> +
> +parse_option () {
> +	local opt="$1"
> ...
> +	case "$opt" in
> +	--*)
> +		parse_option "$opt" ;;

I think J6t's suggestion to the previous round still has merit here.

Show 6 quoted lines

> +	-?*)
> +		# bundled short options must be fed separately to parse_option
> +		opt=${opt#-}
> +		while test -n "$opt"
> +		do
> +			extra=${opt#?}

Take the rest of the string after stripping the first one in $extra ...

> +			this=${opt%$extra}

... and then strip that tail part from the end, which would give the first letter in $this.

> +			opt=$extra

And the next round will use the remainder after taking $this out of the bundled options from the front.

Makes sense.

> +			parse_option "-$this"
> +		done

Thanks