Re: [PATCH v2 7/7] bisect: allows any terms set by user

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

Antoine Delaite <antoine.delaite@ensimag.grenoble-inp.fr> writes:

> -USAGE='[help|start|bad|good|new|old|skip|next|reset|visualize|replay|log|run]'
> +USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'

I think this patch makes the whole series go in the right direction.

I wonder if you can skip the "we only support new/old if you are not
doing bog-standard bad/good" step and start from this "bisect terms"
one, though.

Then you do not even have to treat new/old any specially, and do not
even have to list them in the above list.

> @@ -79,9 +81,16 @@ bisect_start() {
>  	orig_args=$(git rev-parse --sq-quote "$@")
>  	bad_seen=0
>  	eval=''
> -	# start_bad_good is used to detect if we did a 
> -	# 'git bisect start bad_rev good_rev'
> -	start_bad_good=0
> +	# terms_defined is used to detect if we did a
> +	# 'git bisect start bad_rev good_rev' or if the user
> +	# defined his own terms with git bisect terms
> +	terms_defined=0

I like this change very much; it removes the mysteriously misnamed
start-bad-good variable (because you do not really _care_ that
'start' was what implicitly decided that good/bad pair is the term
we use in this session; what you care is that the terms are already
known or not).

That is another reason why I think it would be a better organization
for the patch series to do without the intermediate "we now add new/old
as another hardcoded values on top of the traditional bad/good".

That is, I would think a reasonable progression of the series would
look more like these three steps:

 - preliminary clean-up steps (e.g. "correct 'mistook'");

 - use $name_new and $name_old throughout the code, giving them
   'bad' and 'good' as hardcoded values; finally

 - add 'bisect terms' support.

Thanks.