Re: [PATCH v2] Introduce <branch>@{upstream} as shortcut to the tracked branch
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Sep 10, 2009, 16:18 UTC
- 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:
Show 41 quoted lines
> 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