{"thread":{"id":"33698","subject":"[PATCH] sha1_name: reorganize get_sha1_basic()","startedAt":"2013-05-02T00:49:51Z","lastAt":"2013-05-02T17:52:38Z","messageCount":6,"participants":["Felipe Contreras","Ramkumar Ramachandra","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"216222","messageId":"1367455791-30091-1-git-send-email-felipe.contreras@gmail.com","threadId":"33698","inReplyTo":null,"subject":"[PATCH] sha1_name: reorganize get_sha1_basic()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-02T00:49:51Z","receivedAt":"2013-05-02T00:49:51Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Through the years the functionality to handle @{-N} and @{u} has moved\naround the code, and as a result, code that once made sense, doesn't any\nmore.\n\nThere is no need to call this function recursively with the branch of\n@{-N} substituted because dwim_{ref,log} already replaces it.\n\nHowever, there's one corner-case where @{-N} resolves to a detached\nHEAD, in which case we wouldn't get any ref back.\n\nSo we parse the nth-prior manually, and deal with it depending on\nweather it's a SHA-1, or a ref.\n\nSigned-off-by: Felipe Contreras \n---\n sha1_name.c | 28 ++++++++++++++++------------\n 1 file changed, 16 insertions(+), 12 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 3820f28..110afbb 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -431,6 +431,7 @@ static inline int upstream_mark(const char *string, int len)\n }\n \n static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned lookup_flags);\n+static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf);\n \n static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n {\n@@ -438,6 +439,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n \tchar *real_ref = NULL;\n \tint refs_found = 0;\n \tint at, reflog_len;\n+\tstruct strbuf buf = STRBUF_INIT;\n \n \tif (len == 40 && !get_sha1_hex(str, sha1))\n \t\treturn 0;\n@@ -447,6 +449,9 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n \tif (len && str[len-1] == '}') {\n \t\tfor (at = len-2; at >= 0; at--) {\n \t\t\tif (str[at] == '@' && str[at+1] == '{') {\n+\t\t\t\tif (at == 0 && str[2] == '-')\n+\t\t\t\t\t/* @{-N} syntax */\n+\t\t\t\t\tcontinue;\n \t\t\t\tif (!upstream_mark(str + at, len - at)) {\n \t\t\t\t\treflog_len = (len-1) - (at+2);\n \t\t\t\t\tlen = at;\n@@ -460,20 +465,19 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n \tif (len && ambiguous_path(str, len))\n \t\treturn -1;\n \n-\tif (!len && reflog_len) {\n-\t\tstruct strbuf buf = STRBUF_INIT;\n-\t\tint ret;\n-\t\t/* try the @{-N} syntax for n-th checkout */\n-\t\tret = interpret_branch_name(str+at, &buf);\n-\t\tif (ret > 0) {\n-\t\t\t/* substitute this branch name and restart */\n-\t\t\treturn get_sha1_1(buf.buf, buf.len, sha1, 0);\n-\t\t} else if (ret == 0) {\n-\t\t\treturn -1;\n-\t\t}\n+\tif (interpret_nth_prior_checkout(str, &buf) > 0) {\n+\t\tint detached;\n+\n+\t\tdetached = (buf.len == 40 && !get_sha1_hex(buf.buf, sha1));\n+\t\tstrbuf_release(&buf);\n+\t\tif (detached)\n+\t\t\treturn 0;\n+\t}\n+\n+\tif (!len && reflog_len)\n \t\t/* allow \"@{...}\" to mean the current branch reflog */\n \t\trefs_found = dwim_ref(\"HEAD\", 4, sha1, &real_ref);\n-\t} else if (reflog_len)\n+\telse if (reflog_len)\n \t\trefs_found = dwim_log(str, len, sha1, &real_ref);\n \telse\n \t\trefs_found = dwim_ref(str, len, sha1, &real_ref);\n-- \n1.8.3.rc0.399.gc96a135\n"},{"id":"216224","messageId":"CAMP44s2_riedK8gsHELRgQXd+7XWaTC6ErhPqX3xqWQoc5pJKQ@mail.gmail.com","threadId":"33698","inReplyTo":"1367455791-30091-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] sha1_name: reorganize get_sha1_basic()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-02T00:53:41Z","receivedAt":"2013-05-02T00:53:41Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 1, 2013 at 7:49 PM, Felipe Contreras\n wrote:\n> Through the years the functionality to handle @{-N} and @{u} has moved\n> around the code, and as a result, code that once made sense, doesn't any\n> more.\n>\n> There is no need to call this function recursively with the branch of\n> @{-N} substituted because dwim_{ref,log} already replaces it.\n>\n> However, there's one corner-case where @{-N} resolves to a detached\n> HEAD, in which case we wouldn't get any ref back.\n>\n> So we parse the nth-prior manually, and deal with it depending on\n> weather it's a SHA-1, or a ref.\n\nForgot to mention; inspired by a patch from Ramkumar Ramachandra.\n\n-- \nFelipe Contreras\n"},{"id":"216225","messageId":"CAMP44s0_9aXu0eZ0w_dkkoR4No3ZzKVsdzAb_E1wud-ZpOW6dg@mail.gmail.com","threadId":"33698","inReplyTo":"1367455791-30091-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] sha1_name: reorganize get_sha1_basic()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-02T01:00:59Z","receivedAt":"2013-05-02T01:00:59Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 1, 2013 at 7:49 PM, Felipe Contreras\n wrote:\n> Through the years the functionality to handle @{-N} and @{u} has moved\n> around the code, and as a result, code that once made sense, doesn't any\n> more.\n>\n> There is no need to call this function recursively with the branch of\n> @{-N} substituted because dwim_{ref,log} already replaces it.\n>\n> However, there's one corner-case where @{-N} resolves to a detached\n> HEAD, in which case we wouldn't get any ref back.\n>\n> So we parse the nth-prior manually, and deal with it depending on\n> weather it's a SHA-1, or a ref.\n>\n> Signed-off-by: Felipe Contreras \n> ---\n> sha1_name.c | 28 ++++++++++++++++------------\n> 1 file changed, 16 insertions(+), 12 deletions(-)\n>\n> diff --git a/sha1_name.c b/sha1_name.c\n> index 3820f28..110afbb 100644\n> --- a/sha1_name.c\n> +++ b/sha1_name.c\n> @@ -431,6 +431,7 @@ static inline int upstream_mark(const char *string, int len)\n> }\n>\n> static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned lookup_flags);\n> +static int interpret_nth_prior_checkout(const char *name, struct strbuf *buf);\n>\n> static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n> {\n> @@ -438,6 +439,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n> char *real_ref = NULL;\n> int refs_found = 0;\n> int at, reflog_len;\n> + struct strbuf buf = STRBUF_INIT;\n>\n> if (len == 40 && !get_sha1_hex(str, sha1))\n> return 0;\n> @@ -447,6 +449,9 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n> if (len && str[len-1] == '}') {\n> for (at = len-2; at >= 0; at--) {\n> if (str[at] == '@' && str[at+1] == '{') {\n> + if (at == 0 && str[2] == '-')\n> + /* @{-N} syntax */\n> + continue;\n> if (!upstream_mark(str + at, len - at)) {\n> reflog_len = (len-1) - (at+2);\n> len = at;\n> @@ -460,20 +465,19 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n> if (len && ambiguous_path(str, len))\n> return -1;\n>\n> - if (!len && reflog_len) {\n> - struct strbuf buf = STRBUF_INIT;\n> - int ret;\n> - /* try the @{-N} syntax for n-th checkout */\n> - ret = interpret_branch_name(str+at, &buf);\n> - if (ret > 0) {\n> - /* substitute this branch name and restart */\n> - return get_sha1_1(buf.buf, buf.len, sha1, 0);\n> - } else if (ret == 0) {\n> - return -1;\n> - }\n> + if (interpret_nth_prior_checkout(str, &buf) > 0) {\n> + int detached;\n> +\n> + detached = (buf.len == 40 && !get_sha1_hex(buf.buf, sha1));\n> + strbuf_release(&buf);\n> + if (detached)\n> + return 0;\n> + }\n> +\n> + if (!len && reflog_len)\n> /* allow \"@{...}\" to mean the current branch reflog */\n> refs_found = dwim_ref(\"HEAD\", 4, sha1, &real_ref);\n> - } else if (reflog_len)\n> + else if (reflog_len)\n> refs_found = dwim_log(str, len, sha1, &real_ref);\n> else\n> refs_found = dwim_ref(str, len, sha1, &real_ref);\n> --\n\nThis might make more sense:\n\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -438,8 +438,7 @@ static int get_sha1_basic(const char *str, int\nlen, unsigned char *sha1)\n static const char *warn_msg = \"refname '%.*s' is ambiguous.\";\n char *real_ref = NULL;\n int refs_found = 0;\n- int at, reflog_len;\n- struct strbuf buf = STRBUF_INIT;\n+ int at, reflog_len, nth_prior = 0;\n\n if (len == 40 && !get_sha1_hex(str, sha1))\n return 0;\n@@ -449,9 +448,10 @@ static int get_sha1_basic(const char *str, int\nlen, unsigned char *sha1)\n if (len && str[len-1] == '}') {\n for (at = len-2; at >= 0; at--) {\n if (str[at] == '@' && str[at+1] == '{') {\n- if (at == 0 && str[2] == '-')\n- /* @{-N} syntax */\n+ if (at == 0 && str[2] == '-') {\n+ nth_prior = 1;\n continue;\n+ }\n if (!upstream_mark(str + at, len - at)) {\n reflog_len = (len-1) - (at+2);\n len = at;\n@@ -465,9 +465,12 @@ static int get_sha1_basic(const char *str, int\nlen, unsigned char *sha1)\n if (len && ambiguous_path(str, len))\n return -1;\n\n- if (interpret_nth_prior_checkout(str, &buf) > 0) {\n+ if (nth_prior) {\n+ struct strbuf buf = STRBUF_INIT;\n int detached;\n\n+ interpret_nth_prior_checkout(str, &buf);\n+\n detached = (buf.len == 40 && !get_sha1_hex(buf.buf, sha1));\n strbuf_release(&buf);\n if (detached)\n\n-- \nFelipe Contreras\n"},{"id":"216237","messageId":"CALkWK0=qknGMuHN-zJzLXDNRLFdaBssccnkXbXbM8SZ+ORKiZA@mail.gmail.com","threadId":"33698","inReplyTo":"1367455791-30091-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] sha1_name: reorganize get_sha1_basic()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-02T09:40:48Z","receivedAt":"2013-05-02T09:40:48Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> @@ -447,6 +449,9 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n> if (len && str[len-1] == '}') {\n> for (at = len-2; at >= 0; at--) {\n> if (str[at] == '@' && str[at+1] == '{') {\n> + if (at == 0 && str[2] == '-')\n> + /* @{-N} syntax */\n> + continue;\n\nNicely done. Thanks.\n"},{"id":"216243","messageId":"CACsJy8CWGJZByedxhzuU8OkPPBc0XmfZqE_e_ehmt55t-tHmsQ@mail.gmail.com","threadId":"33698","inReplyTo":"1367455791-30091-1-git-send-email-felipe.contreras@gmail.com","subject":"Re: [PATCH] sha1_name: reorganize get_sha1_basic()","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2013-05-02T10:19:44Z","receivedAt":"2013-05-02T10:19:44Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, May 2, 2013 at 7:49 AM, Felipe Contreras\n wrote:\n> Through the years the functionality to handle @{-N} and @{u} has moved\n> around the code, and as a result, code that once made sense, doesn't any\n> more.\n>\n> There is no need to call this function recursively with the branch of\n> @{-N} substituted because dwim_{ref,log} already replaces it.\n>\n> However, there's one corner-case where @{-N} resolves to a detached\n> HEAD, in which case we wouldn't get any ref back.\n>\n> So we parse the nth-prior manually, and deal with it depending on\n> weather it's a SHA-1, or a ref.\n\nIf you are brave, reorganize the whole extended sha-1 parsing code. I\nthink the right-to-left parsing is confusing, not to mention it has to\ntreat \":path\" differently. But may be that's too big work for little\ngain.\n--\nDuy\n"},{"id":"216294","messageId":"CAMP44s2HPRpbot2YhTmRNRmGj_1-+BdnjW9kTVXwq3Ni0YiObA@mail.gmail.com","threadId":"33698","inReplyTo":"CACsJy8CWGJZByedxhzuU8OkPPBc0XmfZqE_e_ehmt55t-tHmsQ@mail.gmail.com","subject":"Re: [PATCH] sha1_name: reorganize get_sha1_basic()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-02T17:52:38Z","receivedAt":"2013-05-02T17:52:38Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, May 2, 2013 at 5:19 AM, Duy Nguyen wrote:\n> On Thu, May 2, 2013 at 7:49 AM, Felipe Contreras\n> wrote:\n>> Through the years the functionality to handle @{-N} and @{u} has moved\n>> around the code, and as a result, code that once made sense, doesn't any\n>> more.\n>>\n>> There is no need to call this function recursively with the branch of\n>> @{-N} substituted because dwim_{ref,log} already replaces it.\n>>\n>> However, there's one corner-case where @{-N} resolves to a detached\n>> HEAD, in which case we wouldn't get any ref back.\n>>\n>> So we parse the nth-prior manually, and deal with it depending on\n>> weather it's a SHA-1, or a ref.\n>\n> If you are brave, reorganize the whole extended sha-1 parsing code. I\n> think the right-to-left parsing is confusing, not to mention it has to\n> treat \":path\" differently. But may be that's too big work for little\n> gain.\n\nYeah, it took me a while to figure it was the other way around to what\nI thought was natural. Maybe after the dust with '@' is settled.\n\nFTR. This doesn't work @{u}@{u}.\n\n-- \nFelipe Contreras\n"}]}