[PATCH 3/5] sha1_name.c: simplify @-parsing in get_sha1_basic()

[Ramkumar Ramachandra <artagnon@gmail.com>]

Presently, the logic for @-parsing is horribly convoluted.  Prove that
it is very straightforward by laying out the three cases (@{N},
@{u[upstream], and @{-N}) and explaining how each case should be
handled in comments.  All tests pass, and no functional changes have
been introduced.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 sha1_name.c | 65 ++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 20 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index be1d12c..f30e344 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -463,9 +463,26 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 		 */
 		for (at = len - 4; at >= 0; at--) {
 			if (str[at] == '@' && str[at+1] == '{') {
+				/* Set reflog_len only if we
+				 * don't see a @{u[pstream]}.  The
+				 * @{N} and @{-N} forms have to do
+				 * with reflog digging.
+				 */
+
+				/* Setting len to at means that we are
+				 * only going to process the part
+				 * before the @ until we reach "if
+				 * (reflog)" at the end of the
+				 * function.  That is only applicable
+				 * in the @{N} case; in the @{-N} and
+				 * @{u[pstream]} cases, we will run it
+				 * through interpret_branch_name().
+				 */
+
 				if (!upstream_mark(str + at, len - at)) {
 					reflog_len = (len-1) - (at+2);
-					len = at;
+					if (str[at + 2] != '-')
+						len = at;
 				}
 				break;
 			}
@@ -476,22 +493,34 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 	if (len && ambiguous_path(str, len))
 		return -1;
 
-	if (!len && reflog_len) {
-		struct strbuf buf = STRBUF_INIT;
-		int ret;
-		/* try the @{-N} syntax for n-th checkout */
-		ret = interpret_branch_name(str+at, &buf);
-		if (ret > 0) {
-			/* substitute this branch name and restart */
-			return get_sha1_1(buf.buf, buf.len, sha1, 0);
-		} else if (ret == 0) {
-			return -1;
+	if (reflog_len) {
+		if (!len)
+			/* In the @{N} case where there's nothing
+			 * before the @.
+			 */
+			refs_found = dwim_log("HEAD", 4, sha1, &real_ref);
+		else {
+			/* The @{N} case where there is something
+			 * before the @ for dwim_log to figure out,
+			 * and the @{-N} case.
+			 */
+			refs_found = dwim_log(str, len, sha1, &real_ref);
+
+			if (!refs_found && str[2] == '-') {
+				/* The @{-N} case that resolves to a
+				 * detached HEAD (ie. not a ref)
+				 */
+				struct strbuf buf = STRBUF_INIT;
+				if (interpret_branch_name(str, &buf) > 0) {
+					get_sha1_hex(buf.buf, sha1);
+					refs_found = 1;
+					reflog_len = 0;
+				}
+				strbuf_release(&buf);
+			}
 		}
-		/* allow "@{...}" to mean the current branch reflog */
-		refs_found = dwim_ref("HEAD", 4, sha1, &real_ref);
-	} else if (reflog_len)
-		refs_found = dwim_log(str, len, sha1, &real_ref);
-	else
+	} else
+		/* The @{u[pstream]} case */
 		refs_found = dwim_ref(str, len, sha1, &real_ref);
 
 	if (!refs_found)
@@ -506,10 +535,6 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 		unsigned long co_time;
 		int co_tz, co_cnt;
 
-		/* a @{-N} placed anywhere except the start is an error */
-		if (str[at+2] == '-')
-			return -1;
-
 		/* Is it asking for N-th entry, or approxidate? */
 		for (i = nth = 0; 0 <= nth && i < reflog_len; i++) {
 			char ch = str[at+2+i];
-- 
1.8.3.rc0.24.g6456091