Re: [PATCH/RFC v4 3/5] sha1_name: support @{-N} syntax in get_sha1()

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 17, 2009, 19:37 UTC
Message-ID: <7v63kd7mgv.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1232208597-29249-4-git-send-email-trast@student.ethz.ch>

Thomas Rast <trast@student.ethz.ch> writes:

Show 21 quoted lines

> Let get_sha1() parse the @{-N} syntax, with docs and tests.
>
> Note that while @{-1}^2, @{-2}~5 and such are supported, @{-1}@{1} is
> currently not allowed.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ...
> @@ -324,6 +326,16 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
>  		return -1;
>  
>  	if (!len && reflog_len) {
> +		struct strbuf buf = STRBUF_INIT;
> +		int ret;
> +		/* try the @{-N} syntax for n-th checkout */
> +		ret = interpret_nth_last_branch(str+at, &buf);
> +		if (ret > 0) {
> +			/* substitute this branch name and restart */
> +			return get_sha1_1(buf.buf, buf.len, sha1);
> +		} else if (ret == 0) {
> +			return -1;
> +		}

What are the possible failure cases, and what do we want to tell the end-user?

 - You asked for 3rd but there weren't that many switches yet, and ask
   "git rev-parse --verify @{-3}".

   Are we Ok with "fatal: Needed a single revision" from rev-parse?  Do we
   want to show "fatal: @{-3}: not that many branch switches yet"?

   What happens to "git checkout @{-3}" in this case?  Having checkout say
   "fatal: invalid reference: @{-3}" would be fine in this case, I think.

 - You try "git checkout @{-3}", you were on "frotz" branch back then, but
   the branch does not exist anymore.

   I think you will get "fatal: invalid reference: frotz" from checkout,
   which should be fine.

There also is a case where nth_last_branch() may find something that is not a branch (e.g. "git checkout HEAD^"), but I am hoping we can label that as a bug in nth_last_branch() and fix it later.