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

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Aug 18, 2010, 09:16 UTC
Message-ID: <20100818091603.GA6263@burratino>
In-Reply-To: <AANLkTi=3z9gJdT8LL3NANFyppUjvOVcrszjf5J5zAKPe@mail.gmail.com>

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