From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Thu, 10 Sep 2009 16:18:46 GMT
Subject: Re: [PATCH v2] Introduce <branch>@{upstream} as shortcut to the tracked branch
Message-ID: <alpine.DEB.1.00.0909101817521.8306@pacific.mpi-cbg.de>
In-Reply-To: <20090910155547.GA12409@coredump.intra.peff.net>

Hi,

On Thu, 10 Sep 2009, Jeff King wrote:

> I wrote:
> 
> > +	ret = tracked_suffix(*string, *len);
> > +	if (ret) {
> > +		char *ref = xstrndup(*string, *len - ret);
> > +		struct branch *tracking = branch_get(*ref ? ref : NULL);
> > +
> > +		free(ref);
> > +		if (!tracking)
> > +			die ("No tracking branch found for '%s'", ref);
> > +		if (tracking->merge && tracking->merge[0]->dst) {
> > +			*string = xstrdup(tracking->merge[0]->dst);
> > +			*len = strlen(*string);
> > +			return (char *)*string;
> > +		}
> > +	}
> > +
> 
> I don't think it is a good idea to die for !tracking, but not for
> !tracking->merge. That leads to inconsistent user-visible results:
> 
>   $ git checkout HEAD^0
>   $ git rev-parse HEAD@{u}
>   fatal: No tracking branch found for 'HEAD'
>   $ git rev-parse bogus@{u}
>   bogus@{u}
>   fatal: ambiguous argument 'bogus@{u}': unknown revision or path not in the working tree.
>   Use '--' to separate paths from revisions
> 
> Shouldn't both cases say the same thing?
> 
> Also, your die message has two problems:
> 
>  1. It looks at ref immediately after it is free'd, spewing junk.
> 
>  2. Ref can be the empty string, which gives you the ugly:
> 
>        fatal: No tracking branch found for ''
> 
>     Should we munge that into HEAD (or "the current branch") for the
>     user?

All true, but I cannot take care of it today.

Ciao,
Dscho