v2branch: reject --show-current with -v

3 messages between Mar 22, 2026 and Mar 22, 2026, from K Jayatheerth, Phillip Wood, Junio C Hamano.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

K JayatheerthMar 22, 2026, 06:07 UTC on lore

The --show-current option doesn't know how to handle verbose logic. In such a case, we want the program to die when both flags are used together.

Acked-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
---
I only changed the title of the commit message.
The previous one (i.e remove unnecessary verbose flag) sounded
like we are nuking the whole verbose flag.

The acked by tag exists here because this was supposed to be queued, I am resending this to remind. The originial discussion however is here: https://lore.kernel.org/git/20251226032804.596058-1-jayatheerthkulkarni2005@gmail.com/T/#u

 builtin/branch.c  | 2 ++
 t/t3200-branch.sh | 4 ++++
 2 files changed, 6 insertions(+)

Show changes to 2 files +6 −0

builtin/branch.c, t/t3200-branch.sh

diff --git a/builtin/branch.c b/builtin/branch.c
index a1a43380d0..cab22e1538 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -861,6 +861,8 @@ int cmd_branch(int argc,
 		ret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);
 		goto out;
 	} else if (show_current) {
+		if (filter.verbose)
+			die(_("options '%s' and '%s' cannot be used together"), "--show-current", "-v");
 		print_current_branch_name();
 		ret = 0;
 		goto out;
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index c58e505c43..5bb49de9c2 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -1716,4 +1716,8 @@ test_expect_success 'errors if given a bad branch name' '
 	test_cmp expect actual
 '
 
+test_expect_success 'git branch --show-current rejects -v' '
+	test_must_fail git branch --show-current -v
+'
+
 test_done

-- 
2.53.0

Phillip WoodMar 22, 2026, 16:34 UTC in reply to K Jayatheerth on lore

Re: [PATCH v2] branch: reject --show-current with -v

On 22/03/2026 06:07, K Jayatheerth wrote:

> The --show-current option doesn't know how to handle verbose
> logic. In such a case, we want the program to die when both flags
> are used together.

Is there any reason why --show-current couldn't be made to work with --verbose instead of dying?

Show 9 quoted lines

> 
> Acked-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
> ---
> I only changed the title of the commit message.
> The previous one (i.e remove unnecessary verbose flag) sounded
> like we are nuking the whole verbose flag.
> 
> The acked by tag exists here because this was supposed to be queued,

That's not what the Acked-by: tag means - you should remove it

Show 18 quoted lines

> I am resending this to remind.
> The originial discussion however is here:
> https://lore.kernel.org/git/20251226032804.596058-1-jayatheerthkulkarni2005@gmail.com/T/#u
> 
>   builtin/branch.c  | 2 ++
>   t/t3200-branch.sh | 4 ++++
>   2 files changed, 6 insertions(+)
> 
> diff --git a/builtin/branch.c b/builtin/branch.c
> index a1a43380d0..cab22e1538 100644
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -861,6 +861,8 @@ int cmd_branch(int argc,
>   		ret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);
>   		goto out;
>   	} else if (show_current) {
> +		if (filter.verbose)
> +			die(_("options '%s' and '%s' cannot be used together"), "--show-current", "-v");

It would be better to use die_for_incompatible_opt2() here so that the message is consistent with other commands.

Show 13 quoted lines

>   		print_current_branch_name();
>   		ret = 0;
>   		goto out;
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index c58e505c43..5bb49de9c2 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -1716,4 +1716,8 @@ test_expect_success 'errors if given a bad branch name' '
>   	test_cmp expect actual
>   '
>   
> +test_expect_success 'git branch --show-current rejects -v' '
> +	test_must_fail git branch --show-current -v

This checks that the command fails but does not check _why_ in failed. It would be better to redirect stderr to a file and use test_grep to check the error message matches what we expect as well checking that the command fails.

Thanks

Phillip

> +'
> +
>   test_done

Junio C HamanoMar 22, 2026, 21:42 UTC in reply to Phillip Wood on lore

Re: [PATCH v2] branch: reject --show-current with -v

Phillip Wood <phillip.wood123@gmail.com> writes:

Show 19 quoted lines

> On 22/03/2026 06:07, K Jayatheerth wrote:
>> The --show-current option doesn't know how to handle verbose
>> logic. In such a case, we want the program to die when both flags
>> are used together.
>
> Is there any reason why --show-current couldn't be made to work with 
> --verbose instead of dying?
>
>> 
>> Acked-by: Junio C Hamano <gitster@pobox.com>
>> Signed-off-by: K Jayatheerth <jayatheerthkulkarni2005@gmail.com>
>> ---
>> I only changed the title of the commit message.
>> The previous one (i.e remove unnecessary verbose flag) sounded
>> like we are nuking the whole verbose flag.
>> 
>> The acked by tag exists here because this was supposed to be queued,
>
> That's not what the Acked-by: tag means - you should remove it

I was about to write "Yes, I do not even recall seeing this patch", noticed the "v2" label, but couldn't find corresponding v1. If the message had "In-reply-to: <xmqqldiplvyd.fsf@gitster.g>", that would not have happened. FWIW, Sergey, who gave the "why" to start the thread, deserves much more credit than I would, I would have to say.

I did say "This is certainly an improvement over status quo.", and "is good enough for now", so it is not completely unwarranted to have my Ack there. Without any context it is hard to see, though.

I _think_ what happened was later, after that "Ack" was given, Sergey brought up a good point, "why not consider what should -v mean in the context of --show-current before proceeding", which is more or less the same as your point above, so either I did not pick up the patch right there (expecting some form of response), or picked it up but discarded (after seeing no response came), or something like that.

Since there are two people who independently questioned the wisdom of erroring out on "-v" before thinking things through to see if we can come up with a good behaviour for "--show-current -v", perhaps we should do so before proceeding.

So, I am not picking this iteration up, at least not yet.

Show 13 quoted lines

>> diff --git a/builtin/branch.c b/builtin/branch.c
>> index a1a43380d0..cab22e1538 100644
>> --- a/builtin/branch.c
>> +++ b/builtin/branch.c
>> @@ -861,6 +861,8 @@ int cmd_branch(int argc,
>>   		ret = delete_branches(argc, argv, delete > 1, filter.kind, quiet);
>>   		goto out;
>>   	} else if (show_current) {
>> +		if (filter.verbose)
>> +			die(_("options '%s' and '%s' cannot be used together"), "--show-current", "-v");
>
> It would be better to use die_for_incompatible_opt2() here so that the 
> message is consistent with other commands.

Yes.  That would be much better.

Show 11 quoted lines

>> +test_expect_success 'git branch --show-current rejects -v' '
>> +	test_must_fail git branch --show-current -v
>
> This checks that the command fails but does not check _why_ in failed. 
> It would be better to redirect stderr to a file and use test_grep to 
> check the error message matches what we expect as well checking that the 
> command fails.
>
> Thanks
>
> Phillip

Good suggestion.