Re: [PATCH v10.1 7/7] bisect: allow any terms set by user

Christian Couder [off-list ref] writes:

On Sat, Jun 27, 2015 at 6:25 AM, Junio C Hamano [off-list ref] wrote:

quoted

On Fri, Jun 26, 2015 at 9:10 PM, Christian Couder
[off-list ref] wrote:

quoted

If we don't want to support positional arguments, then I would suggest
supporting first the following instead:

         git bisect terms --name-good=fast --name-bad=slow
         git bisect terms --name-bad=slow --name-good=fast

This would make the interface consistent with the code.

Which somewhat defeats the point of introducing "old" and "new", though.
The "terms" support is for people who feel that good/bad would be too confusing
for the particular bisect session (e.g. because they are hunting for a fix).

Well if --name-old and --name-new are also available as synonyms, it
would not be too bad I think.
People could use the option names that fit their mental model or their
use case better.

OK, I'll add both.

quoted

quoted

quoted

We may want to start supporting

        git bisect start --new=master --old=maint

Maybe we could also support:

git bisect start --name-good=fast --name-bad=slow --fast=maint --slow=master

The same comment for the token after --name-, but allowing the terms to be set
at "start" could be a type-saver.  With need for added "--name-"
prefix (worse, twice),
I am not sure if it would be seen as a useful type-saver, though.

At least people don't need to remember if they have to use "git bisect
term" before or after starting :-)

OK, first lesson learnt: the UI is controversial. So, I'm working on a
series that splits the last patch into several preparation steps, and a
very simple patch to implement the UI.

I have a draft here https://github.com/moy/git/commits/bisect-terms
I'll go through it once more and send it later.

As a teaser, the patch implementing the UI is just:

--- a/git-bisect.sh
+++ b/git-bisect.sh

@@ -70,6 +70,26 @@ bisect_autostart() {
 	}
 }
 
+parse_name_args() {
+	while [ $# -gt 0 ]; do
+		arg="$1"
+		case "$arg" in
+		--name-good|--name-old)
+			shift
+			must_write_terms=1
+			NAME_GOOD=$1
+			shift ;;
+		--name-bad|--name-new)
+			shift
+			must_write_terms=1
+			NAME_BAD=$1
+			shift ;;
+		*)
+			shift ;;
+		esac
+	done
+}
+
 bisect_start() {
 	#
 	# Check for one bad and then some good revisions.

@@ -88,6 +108,9 @@ bisect_start() {
 	else
 		mode=''
 	fi
+	# Parse --name-* options before the other to allow any mix of
+	# --name-* and revisions on the command-line.
+	parse_name_args "$@"
 	while [ $# -gt 0 ]; do
 		arg="$1"
 		case "$arg" in

@@ -98,6 +121,8 @@ bisect_start() {
 		--no-checkout)
 			mode=--no-checkout
 			shift ;;
+		--name-good|--name-old|--name-bad|--name-new)
+			shift 2 ;;
 		--*)
 			die "$(eval_gettext "unrecognised option: '\$arg'")" ;;
 		*)

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/