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

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

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,