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 <artag...@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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to