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

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

Junio C Hamano <gitster@pobox.com> writes:

> 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.

Just in case I confused readers with a message that apparently
conflicts with what I said in the ancient thread:

  http://thread.gmane.org/gmane.comp.version-control.git/199758/focus=200025

I am admitting that I was wrong.  Or perhaps I was right back then,
but the world has changed ;-).

We have been hearing "bisect bad/good" is hard to use for the last 3
years since that thread discussed this topic, and that made me
realize that addition of single new/old may not be good enough, and
we should bite the bullet to support 'bisect terms' properly, making
bad/good and new/old even less special (contrary to what I said back
then in that thread "we only need these two pairs"), following the
suggestion by Phil Hord in that thread.

And the suggested three-step approach above reflects that new world
order in my mind.  We admit that the machinery should have been
built around a value-agnostic "old vs new" in the first place, but
unfortunately it wasn't.  So we belatedly update the system to use
these two terms internally to express the logic by naming the
variables name_old and name_new after these two value-agnostic
concepts, and then support the traditional "good vs bad" as a mere
default values for the names of old and new states.

One thing I forgot to say in the review of this patch:

> +bisect_terms () {
> +	test $# -eq 2 ||
> +	die "You need to give me at least two arguments"
> +
> +	if ! test -s "$GIT_DIR/BISECT_START"
> +	then
> +		echo $1 >"$GIT_DIR/BISECT_TERMS" &&
> +		echo $2 >>"$GIT_DIR/BISECT_TERMS" &&
> +		echo "1" > "$GIT_DIR/TERMS_DEFINED"
> +	else
> +		die "A bisection has already started, please use "\
> +		"'git bisect reset' to restart and change the terms"
> +	fi
> +}
> +

I think "git bisect terms" is a good way to help a user to recall
what two names s/he decided to use for the current session.  So
dying 'already started' with suggestion for 'reset' is OK, but at
the same time, helping the user to continue the current bisection by
giving a message along the lines of "You are hunting for a commit
that is at the boundary of the old state (you are calling it
'$NAME_OLD') and the new state ('$NAME_NEW')" would be a good idea.

Thanks.