Re: [PATCH v2 2/7] test-lib: parse some --options earlier

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Mon, Dec 17, 2018 at 04:44:36PM -0500, Jeff King wrote:
> On Tue, Dec 11, 2018 at 01:42:45PM +0100, SZEDER Gábor wrote:

> > diff --git a/t/test-lib.sh b/t/test-lib.sh
> > index 9a3f7930a3..efdb6be3c8 100644
> > --- a/t/test-lib.sh
> > +++ b/t/test-lib.sh
> > @@ -264,58 +264,65 @@ test "x$TERM" != "xdumb" && (
> >  	) &&
> >  	color=t
> >  
> > -while test "$#" -ne 0
> > +store_arg_to=
> > +prev_opt=
> > +for opt
> >  do
> > -	case "$1" in
> > +	if test -n "$store_arg_to"
> > +	then
> > +		eval $store_arg_to=\$opt
> > +		store_arg_to=
> > +		prev_opt=
> > +		continue
> > +	fi
> 
> OK, so this is set for the unstuck options, which then pick up the
> option in the next loop iteration. That's perhaps less gross than my
> "re-build the options with set --" trick.
> 
> A simple variable set is enough for "-r". In theory we could make this:
> 
>   if test -n "$handle_unstuck_arg"
>   then
> 	eval "$handle_unstuck_arg \$1"
>   fi
>   ...
> 
>   -r)
> 	handle_unstuck_arg=handle_opt_r ;;
> 
> and handle_opt_r() could do whatever it wants. But I don't really
> foresee us adding a lot of new options

Yeah, I would refrain from making it too general and fancy with a
callback function for now, when there is only a single option that
could use it.

> (in fact, given that this is just
> the internal tests, I am tempted to say that we should just make it
> "-r<arg>" for the sake of simplicity and consistency. But maybe somebody
> would be annoyed. I have never used "-r" ever myself).

I didn't even know what '-r' does...

And I agree that changing it to '-r<arg>' would be the best, but this
patch series is about adding '--stress', so changing how '-r' gets its
mandatory argument (and potentially annoying someone) is beyond the
scope, I would say.