Re: [PATCH] checkout: updates to tracking report

From: Daniel Barkalow <barkalow@iabervon.org>
Date: Feb 21, 2008, 17:02 UTC
Message-ID: <alpine.LNX.1.00.0802211146410.19024@iabervon.org>
In-Reply-To: <7vk5kzynya.fsf_-_@gitster.siamese.dyndns.org>

On Wed, 20 Feb 2008, Junio C Hamano wrote:

Show 19 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> >>  	struct branch *branch = branch_get(NULL);
> >
> > Shouldn't this be branch_get(new->name)?
> 
> Perhaps.
> 
> > ...
> > While not uncommon colloquially, "behind of" is not grammatically
> > correct. "behind" is a preposition, so the "of" is redundant (it is
> > necessary in the top string because "ahead" is an adverb).
> 
> Thanks.
> 
> -- >8 --
> 
> Ask branch_get() for the new branch explicitly instead of
> letting it return a potentially stale information.

I think it would be good to update current_branch in remote.c when we change branches, just on principle, but we also might as well not depend on that here.

Show 11 quoted lines

> Tighten the logic to find the tracking branch to deal better
> with misconfigured repositories (i.e. branch.*.merge can exist
> but it may not have a refspec that fetches to .it)
> 
> Also fixes grammar in a message, as pointed out by Jeff King.
> 
> The function is about reporting and not automatically
> fast-forwarding to the upstream, so stop calling it
> "adjust-to".
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Acked-by: Daniel Barkalow <barkalow@iabervon.org>

Show 30 quoted lines

> ---
>  builtin-checkout.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/builtin-checkout.c b/builtin-checkout.c
> index f51b77a..19413eb 100644
> --- a/builtin-checkout.c
> +++ b/builtin-checkout.c
> @@ -290,7 +290,7 @@ static int merge_working_tree(struct checkout_opts *opts,
>  	return 0;
>  }
>  
> -static void adjust_to_tracking(struct branch_info *new, struct checkout_opts *opts)
> +static void report_tracking(struct branch_info *new, struct checkout_opts *opts)
>  {
>  	/*
>  	 * We have switched to a new branch; is it building on
> @@ -306,13 +306,13 @@ static void adjust_to_tracking(struct branch_info *new, struct checkout_opts *op
>  	int rev_argc;
>  	int num_ours, num_theirs;
>  	const char *remote_msg;
> -	struct branch *branch = branch_get(NULL);
> +	struct branch *branch = branch_get(new->name);
>  
>  	/*
>  	 * Nothing to report unless we are marked to build on top of
>  	 * somebody else.
>  	 */
> -	if (!branch || !branch->merge)
> +	if (!branch || !branch->merge || !branch->merge[0] || !branch->merge[0]->dst)

branch->merge[i] can't actually be NULL; all of the allocated space in the array gets stuff stored in it.

Show 24 quoted lines

>  		return;
>  
>  	/*
> @@ -370,7 +370,7 @@ static void adjust_to_tracking(struct branch_info *new, struct checkout_opts *op
>  		       remote_msg, base,
>  		       num_ours, (num_ours == 1) ? "" : "s");
>  	else if (!num_ours)
> -		printf("Your branch is behind of the tracked%s branch '%s' "
> +		printf("Your branch is behind the tracked%s branch '%s' "
>  		       "by %d commit%s,\n"
>  		       "and can be fast-forwarded.\n",
>  		       remote_msg, base,
> @@ -426,7 +426,7 @@ static void update_refs_for_switch(struct checkout_opts *opts,
>  	remove_branch_state();
>  	strbuf_release(&msg);
>  	if (!opts->quiet && (new->path || !strcmp(new->name, "HEAD")))
> -		adjust_to_tracking(new, opts);
> +		report_tracking(new, opts);
>  }
>  
>  static int switch_branches(struct checkout_opts *opts,
> -- 
> 1.5.4.2.261.g851a5
>