threads / patch / 24778

patchDo not display 'Switched to a new branch' when the branch existed

Subject: [PATCH] Do not display 'Switched to a new branch' when the branch existed

## tl;dr

10 messages between Aug 18, 2010 and Aug 25, 2010. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Knittl· Aug 18, 2010, 08:28 UTC · lore
From cc6410b89b85822aadc5a7843b7398209957e549 Mon Sep 17 00:00:00 2001
From: Tay Ray Chuan <rctay89@gmail.com>
Date: Thu, 24 Jun 2010 03:29:00 +0800
Subject: [PATCH] builtin/checkout: fix info message for `git checkout <branch>`

Since 02ac98374eefbe4a46d4b53a8a78057ad8ad39b7 `git checkout` would always display 'Switched to a new branch <branch>` even if the branch had already existed.

Signed-off-by: Daniel Knittl-Frank <knittl89+git@googlemail.com>
---

git checkout should only display 'Switched to a new branch <branch>' when it creates a new branch, not when it simply switches branches.

ps. I'm not sure about the style used in git for nested ternary statements (if they should even be used …)

 builtin/checkout.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
Show changes to builtin/checkout.c +3 −1
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 4ad7427..ed7cde1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -536,7 +536,9 @@ static void update_refs_for_switch(struct
checkout_opts *opts,
 					new->name);
 			else
 				fprintf(stderr, "Switched to%s branch '%s'\n",
-					opts->branch_exists ? " and reset" : " a new",
+					opts->branch_exists
+						? " and reset"
+						: opts->new_branch ? " a new" : "",
 					new->name);
 		}
 		if (old->path && old->name) {

-- 
1.7.1.574.g421e3
Knittl· Aug 18, 2010, 08:38 UTC · re: Knittl · lore

[PATCH re-roll] Do not display 'Switched to a new branch' when the branch existed

From 16f540c87f8c7b87692dfd488d507802ae975312 Mon Sep 17 00:00:00 2001
From: Daniel Knittl-Frank <knittl89+git@googlemail.com>
Date: Wed, 18 Aug 2010 10:35:42 +0200
Subject: [PATCH] builtin/checkout: fix info message for `git checkout <branch>`

Since 02ac98374eefbe4a46d4b53a8a78057ad8ad39b7 `git checkout` would always display 'Switched to a new branch <branch>` even if the branch had already existed.

Signed-off-by: Daniel Knittl-Frank <knittl89+git@googlemail.com>
---
stupid me, i forgot to reset author in re-used commit …
 builtin/checkout.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
Show changes to builtin/checkout.c +3 −1
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 4ad7427..ed7cde1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -536,7 +536,9 @@ static void update_refs_for_switch(struct
checkout_opts *opts,
 					new->name);
 			else
 				fprintf(stderr, "Switched to%s branch '%s'\n",
-					opts->branch_exists ? " and reset" : " a new",
+					opts->branch_exists
+						? " and reset"
+						: opts->new_branch ? " a new" : "",
 					new->name);
 		}
 		if (old->path && old->name) {

-- 
1.7.1.574.g421e3


-- 
typed with http://neo-layout.org
myFtPhp -- visit http://myftphp.sf.net -- v. 0.4.7 released!
Jonathan Nieder· Aug 18, 2010, 09:16 UTC · re: Knittl · lore

Re: [PATCH re-roll] Do not display 'Switched to a new branch' when the branch existed

Hi,
Warning: nitpicks coming.
Knittl wrote:
> From 16f540c87f8c7b87692dfd488d507802ae975312 Mon Sep 17 00:00:00 2001
> From: Daniel Knittl-Frank <knittl89+git@googlemail.com>
> Date: Wed, 18 Aug 2010 10:35:42 +0200
> Subject: [PATCH] builtin/checkout: fix info message for `git checkout <branch>`

On the git list, there are two formats often used for patches (see Documentation/SubmittingPatches for details): whole-message patches, which look like this:

	git checkout should only display 'Switched to a new branch <branch>'
	when it creates a new branch, not when it simply switches branches.
	This fixes a bug introduced by 02ac9837 (builtin/checkout:
	learn -B, 2010-06-24).
	Signed-off-by: Daniel Knittl-Frank <knittl89+git@googlemail.com>
	---
	comments of the moment
	 diffstat
	...
and "inline" patches, which look like this:
	comments of the moment
	-- 8< --
	Subject: patch subject
	patch rationale
	---
	 diffstat
	...
and sometimes get used when it is more natural for discussion.

The "From " line and so on output by "git format-patch" are for your mailer. Clarifying From:, Date:, and Subject: lines at the start of your message are allowed, though, and can be useful when forwarding patches from someone else.

Show 10 quoted lines
> +++ b/builtin/checkout.c
> @@ -536,7 +536,9 @@ static void update_refs_for_switch(struct
> checkout_opts *opts,
>  					new->name);
>  			else
>  				fprintf(stderr, "Switched to%s branch '%s'\n",
> -					opts->branch_exists ? " and reset" : " a new",
> +					opts->branch_exists
> +						? " and reset"
> +						: opts->new_branch ? " a new" : "",
Maybe it would be clearer to write
	opts->new_branch ? " a new"
		: opts->branch_exists ? " and reset"
		: "",
to emphasize that this is a list of condition/result pairs?
The functionality of your patch is obviously good.  Thanks.
Jonathan
Tay Ray Chuan· Aug 18, 2010, 13:39 UTC · re: Jonathan Nieder · lore

Re: [PATCH re-roll] Do not display 'Switched to a new branch' when the branch existed

Hi,
On Wed, Aug 18, 2010 at 5:16 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Hi,
Johnathan, thanks for the heads up.
Show 6 quoted lines
> [snip]
>
> The "From " line and so on output by "git format-patch" are for your
> mailer.  Clarifying From:, Date:, and Subject: lines at the start of
> your message are allowed, though, and can be useful when forwarding
> patches from someone else.

Knittl, I wonder how you generated this patch? Were you working on top of the "bad" commit?

Show 10 quoted lines
>> +++ b/builtin/checkout.c
>> @@ -536,7 +536,9 @@ static void update_refs_for_switch(struct
>> checkout_opts *opts,
>>                                       new->name);
>>                       else
>>                               fprintf(stderr, "Switched to%s branch '%s'\n",
>> -                                     opts->branch_exists ? " and reset" : " a new",
>> +                                     opts->branch_exists
>> +                                             ? " and reset"
>> +                                             : opts->new_branch ? " a new" : "",
Strange - I thought I had this sorted out. Thanks for spotting this.
Show 7 quoted lines
> Maybe it would be clearer to write
>
>        opts->new_branch ? " a new"
>                : opts->branch_exists ? " and reset"
>                : "",
>
> to emphasize that this is a list of condition/result pairs?
We could do with some parentheses - here's my take:
	fprintf(stderr, "Switched to%s branch '%s'\n",
		(opts->branch_exists ? " and reset" :
			(opts->new_branch ? " a new" : "")),
		new->name);
-- 
Cheers,
Ray Chuan
Knittl· Aug 24, 2010, 06:50 UTC · re: Tay Ray Chuan · lore

Re: [PATCH re-roll] Do not display 'Switched to a new branch' when the branch existed

sorry for the late reply, i hadn't had access to internet for the last week and as it turns i sent my response only to tay

On Wed, Aug 18, 2010 at 3:39 PM, Tay Ray Chuan <rctay89@gmail.com> wrote:
Show 12 quoted lines
> Hi,
>
> On Wed, Aug 18, 2010 at 5:16 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> [snip]
>
>> The "From " line and so on output by "git format-patch" are for your
>> mailer.  Clarifying From:, Date:, and Subject: lines at the start of
>> your message are allowed, though, and can be useful when forwarding
>> patches from someone else.
>
> Knittl, I wonder how you generated this patch? Were you working on top
> of the "bad" commit?

yes, i branched off of your bad commit (or rather the commit after your bad commit "fix detached head usage") and created the commit with git commit -c HEAD^ to have the same heading and similar wording without opening a second terminal to copy it over. so i accidentally sent the patch with your name as author, which i then fixed with git amend --reset-author

Show 12 quoted lines
>>> +++ b/builtin/checkout.c
>>> @@ -536,7 +536,9 @@ static void update_refs_for_switch(struct
>>> checkout_opts *opts,
>>>                                       new->name);
>>>                       else
>>>                               fprintf(stderr, "Switched to%s branch '%s'\n",
>>> -                                     opts->branch_exists ? " and reset" : " a new",
>>> +                                     opts->branch_exists
>>> +                                             ? " and reset"
>>> +                                             : opts->new_branch ? " a new" : "",
>
> Strange - I thought I had this sorted out. Thanks for spotting this.
i tested with next and pu and both tips had the same (confusing) message.
Show 14 quoted lines
>> Maybe it would be clearer to write
>>
>>        opts->new_branch ? " a new"
>>                : opts->branch_exists ? " and reset"
>>                : "",
>>
>> to emphasize that this is a list of condition/result pairs?
>
> We could do with some parentheses - here's my take:
>
>        fprintf(stderr, "Switched to%s branch '%s'\n",
>                (opts->branch_exists ? " and reset" :
>                        (opts->new_branch ? " a new" : "")),
>                new->name);
that's not really for me to decide, but i'm fine with either version
cheers
-- 
typed with http://neo-layout.org
myFtPhp -- visit http://myftphp.sf.net -- v. 0.4.7 released!
Tay Ray Chuan· Aug 24, 2010, 13:06 UTC · re: Knittl · lore

Re: [PATCH re-roll] Do not display 'Switched to a new branch' when the branch existed

Hi,
On Tue, Aug 24, 2010 at 2:50 PM, Knittl <knittl89@googlemail.com> wrote:
> sorry for the late reply, i hadn't had access to internet for the last
> week and as it turns i sent my response only to tay
just a heads-up - this has already been fixed since 09a0ec5 in master.
-- 
Cheers,
Ray Chuan
Knittl· Aug 25, 2010, 11:51 UTC · re: Tay Ray Chuan · lore

Re: [PATCH re-roll] Do not display 'Switched to a new branch' when the branch existed

On Tue, Aug 24, 2010 at 3:06 PM, Tay Ray Chuan <rctay89@gmail.com> wrote:
Show 7 quoted lines
> Hi,
>
> On Tue, Aug 24, 2010 at 2:50 PM, Knittl <knittl89@googlemail.com> wrote:
>> sorry for the late reply, i hadn't had access to internet for the last
>> week and as it turns i sent my response only to tay
>
> just a heads-up - this has already been fixed since 09a0ec5 in master.
oh. good :)
-- 
typed with http://neo-layout.org
myFtPhp -- visit http://myftphp.sf.net -- v. 0.4.7 released!
Junio C Hamano· Aug 18, 2010, 20:59 UTC · re: Jonathan Nieder · lore

Re: [PATCH re-roll] Do not display 'Switched to a new branch' when the branch existed

Jonathan Nieder <jrnieder@gmail.com> writes:
> The functionality of your patch is obviously good.  Thanks.

In what way is it good? I am especially worried about the word "reset" being confusing.

You are switching to a new context to work on something else, so I don't necessarily think it is confusing that the word "new branch" in this message does not mean "a branch that did not exist before this operation (i.e. a newly created branch)."

Junio C Hamano· Aug 18, 2010, 23:38 UTC · re: Junio C Hamano · lore

Re: [PATCH re-roll] Do not display 'Switched to a new branch' when the branch existed

Junio C Hamano <gitster@pobox.com> writes:
Show 11 quoted lines
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> The functionality of your patch is obviously good.  Thanks.
>
> In what way is it good?  I am especially worried about the word "reset"
> being confusing.
>
> You are switching to a new context to work on something else, so I don't
> necessarily think it is confusing that the word "new branch" in this
> message does not mean "a branch that did not exist before this operation
> (i.e. a newly created branch)."

Ahh, please disregard the above; I somehow failed to see that this is only in the "-b/-B" codepath. Sorry for the noise.

Tay Ray Chuan· Aug 19, 2010, 03:21 UTC · lore

Re: [PATCH re-roll] Do not display 'Switched to a new branch' when the branch existed

Hi,

oops, seems like you dropped everyone from the Cc list, including the mailing list. Try using the "Reply to all" next time.

On Wed, Aug 18, 2010 at 9:56 PM, Knittl <knittl89@googlemail.com> wrote:
Show 7 quoted lines
> [snip
> yes, i branched off of your bad commit (or rather the commit after
> your bad commit "fix detached head usage") and created the commit with
> git commit -c HEAD^ to have the same heading and similar wording
> without opening a second terminal to copy it over. so i accidentally
> sent the patch with your name as author, which i then fixed with git
> amend --reset-author

Why copy over the old commit message? You should be writing one that fits what you're did, not what *I* did.

-- 
Cheers,
Ray Chuan

← back to recent threads