{"thread":{"id":"65340","subject":"[PATCH v2 0/2] Avoid hardcoded \"good\"/\"bad\" bisect terms","startedAt":"2026-03-23T23:07:26Z","lastAt":"2026-03-24T17:33:03Z","messageCount":9,"participants":["Jonas Rebmann","Phillip Wood","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"539789","messageId":"20260323-bisect-terms-v2-0-8d6bdb2c9c7e@schlaraffenlan.de","threadId":"65340","inReplyTo":null,"subject":"[PATCH v2 0/2] Avoid hardcoded \"good\"/\"bad\" bisect terms","fromName":"Jonas Rebmann","fromEmail":"kernel@schlaraffenlan.de","sentAt":"2026-03-23T22:48:58Z","receivedAt":"2026-03-23T23:07:26Z","isPatch":true,"sender":{"key":"kernel@schlaraffenlan.de","avatar":null},"body":"While checking whether all output messages of git bisect where covered\nby [PATCH 1/2] bisect: use selected alternate terms in status output\nI found hardcoded good/bad refs leading to incompatibility of git\nrev-parse --bisect with alternate bisect run terms. This is addressed\nby [PATCH 2/2] rev-parse: use selected alternate terms too look up refs\n\nSigned-off-by: Jonas Rebmann <kernel@schlaraffenlan.de>\n---\nChanges in v2:\n- Improve commit message\n- Add tests\n- Include second patch for hardcoded good/bad in rev-parse\n- Link to v1: https://lore.kernel.org/r/20260320-bisect-terms-v1-1-c30c9540542a@schlaraffenlan.de\n\n---\nJonas Rebmann (2):\n      bisect: use selected alternate terms in status output\n      rev-parse: use selected alternate terms too look up refs\n\n builtin/bisect.c            | 23 +++++++++++++----------\n builtin/rev-parse.c         |  8 ++++++--\n t/t6030-bisect-porcelain.sh | 16 ++++++++++++++--\n 3 files changed, 33 insertions(+), 14 deletions(-)\n---\nbase-commit: 1eceb487f285f1efa78465e6208770318f9f4892\nchange-id: 20260320-bisect-terms-76036676769c\n\nBest regards,\n--  \nJonas Rebmann <kernel@schlaraffenlan.de>\n\n"},{"id":"539793","messageId":"20260323-bisect-terms-v2-1-8d6bdb2c9c7e@schlaraffenlan.de","threadId":"65340","inReplyTo":"20260323-bisect-terms-v2-0-8d6bdb2c9c7e@schlaraffenlan.de","subject":"[PATCH v2 1/2] bisect: use selected alternate terms in status output","fromName":"Jonas Rebmann","fromEmail":"kernel@schlaraffenlan.de","sentAt":"2026-03-23T22:48:59Z","receivedAt":"2026-03-24T00:07:27Z","isPatch":true,"sender":{"key":"kernel@schlaraffenlan.de","avatar":null},"body":"Alternate bisect terms are helpful when the terms \"good\" and \"bad\" are\nconfusing such as when bisecting for the resolution of an issue (the\nfirst good commit) rather than the introduction of a regression.\n\nThese terms must be used when marking a commit (e.g. `git bisect new`),\nthey will be used in reference names (e.g. refs/bisect/new) and they are\nused in parts of git's log output such as \"<sha> was both old and new\"\nin git bisect skip's output.\n\nHowever, hardcoded \"good\"/\"bad\" terms are still used in a few status\nmessages and can cause confusion about the status of the bisect such as:\n\n  $ git bisect old\n  [sha] is the first new commit\n\nor about the required action such as:\n\n  status: waiting for bad commit, 1 good commit known\n  $ git bisect bad\n  error: Invalid command: you're currently in a new/old bisect\n  fatal: unknown command: 'bad'\n\nThis commit updates all remaining output messages which use hardcoded\n\"good\" and \"bad\" terms to use the selected terms consistently across the\nbisect output and adds tests.\n\nSigned-off-by: Jonas Rebmann <kernel@schlaraffenlan.de>\n---\n builtin/bisect.c            | 23 +++++++++++++----------\n t/t6030-bisect-porcelain.sh | 16 ++++++++++++++--\n 2 files changed, 27 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/bisect.c b/builtin/bisect.c\nindex 4520e585d0..ee6a2c83b8 100644\n--- a/builtin/bisect.c\n+++ b/builtin/bisect.c\n@@ -465,13 +465,16 @@ static void bisect_print_status(const struct bisect_terms *terms)\n \t\treturn;\n \n \tif (!state.nr_good && !state.nr_bad)\n-\t\tbisect_log_printf(_(\"status: waiting for both good and bad commits\\n\"));\n+\t\tbisect_log_printf(_(\"status: waiting for both %s and %s commits\\n\"),\n+\t\t\t\t  terms->term_good, terms->term_bad);\n \telse if (state.nr_good)\n-\t\tbisect_log_printf(Q_(\"status: waiting for bad commit, %d good commit known\\n\",\n-\t\t\t\t     \"status: waiting for bad commit, %d good commits known\\n\",\n-\t\t\t\t     state.nr_good), state.nr_good);\n+\t\tbisect_log_printf(Q_(\"status: waiting for %s commit, %d %s commit known\\n\",\n+\t\t\t\t     \"status: waiting for %s commit, %d %s commits known\\n\",\n+\t\t\t\t     state.nr_good),\n+\t\t\t\t  terms->term_bad, state.nr_good, terms->term_good);\n \telse\n-\t\tbisect_log_printf(_(\"status: waiting for good commit(s), bad commit known\\n\"));\n+\t\tbisect_log_printf(_(\"status: waiting for %s commit(s), %s commit known\\n\"),\n+\t\t\t\t  terms->term_good, terms->term_bad);\n }\n \n static int bisect_next_check(const struct bisect_terms *terms,\n@@ -1262,14 +1265,14 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n \t\t\tint rc = verify_good(terms, command.buf);\n \t\t\tis_first_run = 0;\n \t\t\tif (rc < 0 || 128 <= rc) {\n-\t\t\t\terror(_(\"unable to verify %s on good\"\n-\t\t\t\t\t\" revision\"), command.buf);\n+\t\t\t\terror(_(\"unable to verify %s on %s\"\n+\t\t\t\t\t\" revision\"), command.buf, terms->term_good);\n \t\t\t\tres = BISECT_FAILED;\n \t\t\t\tbreak;\n \t\t\t}\n \t\t\tif (rc == res) {\n-\t\t\t\terror(_(\"bogus exit code %d for good revision\"),\n-\t\t\t\t      rc);\n+\t\t\t\terror(_(\"bogus exit code %d for %s revision\"),\n+\t\t\t\t      rc, terms->term_good);\n \t\t\t\tres = BISECT_FAILED;\n \t\t\t\tbreak;\n \t\t\t}\n@@ -1314,7 +1317,7 @@ static int bisect_run(struct bisect_terms *terms, int argc, const char **argv)\n \t\t\tputs(_(\"bisect run success\"));\n \t\t\tres = BISECT_OK;\n \t\t} else if (res == BISECT_INTERNAL_SUCCESS_1ST_BAD_FOUND) {\n-\t\t\tputs(_(\"bisect found first bad commit\"));\n+\t\t\tprintf(_(\"bisect found first %s commit\\n\"), terms->term_bad);\n \t\t\tres = BISECT_OK;\n \t\t} else if (res) {\n \t\t\terror(_(\"bisect run failed: 'git bisect %s'\"\ndiff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh\nindex 1ba9ca219e..9d28d1eedb 100755\n--- a/t/t6030-bisect-porcelain.sh\n+++ b/t/t6030-bisect-porcelain.sh\n@@ -1077,8 +1077,10 @@ test_expect_success 'bisect terms shows good/bad after start' '\n \n test_expect_success 'bisect start with one term1 and term2' '\n \tgit bisect reset &&\n-\tgit bisect start --term-old term2 --term-new term1 &&\n-\tgit bisect term2 $HASH1 &&\n+\tgit bisect start --term-old term2 --term-new term1 >bisect_result &&\n+\tgrep \"status: waiting for both term2 and term1 commits\" bisect_result &&\n+\tgit bisect term2 $HASH1 >bisect_result &&\n+\tgrep \"status: waiting for term1 commit, 1 term2 commit known\" bisect_result &&\n \tgit bisect term1 $HASH4 &&\n \tgit bisect term1 &&\n \tgit bisect term1 >bisect_result &&\n@@ -1103,6 +1105,16 @@ test_expect_success 'bisect replay with term1 and term2' '\n \tgit bisect reset\n '\n \n+test_expect_success 'bisect run term1 term2' '\n+\tgit bisect reset &&\n+\tgit bisect start --term-new term1 --term-old term2 $HASH4 $HASH1 &&\n+\tgit bisect term1 &&\n+\tgit bisect run false >bisect_result &&\n+\tgrep \"bisect found first term1 commit\" bisect_result &&\n+\tgit bisect log >log_to_replay.txt &&\n+\tgit bisect reset\n+'\n+\n test_expect_success 'bisect start term1 term2' '\n \tgit bisect reset &&\n \tgit bisect start --term-new term1 --term-old term2 $HASH4 $HASH1 &&\n\n-- \n2.53.0\n\n"},{"id":"539809","messageId":"20260323-bisect-terms-v2-2-8d6bdb2c9c7e@schlaraffenlan.de","threadId":"65340","inReplyTo":"20260323-bisect-terms-v2-0-8d6bdb2c9c7e@schlaraffenlan.de","subject":"[PATCH v2 2/2] rev-parse: use selected alternate terms too look up refs","fromName":"Jonas Rebmann","fromEmail":"kernel@schlaraffenlan.de","sentAt":"2026-03-23T22:49:00Z","receivedAt":"2026-03-24T07:12:34Z","isPatch":true,"sender":{"key":"kernel@schlaraffenlan.de","avatar":null},"body":"An old/new bisect will name refs \"refs/bisect/old\" (or new) accordingly\nso the hardcoded \"refs/bisect/bad\" (and good) yields no results in a\nbisect using alternate terms.\n\nUse the current bisect_terms to make rev-parse --bisect work in an\nalternate term bisect.\n\nSigned-off-by: Jonas Rebmann <kernel@schlaraffenlan.de>\n---\n builtin/rev-parse.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 01a62800e8..f20f0554ed 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -10,6 +10,7 @@\n #include \"builtin.h\"\n \n #include \"abspath.h\"\n+#include \"bisect.h\"\n #include \"config.h\"\n #include \"commit.h\"\n #include \"environment.h\"\n@@ -940,11 +941,14 @@ int cmd_rev_parse(int argc,\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--bisect\")) {\n+\t\t\t\tchar *term_bad = NULL;\n+\t\t\t\tchar *term_good = NULL;\n \t\t\t\tstruct refs_for_each_ref_options opts = { 0 };\n-\t\t\t\topts.prefix = \"refs/bisect/bad\";\n+\t\t\t\tread_bisect_terms(&term_bad, &term_good);\n+\t\t\t\topts.prefix = xstrfmt(\"refs/bisect/%s\", term_bad);\n \t\t\t\trefs_for_each_ref_ext(get_main_ref_store(the_repository),\n \t\t\t\t\t\t      show_reference, NULL, &opts);\n-\t\t\t\topts.prefix = \"refs/bisect/good\";\n+\t\t\t\topts.prefix = xstrfmt(\"refs/bisect/%s\", term_good);\n \t\t\t\trefs_for_each_ref_ext(get_main_ref_store(the_repository),\n \t\t\t\t\t\t      anti_reference, NULL, &opts);\n \t\t\t\tcontinue;\n\n-- \n2.53.0\n\n"},{"id":"539827","messageId":"f8f7a220-c40a-480d-b0d0-abfcf5c83157@gmail.com","threadId":"65340","inReplyTo":"20260323-bisect-terms-v2-1-8d6bdb2c9c7e@schlaraffenlan.de","subject":"Re: [PATCH v2 1/2] bisect: use selected alternate terms in status output","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-24T10:43:06Z","receivedAt":"2026-03-24T10:43:13Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Jonas\n\nOn 23/03/2026 22:48, Jonas Rebmann wrote:\n> \n> diff --git a/builtin/bisect.c b/builtin/bisect.c\n> index 4520e585d0..ee6a2c83b8 100644\n> --- a/builtin/bisect.c\n> +++ b/builtin/bisect.c\n> @@ -465,13 +465,16 @@ static void bisect_print_status(const struct bisect_terms *terms)\n>   \t\treturn;\n>   \n>   \tif (!state.nr_good && !state.nr_bad)\n> -\t\tbisect_log_printf(_(\"status: waiting for both good and bad commits\\n\"));\n> +\t\tbisect_log_printf(_(\"status: waiting for both %s and %s commits\\n\"),\n> +\t\t\t\t  terms->term_good, terms->term_bad);\n\nIf we're going to start using alternative terms it might be better to \nenclose them in single quotes to make it clearer that we're referencing \nthe term names. Looking at the test below\n\n\t\"status: waiting for both 'term1' and 'term2' commits\"\n\nis clearer to me than\n\n\t\"status: waiting for both term1 and term2 commits\"\n\n>   test_expect_success 'bisect start with one term1 and term2' '\n>   \tgit bisect reset &&\n> -\tgit bisect start --term-old term2 --term-new term1 &&\n> -\tgit bisect term2 $HASH1 &&\n> +\tgit bisect start --term-old term2 --term-new term1 >bisect_result &&\n> +\tgrep \"status: waiting for both term2 and term1 commits\" bisect_result &&\n\nUsing test_grep would make debugging test failures easier as, if it \nfails, it prints a helpful diagnostic message.\n\nThanks\n\nPhillip\n\n> +\tgit bisect term2 $HASH1 >bisect_result &&\n> +\tgrep \"status: waiting for term1 commit, 1 term2 commit known\" bisect_result &&\n>   \tgit bisect term1 $HASH4 &&\n>   \tgit bisect term1 &&\n>   \tgit bisect term1 >bisect_result &&\n> @@ -1103,6 +1105,16 @@ test_expect_success 'bisect replay with term1 and term2' '\n>   \tgit bisect reset\n>   '\n>   \n> +test_expect_success 'bisect run term1 term2' '\n> +\tgit bisect reset &&\n> +\tgit bisect start --term-new term1 --term-old term2 $HASH4 $HASH1 &&\n> +\tgit bisect term1 &&\n> +\tgit bisect run false >bisect_result &&\n> +\tgrep \"bisect found first term1 commit\" bisect_result &&\n> +\tgit bisect log >log_to_replay.txt &&\n> +\tgit bisect reset\n> +'\n> +\n>   test_expect_success 'bisect start term1 term2' '\n>   \tgit bisect reset &&\n>   \tgit bisect start --term-new term1 --term-old term2 $HASH4 $HASH1 &&\n> \n\n"},{"id":"539828","messageId":"d366fc82-efcc-46cb-9536-cd38b1fd18d4@gmail.com","threadId":"65340","inReplyTo":"20260323-bisect-terms-v2-2-8d6bdb2c9c7e@schlaraffenlan.de","subject":"Re: [PATCH v2 2/2] rev-parse: use selected alternate terms too look up refs","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-24T10:49:32Z","receivedAt":"2026-03-24T10:49:38Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Jonas\n\nOn 23/03/2026 22:49, Jonas Rebmann wrote:\n> An old/new bisect will name refs \"refs/bisect/old\" (or new) accordingly\n> so the hardcoded \"refs/bisect/bad\" (and good) yields no results in a\n> bisect using alternate terms.\n> \n> Use the current bisect_terms to make rev-parse --bisect work in an\n> alternate term bisect.\n\nIt would be clearer if the commit message started by stating the problem \nthat it is solving i.e. \"git rev-parse --bisect\" does not work if the \nbisect is using alternate term names.\n\n> \n> Signed-off-by: Jonas Rebmann <kernel@schlaraffenlan.de>\n> ---\n>   builtin/rev-parse.c | 8 ++++++--\n>   1 file changed, 6 insertions(+), 2 deletions(-)\n> \n> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\n> index 01a62800e8..f20f0554ed 100644\n> --- a/builtin/rev-parse.c\n> +++ b/builtin/rev-parse.c\n> @@ -10,6 +10,7 @@\n>   #include \"builtin.h\"\n>   \n>   #include \"abspath.h\"\n> +#include \"bisect.h\"\n>   #include \"config.h\"\n>   #include \"commit.h\"\n>   #include \"environment.h\"\n> @@ -940,11 +941,14 @@ int cmd_rev_parse(int argc,\n>   \t\t\t\tcontinue;\n>   \t\t\t}\n>   \t\t\tif (!strcmp(arg, \"--bisect\")) {\n> +\t\t\t\tchar *term_bad = NULL;\n> +\t\t\t\tchar *term_good = NULL;\n>   \t\t\t\tstruct refs_for_each_ref_options opts = { 0 };\n> -\t\t\t\topts.prefix = \"refs/bisect/bad\";\n> +\t\t\t\tread_bisect_terms(&term_bad, &term_good);\n\nIf we fail to read the terms because there is no bisect in progress then \nterm_bad and term_good will be NULL and so the next line will segfault. \nWe should also free term_bad and term_good once we've finished with them \nto avoid a memory leak. It would be a good idea to add some tests.\n\nThanks\n\nPhillip\n\n> +\t\t\t\topts.prefix = xstrfmt(\"refs/bisect/%s\", term_bad);\n>   \t\t\t\trefs_for_each_ref_ext(get_main_ref_store(the_repository),\n>   \t\t\t\t\t\t      show_reference, NULL, &opts);\n> -\t\t\t\topts.prefix = \"refs/bisect/good\";\n> +\t\t\t\topts.prefix = xstrfmt(\"refs/bisect/%s\", term_good);\n>   \t\t\t\trefs_for_each_ref_ext(get_main_ref_store(the_repository),\n>   \t\t\t\t\t\t      anti_reference, NULL, &opts);\n>   \t\t\t\tcontinue;\n> \n\n"},{"id":"539842","messageId":"888f8670-856a-4ce7-8177-da78ba4f0c8a@schlaraffenlan.de","threadId":"65340","inReplyTo":"d366fc82-efcc-46cb-9536-cd38b1fd18d4@gmail.com","subject":"Re: [PATCH v2 2/2] rev-parse: use selected alternate terms too look up refs","fromName":"Jonas Rebmann","fromEmail":"kernel@schlaraffenlan.de","sentAt":"2026-03-24T12:30:54Z","receivedAt":"2026-03-24T13:08:24Z","isPatch":true,"sender":{"key":"kernel@schlaraffenlan.de","avatar":null},"body":"Hi Phillip,\n\nThank you for your feedback, it will be addressed in v3.\n\nOn 24/03/2026 11.49, Phillip Wood wrote:\n> If we fail to read the terms because there is no bisect in progress\n> then term_bad and term_good will be NULL and so the next line will\n> segfault.\n\nMy understanding of read_bisect_terms() was that it never sets the terms\nto NULL, that if no bisect is in progress, .git/BISECT_TERMS does not\nexist, and the terms default to \"good\"/\"bad\" here in bisect.c:\n\n\tif (errno == ENOENT) {\n\t\tfree(*read_bad);\n\t\t*read_bad = xstrdup(\"bad\");\n\t\tfree(*read_good);\n\t\t*read_good = xstrdup(\"good\");\n\t\treturn;\n\t} else {\n\t\tdie_errno(_(\"could not read file '%s'\"), filename);\n\t}\n\nSo is a NULL-check really needed on caller end?\n\nRegards,\nJonas\n"},{"id":"539844","messageId":"87fr5pjq7n.fsf@gitster.g","threadId":"65340","inReplyTo":"20260323-bisect-terms-v2-2-8d6bdb2c9c7e@schlaraffenlan.de","subject":"Re: [PATCH v2 2/2] rev-parse: use selected alternate terms too look up refs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-24T13:45:48Z","receivedAt":"2026-03-24T13:45:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":null},"body":"Jonas Rebmann <kernel@schlaraffenlan.de> writes:\n\n>  #include \"abspath.h\"\n> +#include \"bisect.h\"\n>  #include \"config.h\"\n>  #include \"commit.h\"\n>  #include \"environment.h\"\n> @@ -940,11 +941,14 @@ int cmd_rev_parse(int argc,\n>  \t\t\t\tcontinue;\n>  \t\t\t}\n>  \t\t\tif (!strcmp(arg, \"--bisect\")) {\n> +\t\t\t\tchar *term_bad = NULL;\n> +\t\t\t\tchar *term_good = NULL;\n>  \t\t\t\tstruct refs_for_each_ref_options opts = { 0 };\n> -\t\t\t\topts.prefix = \"refs/bisect/bad\";\n> +\t\t\t\tread_bisect_terms(&term_bad, &term_good);\n> +\t\t\t\topts.prefix = xstrfmt(\"refs/bisect/%s\", term_bad);\n>  \t\t\t\trefs_for_each_ref_ext(get_main_ref_store(the_repository),\n>  \t\t\t\t\t\t      show_reference, NULL, &opts);\n> -\t\t\t\topts.prefix = \"refs/bisect/good\";\n> +\t\t\t\topts.prefix = xstrfmt(\"refs/bisect/%s\", term_good);\n>  \t\t\t\trefs_for_each_ref_ext(get_main_ref_store(the_repository),\n>  \t\t\t\t\t\t      anti_reference, NULL, &opts);\n\nAren't return values from two xstrfmt() calls leaking in this code?\n\n>  \t\t\t\tcontinue;\n"},{"id":"539845","messageId":"5e7aa1bb-1eb7-4b8c-8bd1-032be6a02a82@gmail.com","threadId":"65340","inReplyTo":"888f8670-856a-4ce7-8177-da78ba4f0c8a@schlaraffenlan.de","subject":"Re: [PATCH v2 2/2] rev-parse: use selected alternate terms too look up refs","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-24T14:11:13Z","receivedAt":"2026-03-24T14:11:20Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Jonas\n\nOn 24/03/2026 12:30, Jonas Rebmann wrote:\n> \n> On 24/03/2026 11.49, Phillip Wood wrote:\n>> If we fail to read the terms because there is no bisect in progress\n>> then term_bad and term_good will be NULL and so the next line will\n>> segfault.\n> \n> My understanding of read_bisect_terms() was that it never sets the terms\n> to NULL, that if no bisect is in progress, .git/BISECT_TERMS does not\n> exist, and the terms default to \"good\"/\"bad\" here in bisect.c:\n> \n>      if (errno == ENOENT) {\n>          free(*read_bad);\n>          *read_bad = xstrdup(\"bad\");\n>          free(*read_good);\n>          *read_good = xstrdup(\"good\");\n>          return;\n>      } else {\n>          die_errno(_(\"could not read file '%s'\"), filename);\n>      }\n> \n> So is a NULL-check really needed on caller end?\n\nOh, sorry I'd misread it, you're correct that we don't need a NULL \ncheck. Looking at the history of \"git rev-parse --bisect\" it was \nintroduced in ad3f9a71a82 (Add '--bisect' revision machinery argument, \n2009-10-27) which also added a '--bisect' option to \"git rev-list\". \nLooking at the implementation of that option I wonder if we should make \nrevision.c:for_each_bisect_ref() public and call it from \nbuiltin/rev-parse.c rather than building the refnames and calling \nrefs_for_each_ref_ext().\n\nThanks\n\nPhillip\n\n> \n> Regards,\n> Jonas\n\n"},{"id":"539861","messageId":"xmqq7br116b7.fsf@gitster.g","threadId":"65340","inReplyTo":"f8f7a220-c40a-480d-b0d0-abfcf5c83157@gmail.com","subject":"Re: [PATCH v2 1/2] bisect: use selected alternate terms in status output","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-24T17:33:00Z","receivedAt":"2026-03-24T17:33:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":null},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n> Hi Jonas\n>\n> On 23/03/2026 22:48, Jonas Rebmann wrote:\n>> \n>> diff --git a/builtin/bisect.c b/builtin/bisect.c\n>> index 4520e585d0..ee6a2c83b8 100644\n>> --- a/builtin/bisect.c\n>> +++ b/builtin/bisect.c\n>> @@ -465,13 +465,16 @@ static void bisect_print_status(const struct bisect_terms *terms)\n>>   \t\treturn;\n>>   \n>>   \tif (!state.nr_good && !state.nr_bad)\n>> -\t\tbisect_log_printf(_(\"status: waiting for both good and bad commits\\n\"));\n>> +\t\tbisect_log_printf(_(\"status: waiting for both %s and %s commits\\n\"),\n>> +\t\t\t\t  terms->term_good, terms->term_bad);\n>\n> If we're going to start using alternative terms it might be better to \n> enclose them in single quotes to make it clearer that we're referencing \n> the term names. Looking at the test below\n>\n> \t\"status: waiting for both 'term1' and 'term2' commits\"\n>\n> is clearer to me than\n>\n> \t\"status: waiting for both term1 and term2 commits\"\n\nExcellent.  I failed to consider this, but your reasoning makes\nperfect sense.  When we were limited to hardcoded good and bad,\nthey were clear enough without 'highlighting' with quotes, but that\nis no longer the case.\n\n>>   test_expect_success 'bisect start with one term1 and term2' '\n>>   \tgit bisect reset &&\n>> -\tgit bisect start --term-old term2 --term-new term1 &&\n>> -\tgit bisect term2 $HASH1 &&\n>> +\tgit bisect start --term-old term2 --term-new term1 >bisect_result &&\n>> +\tgrep \"status: waiting for both term2 and term1 commits\" bisect_result &&\n>\n> Using test_grep would make debugging test failures easier as, if it \n> fails, it prints a helpful diagnostic message.\n>\n> Thanks\n>\n> Phillip\n\nThanks for helping.\n"}]}