Re: [PATCH v2] Introduce <branch>@{upstream} as shortcut to the tracked branch

[Jeff King <peff@peff.net>]

On Thu, Sep 10, 2009 at 05:25:57PM +0200, Johannes Schindelin wrote:

> 	Changes since v1:
> 	- changed to @{upstream} (and @{u})

Hmm. After applying v2, I accidentally tried "git rev-parse @{t}" and
was surprised to find that it worked! The problem, of course, is that we
saw that it was not one of our keywords and therefore dumped it into
approxidate, which happily converted it into the current time, and
provided me with HEAD@{now}.

Which is sad, because it means "HEAD@{usptream}" will silently produce
incorrect results.

I don't see a way around it, though, short of tightening approxidate's
parsing. Maybe it could keep a flag for "I noticed _anything_ of value
in this string", and we could barf if it isn't set. That would still
allow arbitrary stuff like:

  the 6th of july

and keep Linus' favorite

  I ate 6 hot dogs in July.

but disallow the most obviously wrong dates like:

  t
  usptream
  total bogosity

> +	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?

-Peff