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

[Junio C Hamano <gitster@pobox.com>]

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

> +parse_option () {
> +	local opt="$@"

I do not think there is any context in which var="$@" makes sense in
shell script (var="$*" is understandable, though).  

Did you mean opt=$1 here?

> +# Parse options while taking care to leave $@ intact, so we will still
> +# have all the original command line options when executing the test
> +# script again for '--tee' and '--verbose-log' below.

The phrase "below" no longer makes much sense after moving lines
around, does it?

> +store_arg_to=
> +prev_opt=
> +for opt
> +do
> +	if test -n "$store_arg_to"
> +	then
> +		eval $store_arg_to=\$opt
> +		store_arg_to=
> +		prev_opt=
> +		continue
> +	fi
> +
> +	case "$opt" in
> +	--*)
> +		parse_option "$opt" ;;
> +	-?*)
> +		# stacked short options must be fed separately to parse_option

Are you calling concatenated short options, e.g. "-abc", as
"stacked"?  It sounds like a very unusual phrasing, at least to me.

> +		for c in $(echo "${opt#-}" | sed 's/./& /g')
> +		do
> +			parse_option "-$c"
> +		done
> +		;;
> +	*)
> +		echo "error: unknown test option '$opt'" >&2; exit 1 ;;
> +	esac
>  
>  	prev_opt=$opt
>  done

I am personally not very enthused (the line counts vs benefit does
not feel so great), but as long as it works correctly and
maintainable, I won't complain too much.