{"thread":{"id":"65327","subject":"[PATCH v2] branch: reject --show-current with -v","startedAt":"2026-03-22T06:08:27Z","lastAt":"2026-03-22T21:43:03Z","messageCount":3,"participants":["K Jayatheerth","Phillip Wood","Junio C Hamano"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"539636","messageId":"20260322060705.53491-1-jayatheerthkulkarni2005@gmail.com","threadId":"65327","inReplyTo":null,"subject":"[PATCH v2] branch: reject --show-current with -v","fromName":"K Jayatheerth","fromEmail":"jayatheerthkulkarni2005@gmail.com","sentAt":"2026-03-22T06:07:05Z","receivedAt":"2026-03-22T06:08:27Z","isPatch":true,"sender":{"key":"jayatheerthkulkarni2005@gmail.com","avatar":"https://avatars.githubusercontent.com/u/148841023?v=4"},"body":"The --show-current option doesn't know how to handle verbose\nlogic. In such a case, we want the program to die when both flags\nare used together.\n\nAcked-by: Junio C Hamano \nSigned-off-by: K Jayatheerth \n---\nI only changed the title of the commit message.\nThe previous one (i.e remove unnecessary verbose flag) sounded\nlike we are nuking the whole verbose flag.\n\nThe acked by tag exists here because this was supposed to be queued,\nI am resending this to remind.\nThe originial discussion however is here:\nhttps://lore.kernel.org/git/20251226032804.596058-1-jayatheerthkulkarni2005@gmail.com/T/#u\n\n builtin/branch.c | 2 ++\n t/t3200-branch.sh | 4 ++++\n 2 files changed, 6 insertions(+)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex a1a43380d0..cab22e1538 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -861,6 +861,8 @@ int cmd_branch(int argc,\n \t\tret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);\n \t\tgoto out;\n \t} else if (show_current) {\n+\t\tif (filter.verbose)\n+\t\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--show-current\", \"-v\");\n \t\tprint_current_branch_name();\n \t\tret = 0;\n \t\tgoto out;\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex c58e505c43..5bb49de9c2 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -1716,4 +1716,8 @@ test_expect_success 'errors if given a bad branch name' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'git branch --show-current rejects -v' '\n+\ttest_must_fail git branch --show-current -v\n+'\n+\n test_done\n-- \n2.53.0\n\n"},{"id":"539651","messageId":"bf5b0901-fc44-4fbc-b66d-5eb062ccd324@gmail.com","threadId":"65327","inReplyTo":"20260322060705.53491-1-jayatheerthkulkarni2005@gmail.com","subject":"Re: [PATCH v2] branch: reject --show-current with -v","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2026-03-22T16:34:12Z","receivedAt":"2026-03-22T16:34:18Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 22/03/2026 06:07, K Jayatheerth wrote:\n> The --show-current option doesn't know how to handle verbose\n> logic. In such a case, we want the program to die when both flags\n> are used together.\n\nIs there any reason why --show-current couldn't be made to work with \n--verbose instead of dying?\n\n> \n> Acked-by: Junio C Hamano \n> Signed-off-by: K Jayatheerth \n> ---\n> I only changed the title of the commit message.\n> The previous one (i.e remove unnecessary verbose flag) sounded\n> like we are nuking the whole verbose flag.\n> \n> The acked by tag exists here because this was supposed to be queued,\n\nThat's not what the Acked-by: tag means - you should remove it\n\n> I am resending this to remind.\n> The originial discussion however is here:\n> https://lore.kernel.org/git/20251226032804.596058-1-jayatheerthkulkarni2005@gmail.com/T/#u\n> \n> builtin/branch.c | 2 ++\n> t/t3200-branch.sh | 4 ++++\n> 2 files changed, 6 insertions(+)\n> \n> diff --git a/builtin/branch.c b/builtin/branch.c\n> index a1a43380d0..cab22e1538 100644\n> --- a/builtin/branch.c\n> +++ b/builtin/branch.c\n> @@ -861,6 +861,8 @@ int cmd_branch(int argc,\n> \t\tret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);\n> \t\tgoto out;\n> \t} else if (show_current) {\n> +\t\tif (filter.verbose)\n> +\t\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--show-current\", \"-v\");\n\nIt would be better to use die_for_incompatible_opt2() here so that the \nmessage is consistent with other commands.\n\n> \t\tprint_current_branch_name();\n> \t\tret = 0;\n> \t\tgoto out;\n> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\n> index c58e505c43..5bb49de9c2 100755\n> --- a/t/t3200-branch.sh\n> +++ b/t/t3200-branch.sh\n> @@ -1716,4 +1716,8 @@ test_expect_success 'errors if given a bad branch name' '\n> \ttest_cmp expect actual\n> '\n> \n> +test_expect_success 'git branch --show-current rejects -v' '\n> +\ttest_must_fail git branch --show-current -v\n\nThis checks that the command fails but does not check _why_ in failed. \nIt would be better to redirect stderr to a file and use test_grep to \ncheck the error message matches what we expect as well checking that the \ncommand fails.\n\nThanks\n\nPhillip\n\n> +'\n> +\n> test_done\n\n"},{"id":"539675","messageId":"xmqqwlz37d7g.fsf@gitster.g","threadId":"65327","inReplyTo":"bf5b0901-fc44-4fbc-b66d-5eb062ccd324@gmail.com","subject":"Re: [PATCH v2] branch: reject --show-current with -v","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-22T21:42:59Z","receivedAt":"2026-03-22T21:43:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> On 22/03/2026 06:07, K Jayatheerth wrote:\n>> The --show-current option doesn't know how to handle verbose\n>> logic. In such a case, we want the program to die when both flags\n>> are used together.\n>\n> Is there any reason why --show-current couldn't be made to work with \n> --verbose instead of dying?\n>\n>> \n>> Acked-by: Junio C Hamano \n>> Signed-off-by: K Jayatheerth \n>> ---\n>> I only changed the title of the commit message.\n>> The previous one (i.e remove unnecessary verbose flag) sounded\n>> like we are nuking the whole verbose flag.\n>> \n>> The acked by tag exists here because this was supposed to be queued,\n>\n> That's not what the Acked-by: tag means - you should remove it\n\nI was about to write \"Yes, I do not even recall seeing this patch\",\nnoticed the \"v2\" label, but couldn't find corresponding v1. If the\nmessage had \"In-reply-to: \", that would\nnot have happened. FWIW, Sergey, who gave the \"why\" to start the\nthread, deserves much more credit than I would, I would have to say.\n\nI did say \"This is certainly an improvement over status quo.\", and\n\"is good enough for now\", so it is not completely unwarranted to\nhave my Ack there. Without any context it is hard to see, though.\n\nI _think_ what happened was later, after that \"Ack\" was given,\nSergey brought up a good point, \"why not consider what should -v\nmean in the context of --show-current before proceeding\", which is\nmore or less the same as your point above, so either I did not pick\nup the patch right there (expecting some form of response), or\npicked it up but discarded (after seeing no response came), or\nsomething like that.\n\nSince there are two people who independently questioned the wisdom\nof erroring out on \"-v\" before thinking things through to see if we\ncan come up with a good behaviour for \"--show-current -v\", perhaps\nwe should do so before proceeding.\n\nSo, I am not picking this iteration up, at least not yet.\n\n>> diff --git a/builtin/branch.c b/builtin/branch.c\n>> index a1a43380d0..cab22e1538 100644\n>> --- a/builtin/branch.c\n>> +++ b/builtin/branch.c\n>> @@ -861,6 +861,8 @@ int cmd_branch(int argc,\n>> \t\tret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);\n>> \t\tgoto out;\n>> \t} else if (show_current) {\n>> +\t\tif (filter.verbose)\n>> +\t\t\tdie(_(\"options '%s' and '%s' cannot be used together\"), \"--show-current\", \"-v\");\n>\n> It would be better to use die_for_incompatible_opt2() here so that the \n> message is consistent with other commands.\n\nYes. That would be much better.\n\n>> +test_expect_success 'git branch --show-current rejects -v' '\n>> +\ttest_must_fail git branch --show-current -v\n>\n> This checks that the command fails but does not check _why_ in failed. \n> It would be better to redirect stderr to a file and use test_grep to \n> check the error message matches what we expect as well checking that the \n> command fails.\n>\n> Thanks\n>\n> Phillip\n\nGood suggestion.\n"}]}