Re: [PATCH/RFC v4 3/5] sha1_name: support @{-N} syntax in get_sha1()
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:58
Thomas Rast [off-list ref] writes:
quoted hunk
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 <redacted> ...@@ -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.