[PATCH] bisect: use selected alternate terms in status output

[PATCH] bisect: use selected alternate terms in status output

[Jonas Rebmann]

Alternate bisect terms are helpful when the terms "good" and "bad" are
confusing such as when bisecting for the resolution of an issue (the
first good commit) rather than the introduction of a regression. If
alternate terms are chosen, the terms "good" and "bad" should not be
used in git's output to avoid confusion.

An old/new bisect should end with
$ git bisect old
[sha] is the first new commit

not with
$ git bisect old
[sha] is the first bad commit

Using hardcoded good/bad vocabulary can give confusion about what action
is required:

  status: waiting for bad commit, 1 good commit known
  $ git bisect bad
  error: Invalid command: you're currently in a new/old bisect
  fatal: unknown command: 'bad'

To avoid confusion, use alternate terms consistently across the bisect
output.

Signed-off-by: Jonas Rebmann <kernel@schlaraffenlan.de>
---
 builtin/bisect.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/builtin/bisect.c b/builtin/bisect.c
index 4520e585d0..ee6a2c83b8 100644
--- a/builtin/bisect.c
+++ b/builtin/bisect.c
@@ -465,13 +465,16 @@ static void bisect_print_status(const struct bisect_terms *terms)
 		return;
 
 	if (!state.nr_good && !state.nr_bad)
-		bisect_log_printf(_("status: waiting for both good and bad commits\n"));
+		bisect_log_printf(_("status: waiting for both %s and %s commits\n"),
+				  terms->term_good, terms->term_bad);
 	else if (state.nr_good)
-		bisect_log_printf(Q_("status: waiting for bad commit, %d good commit known\n",
-				     "status: waiting for bad commit, %d good commits known\n",
-				     state.nr_good), state.nr_good);
+		bisect_log_printf(Q_("status: waiting for %s commit, %d %s commit known\n",
+				     "status: waiting for %s commit, %d %s commits known\n",
+				     state.nr_good),
+				  terms->term_bad, state.nr_good, terms->term_good);
 	else
-		bisect_log_printf(_("status: waiting for good commit(s), bad commit known\n"));
+		bisect_log_printf(_("status: waiting for %s commit(s), %s commit known\n"),
+				  terms->term_good, terms->term_bad);
 }
 
 static int bisect_next_check(const struct bisect_terms *terms,
@@ -1262,14 +1265,14 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
 			int rc = verify_good(terms, command.buf);
 			is_first_run = 0;
 			if (rc < 0 || 128 <= rc) {
-				error(_("unable to verify %s on good"
-					" revision"), command.buf);
+				error(_("unable to verify %s on %s"
+					" revision"), command.buf, terms->term_good);
 				res = BISECT_FAILED;
 				break;
 			}
 			if (rc == res) {
-				error(_("bogus exit code %d for good revision"),
-				      rc);
+				error(_("bogus exit code %d for %s revision"),
+				      rc, terms->term_good);
 				res = BISECT_FAILED;
 				break;
 			}
@@ -1314,7 +1317,7 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
 			puts(_("bisect run success"));
 			res = BISECT_OK;
 		} else if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
-			puts(_("bisect found first bad commit"));
+			printf(_("bisect found first %s commit\n"), terms->term_bad);
 			res = BISECT_OK;
 		} else if (res) {
 			error(_("bisect run failed: 'git bisect %s'"

---
base-commit: 1eceb487f285f1efa78465e6208770318f9f4892
change-id: 20260320-bisect-terms-76036676769c

Best regards,
-- 
Jonas Rebmann <kernel@schlaraffenlan.de>

Re: [PATCH] bisect: use selected alternate terms in status output

[Junio C Hamano]

Jonas Rebmann <kernel@schlaraffenlan.de> writes:

> Alternate bisect terms are helpful when the terms "good" and "bad" are
> confusing such as when bisecting for the resolution of an issue (the
> first good commit) rather than the introduction of a regression. If
> alternate terms are chosen, the terms "good" and "bad" should not be
> used in git's output to avoid confusion.
>
> An old/new bisect should end with
> $ git bisect old
> [sha] is the first new commit
>
> not with
> $ git bisect old
> [sha] is the first bad commit

Well articulated.

To clarify the status quo, you may probably want to describe what
these custom terms are currently used for.  As far as I can tell,

 * "git bisect <good>/<bad>" that marks the commit you just tested
   accepts the custom term for *input*.

 * refs/bisect/<good>-<commit-object-name> (many good commits) and
   refs/bisect/<bad> (a commit that is the oldest bad one currently
   known) use the custom terms, which would show in "git bisect
   visualize" for *output*

 * "X is the first <bad> commit" report should but currently does
   not use the custom term, which you are addressing in this patch.

Do we use good/bad or custom terms anywhere else?  There aren't too
many, so it would be good to be exhaustive in the proposed log
message here.

> Using hardcoded good/bad vocabulary can give confusion about what action
> is required:

"can give confusion" -> "can cause confusion", or, "can be confusing".

>   status: waiting for bad commit, 1 good commit known
>   $ git bisect bad
>   error: Invalid command: you're currently in a new/old bisect
>   fatal: unknown command: 'bad'
>
> To avoid confusion, use alternate terms consistently across the bisect
> output.

Sounds good.

> Signed-off-by: Jonas Rebmann <kernel@schlaraffenlan.de>
> ---
>  builtin/bisect.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)

The changes in the patch look good (but it is hard to tell if this
is exhaustive, or there are places where good/bad are still used).

Thanks.


> diff --git a/builtin/bisect.c b/builtin/bisect.c
> index 4520e585d0..ee6a2c83b8 100644
> --- a/builtin/bisect.c
> +++ b/builtin/bisect.c
> @@ -465,13 +465,16 @@ static void bisect_print_status(const struct bisect_terms *terms)
>  		return;
>  
>  	if (!state.nr_good && !state.nr_bad)
> -		bisect_log_printf(_("status: waiting for both good and bad commits\n"));
> +		bisect_log_printf(_("status: waiting for both %s and %s commits\n"),
> +				  terms->term_good, terms->term_bad);
>  	else if (state.nr_good)
> -		bisect_log_printf(Q_("status: waiting for bad commit, %d good commit known\n",
> -				     "status: waiting for bad commit, %d good commits known\n",
> -				     state.nr_good), state.nr_good);
> +		bisect_log_printf(Q_("status: waiting for %s commit, %d %s commit known\n",
> +				     "status: waiting for %s commit, %d %s commits known\n",
> +				     state.nr_good),
> +				  terms->term_bad, state.nr_good, terms->term_good);
>  	else
> -		bisect_log_printf(_("status: waiting for good commit(s), bad commit known\n"));
> +		bisect_log_printf(_("status: waiting for %s commit(s), %s commit known\n"),
> +				  terms->term_good, terms->term_bad);
>  }
>  
>  static int bisect_next_check(const struct bisect_terms *terms,
> @@ -1262,14 +1265,14 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
>  			int rc = verify_good(terms, command.buf);
>  			is_first_run = 0;
>  			if (rc < 0 || 128 <= rc) {
> -				error(_("unable to verify %s on good"
> -					" revision"), command.buf);
> +				error(_("unable to verify %s on %s"
> +					" revision"), command.buf, terms->term_good);
>  				res = BISECT_FAILED;
>  				break;
>  			}
>  			if (rc == res) {
> -				error(_("bogus exit code %d for good revision"),
> -				      rc);
> +				error(_("bogus exit code %d for %s revision"),
> +				      rc, terms->term_good);
>  				res = BISECT_FAILED;
>  				break;
>  			}
> @@ -1314,7 +1317,7 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)
>  			puts(_("bisect run success"));
>  			res = BISECT_OK;
>  		} else if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {
> -			puts(_("bisect found first bad commit"));
> +			printf(_("bisect found first %s commit\n"), terms->term_bad);
>  			res = BISECT_OK;
>  		} else if (res) {
>  			error(_("bisect run failed: 'git bisect %s'"
>
> ---
> base-commit: 1eceb487f285f1efa78465e6208770318f9f4892
> change-id: 20260320-bisect-terms-76036676769c
>
> Best regards,

Re: [PATCH] bisect: use selected alternate terms in status output

[Junio C Hamano]

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

> Sounds good.
>
>> Signed-off-by: Jonas Rebmann <kernel@schlaraffenlan.de>
>> ---
>>  builtin/bisect.c | 23 +++++++++++++----------
>>  1 file changed, 13 insertions(+), 10 deletions(-)
>
> The changes in the patch look good (but it is hard to tell if this
> is exhaustive, or there are places where good/bad are still used).
>
> Thanks.

One thing I forgot to mention.  This lacks tests to protect the
feature against future breakage.

Thanks.