Re: [PATCH v14 13/27] bisect--helper: `bisect_start` shell function partially in C

Junio C Hamano [off-list ref] writes:

quoted

+	for (i = 0; i < revs.nr; i++) {
+		if (bad_seen)
+			string_list_append(&states, terms->term_good.buf);
+		else {
+			bad_seen = 1;
+			string_list_append(&states, terms->term_bad.buf);
+		}
+	}

This is certainly different from the original.  We used to do one
"bisect_write" per element in revs in the loop; we no longer do that
and instead collect them in states list.  Each element in these two
lists, i.e. revs.item[i] and states.item[i], corresponds to each
other.

That explains why the variable is renamed to states.  We haven't
seen enough to say if this behaviour change is a good idea or not.

Ahh, I misread the original.  It accumulates the writes to be
executed in $eval and does so at the end.  So there is no change in
behaviour.

So please ignore that point in the previous message.  That leaves
only the following points:

 * Perhaps 'retval' with 'goto finish' pattern?

 * ref_exists()?  Perhaps use skip_prefix(head, "refs/heads/, &branch)?

 * if (clean-state) { return -1 }?

 * Is comment on trap relevant here?

Sorry, and thanks.