{"thread":{"id":"12696","subject":"[PATCH] git-rev-parse.txt: clarify meaning of rev~ and rev~0.","startedAt":"2008-03-14T17:20:06Z","lastAt":"2008-03-14T20:13:23Z","messageCount":4,"participants":["Sergei Organov","Linus Torvalds","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"72118","messageId":"87wso5mcs7.fsf@osv.gnss.ru","threadId":"12696","inReplyTo":null,"subject":"[PATCH] git-rev-parse.txt: clarify meaning of rev~ and rev~0.","fromName":"Sergei Organov","fromEmail":"osv@javad.com","sentAt":"2008-03-14T17:20:06Z","receivedAt":"2008-03-14T17:20:06Z","isPatch":true,"sender":{"key":"osv@javad.com","avatar":null},"body":"\nSigned-off-by: Sergei Organov \n---\n Documentation/git-rev-parse.txt | 5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt\nindex 6513c2e..0234f89 100644\n--- a/Documentation/git-rev-parse.txt\n+++ b/Documentation/git-rev-parse.txt\n@@ -200,8 +200,9 @@ blobs contained in a commit.\n object that is the th generation grand-parent of the named\n commit object, following only the first parent. I.e. rev~3 is\n equivalent to rev{caret}{caret}{caret} which is equivalent to\n- rev{caret}1{caret}1{caret}1. See below for a illustration of\n- the usage of this form.\n+ rev{caret}1{caret}1{caret}1. See below for an illustration of\n+ the usage of this form. 'rev{tilde}' is equivalent to 'rev{tilde}0'\n+ which in turn is equivalent to 'rev'.\n \n * A suffix '{caret}' followed by an object type name enclosed in\n brace pair (e.g. `v0.99.8{caret}\\{commit\\}`) means the object\n-- \n1.5.4.4.551.g1658c\n"},{"id":"72128","messageId":"alpine.LFD.1.00.0803141141240.3557@woody.linux-foundation.org","threadId":"12696","inReplyTo":"87wso5mcs7.fsf@osv.gnss.ru","subject":"Re: [PATCH] git-rev-parse.txt: clarify meaning of rev~ and rev~0.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2008-03-14T18:49:40Z","receivedAt":"2008-03-14T18:49:40Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 14 Mar 2008, Sergei Organov wrote:\n>\n> + ... 'rev{tilde}' is equivalent to 'rev{tilde}0'\n> + which in turn is equivalent to 'rev'.\n\nI'd actually prefer to just fix that. \n\nI think it would make more sense to have the same guarantees that rev^ \nhas, namely to always return a commit. I would also suggest that not \ngiving a number would have the same effect of defaulting to 1, not 0.\n\nRight now it's a bit illogical, but at least it's an _undocumented_ \nillogical behaviour. If we document it, we should fix it and document the \nlogical behaviour instead, no?\n\nHere's a patch to make '^' and '~' act the same for the default count (ie \nboth default to 1), and also have the same behaviour for a count of zero.\n\nBefore (no discernible pattern):\n\n\t[torvalds@woody git]$ git rev-parse v1.5.1 v1.5.1^0 v1.5.1~0 v1.5.1^ v1.5.1~\n\t45354a57ee7e3e42c7137db6c94fa968c6babe8d\n\t89815cab95268e8f0f58142b848ac4cd5e9cbdcb\n\t45354a57ee7e3e42c7137db6c94fa968c6babe8d\n\t045f5759c97746589a067461e50fad16f60711ac\n\t45354a57ee7e3e42c7137db6c94fa968c6babe8d\n\nAfter (fairly logical):\n\n\t[torvalds@woody git]$ ./git rev-parse v1.5.1 v1.5.1^0 v1.5.1~0 v1.5.1^ v1.5.1~\n\t45354a57ee7e3e42c7137db6c94fa968c6babe8d\n\t89815cab95268e8f0f58142b848ac4cd5e9cbdcb\n\t89815cab95268e8f0f58142b848ac4cd5e9cbdcb\n\t045f5759c97746589a067461e50fad16f60711ac\n\t045f5759c97746589a067461e50fad16f60711ac\n\nHmm?\n\n(That parent-finding loop is also now much tighter, not that it matters \none whit ;)\n\n\t\tLinus\n\n---\n sha1_name.c | 23 +++++++++++++----------\n 1 files changed, 13 insertions(+), 10 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 8b6c76f..6afc0e8 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -407,18 +407,22 @@ static int get_nth_ancestor(const char *name, int len,\n \t\t\t unsigned char *result, int generation)\n {\n \tunsigned char sha1[20];\n-\tint ret = get_sha1_1(name, len, sha1);\n+\tstruct commit *commit;\n+\tint ret;\n+\n+\tret = get_sha1_1(name, len, sha1);\n \tif (ret)\n \t\treturn ret;\n+\tcommit = lookup_commit_reference(sha1);\n+\tif (!commit)\n+\t\treturn -1;\n \n \twhile (generation--) {\n-\t\tstruct commit *commit = lookup_commit_reference(sha1);\n-\n-\t\tif (!commit || parse_commit(commit) || !commit->parents)\n+\t\tif (parse_commit(commit) || !commit->parents)\n \t\t\treturn -1;\n-\t\thashcpy(sha1, commit->parents->item->object.sha1);\n+\t\tcommit = commit->parents->item;\n \t}\n-\thashcpy(result, sha1);\n+\thashcpy(result, commit->object.sha1);\n \treturn 0;\n }\n \n@@ -564,11 +568,10 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1)\n \t\tcp++;\n \t\twhile (cp < name + len)\n \t\t\tnum = num * 10 + *cp++ - '0';\n-\t\tif (has_suffix == '^') {\n-\t\t\tif (!num && len1 == len - 1)\n-\t\t\t\tnum = 1;\n+\t\tif (!num && len1 == len - 1)\n+\t\t\tnum = 1;\n+\t\tif (has_suffix == '^')\n \t\t\treturn get_parent(name, len1, sha1, num);\n-\t\t}\n \t\t/* else if (has_suffix == '~') -- goes without saying */\n \t\treturn get_nth_ancestor(name, len1, sha1, num);\n \t}\n"},{"id":"72131","messageId":"87prtxm875.fsf@osv.gnss.ru","threadId":"12696","inReplyTo":"alpine.LFD.1.00.0803141141240.3557@woody.linux-foundation.org","subject":"Re: [PATCH] git-rev-parse.txt: clarify meaning of rev~ and rev~0.","fromName":"Sergei Organov","fromEmail":"osv@javad.com","sentAt":"2008-03-14T19:11:58Z","receivedAt":"2008-03-14T19:11:58Z","isPatch":true,"sender":{"key":"osv@javad.com","avatar":null},"body":"Linus Torvalds writes:\n\n> On Fri, 14 Mar 2008, Sergei Organov wrote:\n>>\n>> + ... 'rev{tilde}' is equivalent to 'rev{tilde}0'\n>> + which in turn is equivalent to 'rev'.\n>\n> I'd actually prefer to just fix that. \n>\n> I think it would make more sense to have the same guarantees that rev^ \n> has, namely to always return a commit. I would also suggest that not \n> giving a number would have the same effect of defaulting to 1, not 0.\n>\n> Right now it's a bit illogical, but at least it's an _undocumented_ \n> illogical behaviour. If we document it, we should fix it and document the \n> logical behaviour instead, no?\n>\n\nI entirely agree. I just took (maybe erroneously) Junio's answer to my\noriginal question as \"this behavior is by design\", assuming some\nrationale behind it, so submitted the above patch to documentation.\n\n-- Sergei.\n"},{"id":"72134","messageId":"7vfxutoyho.fsf@gitster.siamese.dyndns.org","threadId":"12696","inReplyTo":"alpine.LFD.1.00.0803141141240.3557@woody.linux-foundation.org","subject":"Re: [PATCH] git-rev-parse.txt: clarify meaning of rev~ and rev~0.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-14T20:13:23Z","receivedAt":"2008-03-14T20:13:23Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds writes:\n\n> On Fri, 14 Mar 2008, Sergei Organov wrote:\n>>\n>> + ... 'rev{tilde}' is equivalent to 'rev{tilde}0'\n>> + which in turn is equivalent to 'rev'.\n>\n> I'd actually prefer to just fix that. \n>\n> I think it would make more sense to have the same guarantees that rev^ \n> has, namely to always return a commit. I would also suggest that not \n> giving a number would have the same effect of defaulting to 1, not 0.\n>\n> Right now it's a bit illogical, but at least it's an _undocumented_ \n> illogical behaviour. If we document it, we should fix it and document the \n> logical behaviour instead, no?\n\nYeah, I like it. Not that I looked at your patch yet (which needs to wait\ntil evening), but I agree with the intent.\n"}]}