{"thread":{"id":"33695","subject":"[PATCH 0/5] A natural solution to the @ -> HEAD problem","startedAt":"2013-05-01T16:20:30Z","lastAt":"2013-05-04T08:16:51Z","messageCount":49,"participants":["Ramkumar Ramachandra","Felipe Contreras","Junio C Hamano","Jonathan Nieder","Jeff King","David Aguilar"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"216147","messageId":"1367425235-14998-1-git-send-email-artagnon@gmail.com","threadId":"33695","inReplyTo":null,"subject":"[PATCH 0/5] A natural solution to the @ -> HEAD problem","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T16:20:30Z","receivedAt":"2013-05-01T16:20:30Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Hi,\n\nFelipe's approach to solving the problem is a recursive reinterpret()\nthat relies on parsing the sequence @[^{].  Seems like quite a\ncontrived hack, and I think we can do much better than this.\n\nHere, I present my approach to solving the problem.  It interprets @\njust like a ref in resolve_ref_unsafe(), after everything else has\nbeen peeled off; the solution is just the few simple lines shown in\n[5/5].\n\nThe core of the series tackles refactoring the @-parsing so that [5/5]\nbecomes possible without doing anything contrived.  [1/5] introduces\nthe problem I started solving: making symbolic refs work properly.\n[2/5] and [3/5] are a result of me reading through the horrible\n@-parsing code.  [4/5] finally solves the symbolic ref problem, and\n[5/5] becomes trivial.\n\nA side-effect of the series is that you can now do:\n\n    $ git symbolic-ref H HEAD\n    $ git show H@{u}\n\nIn other words, symbolic refs actually work now and you can use them\nto achieve a lot of custom overrides.  I think it is a step in the\nright direction.\n\nThanks for listening, and hope you enjoy reading the series as much as\nI enjoyed writing it.\n\nRamkumar Ramachandra (5):\n  t1508 (at-combinations): more tests; document failures\n  sha1_name.c: don't waste cycles in the @-parsing loop\n  sha1_name.c: simplify @-parsing in get_sha1_basic()\n  remote.c: teach branch_get() to treat symrefs other than HEAD\n  refs.c: make @ a pseudo-ref alias to HEAD\n\n Documentation/git-check-ref-format.txt |  2 +\n Documentation/revisions.txt            |  8 +++-\n refs.c                                 | 12 ++++-\n remote.c                               | 14 ++++++\n sha1_name.c                            | 85 +++++++++++++++++++++++++---------\n t/t1400-update-ref.sh                  |  3 ++\n t/t1508-at-combinations.sh             | 15 ++++++\n 7 files changed, 113 insertions(+), 26 deletions(-)\n\n-- \n1.8.3.rc0.24.g6456091\n"},{"id":"216148","messageId":"1367425235-14998-2-git-send-email-artagnon@gmail.com","threadId":"33695","inReplyTo":"1367425235-14998-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T16:20:31Z","receivedAt":"2013-05-01T16:20:31Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"To emphasize what we're testing in @{1}@{u}, document that @{0}@{0} is\nalso nonsense.  This makes it clear that @{<n>} does not resolve to a\nref whose upstream we can determine with @{u}/ reflog we can dig with\n@{0}.\n\nSince HEAD is implicit in @{}, check that HEAD@{} works fine for these\ncases.  It doesn't make sense in @{-<n>} because @{-<n>} cannot be\nused with anything other than HEAD, and interpret_nth_prior_checkout()\nhard-codes \"HEAD\".\n\nAdditionally, document bugs in the @-parser by symbolic-ref'ing @ to\nHEAD and re-running some tests.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n t/t1508-at-combinations.sh | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh\nindex d5d6244..efa2a2a 100755\n--- a/t/t1508-at-combinations.sh\n+++ b/t/t1508-at-combinations.sh\n@@ -39,13 +39,27 @@ test_expect_success 'setup' '\n \n check HEAD new-two\n check \"@{1}\" new-one\n+check \"HEAD@{1}\" new-one\n+check \"@{now}\" new-two\n+check \"HEAD@{now}\" new-two\n check \"@{-1}\" old-two\n check \"@{-1}@{1}\" old-one\n check \"@{u}\" upstream-two\n+check \"HEAD@{u}\" upstream-two\n check \"@{u}@{1}\" upstream-one\n check \"@{-1}@{u}\" master-two\n check \"@{-1}@{u}@{1}\" master-one\n nonsense \"@{u}@{-1}\"\n+nonsense \"@{0}@{0}\"\n nonsense \"@{1}@{u}\"\n+nonsense \"HEAD@{-1}\"\n+\n+# Make sure that the @-parser isn't buggy; check things with\n+# symbolic-ref @ pointing to HEAD.\n+git symbolic-ref @ HEAD\n+check \"@@{1}\" new-one\n+check \"@@{now}\" new-two\n+check \"@@{u}\" upstream-two failure\n+nonsense \"@@{-1}\"\n \n test_done\n-- \n1.8.3.rc0.24.g6456091\n"},{"id":"216150","messageId":"1367425235-14998-3-git-send-email-artagnon@gmail.com","threadId":"33695","inReplyTo":"1367425235-14998-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 2/5] sha1_name.c: don't waste cycles in the @-parsing loop","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T16:20:32Z","receivedAt":"2013-05-01T16:20:32Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"The @-parsing loop unnecessarily checks for the sequence \"@{\" from\nlen - 2 unnecessarily.  We can safely check from len - 4: write out a\ncomment justifying this.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n sha1_name.c | 18 +++++++++++++++++-\n 1 file changed, 17 insertions(+), 1 deletion(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 3820f28..be1d12c 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -445,7 +445,23 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n \t/* basic@{time or number or -number} format to query ref-log */\n \treflog_len = at = 0;\n \tif (len && str[len-1] == '}') {\n-\t\tfor (at = len-2; at >= 0; at--) {\n+\t\t/* str = @}\n+\t\t *       ^\n+\t\t *       len - 2; expression is senseless\n+\t\t *\n+\t\t * str = @{}\n+\t\t *       ^\n+\t\t *       len - 3; expression is still senseless\n+\t\t *\n+\t\t * str = @{.}\n+\t\t *       ^\n+\t\t *       len - 4 where . is any character; expression\n+\t\t *       is worth investigating\n+\t\t *\n+\t\t * Therefore, if str ends with }, search three\n+\t\t * characters earlier for @{\n+\t\t */\n+\t\tfor (at = len - 4; at >= 0; at--) {\n \t\t\tif (str[at] == '@' && str[at+1] == '{') {\n \t\t\t\tif (!upstream_mark(str + at, len - at)) {\n \t\t\t\t\treflog_len = (len-1) - (at+2);\n-- \n1.8.3.rc0.24.g6456091\n"},{"id":"216149","messageId":"1367425235-14998-4-git-send-email-artagnon@gmail.com","threadId":"33695","inReplyTo":"1367425235-14998-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 3/5] sha1_name.c: simplify @-parsing in get_sha1_basic()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T16:20:33Z","receivedAt":"2013-05-01T16:20:33Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Presently, the logic for @-parsing is horribly convoluted.  Prove that\nit is very straightforward by laying out the three cases (@{N},\n@{u[upstream], and @{-N}) and explaining how each case should be\nhandled in comments.  All tests pass, and no functional changes have\nbeen introduced.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n sha1_name.c | 65 ++++++++++++++++++++++++++++++++++++++++++-------------------\n 1 file changed, 45 insertions(+), 20 deletions(-)\n\ndiff --git a/sha1_name.c b/sha1_name.c\nindex be1d12c..f30e344 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -463,9 +463,26 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n \t\t */\n \t\tfor (at = len - 4; at >= 0; at--) {\n \t\t\tif (str[at] == '@' && str[at+1] == '{') {\n+\t\t\t\t/* Set reflog_len only if we\n+\t\t\t\t * don't see a @{u[pstream]}.  The\n+\t\t\t\t * @{N} and @{-N} forms have to do\n+\t\t\t\t * with reflog digging.\n+\t\t\t\t */\n+\n+\t\t\t\t/* Setting len to at means that we are\n+\t\t\t\t * only going to process the part\n+\t\t\t\t * before the @ until we reach \"if\n+\t\t\t\t * (reflog)\" at the end of the\n+\t\t\t\t * function.  That is only applicable\n+\t\t\t\t * in the @{N} case; in the @{-N} and\n+\t\t\t\t * @{u[pstream]} cases, we will run it\n+\t\t\t\t * through interpret_branch_name().\n+\t\t\t\t */\n+\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+\t\t\t\t\tif (str[at + 2] != '-')\n+\t\t\t\t\t\tlen = at;\n \t\t\t\t}\n \t\t\t\tbreak;\n \t\t\t}\n@@ -476,22 +493,34 @@ 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+\tif (reflog_len) {\n+\t\tif (!len)\n+\t\t\t/* In the @{N} case where there's nothing\n+\t\t\t * before the @.\n+\t\t\t */\n+\t\t\trefs_found = dwim_log(\"HEAD\", 4, sha1, &real_ref);\n+\t\telse {\n+\t\t\t/* The @{N} case where there is something\n+\t\t\t * before the @ for dwim_log to figure out,\n+\t\t\t * and the @{-N} case.\n+\t\t\t */\n+\t\t\trefs_found = dwim_log(str, len, sha1, &real_ref);\n+\n+\t\t\tif (!refs_found && str[2] == '-') {\n+\t\t\t\t/* The @{-N} case that resolves to a\n+\t\t\t\t * detached HEAD (ie. not a ref)\n+\t\t\t\t */\n+\t\t\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\t\t\tif (interpret_branch_name(str, &buf) > 0) {\n+\t\t\t\t\tget_sha1_hex(buf.buf, sha1);\n+\t\t\t\t\trefs_found = 1;\n+\t\t\t\t\treflog_len = 0;\n+\t\t\t\t}\n+\t\t\t\tstrbuf_release(&buf);\n+\t\t\t}\n \t\t}\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-\t\trefs_found = dwim_log(str, len, sha1, &real_ref);\n-\telse\n+\t} else\n+\t\t/* The @{u[pstream]} case */\n \t\trefs_found = dwim_ref(str, len, sha1, &real_ref);\n \n \tif (!refs_found)\n@@ -506,10 +535,6 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n \t\tunsigned long co_time;\n \t\tint co_tz, co_cnt;\n \n-\t\t/* a @{-N} placed anywhere except the start is an error */\n-\t\tif (str[at+2] == '-')\n-\t\t\treturn -1;\n-\n \t\t/* Is it asking for N-th entry, or approxidate? */\n \t\tfor (i = nth = 0; 0 <= nth && i < reflog_len; i++) {\n \t\t\tchar ch = str[at+2+i];\n-- \n1.8.3.rc0.24.g6456091\n"},{"id":"216151","messageId":"1367425235-14998-5-git-send-email-artagnon@gmail.com","threadId":"33695","inReplyTo":"1367425235-14998-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 4/5] remote.c: teach branch_get() to treat symrefs other than HEAD","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T16:20:34Z","receivedAt":"2013-05-01T16:20:34Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Currently, branch_get() either accepts either a branch name, empty\nstring, or the magic four-letter word \"HEAD\".  Make it additionally\nhandle symbolic refs that point to a branch.\n\nUpdate sha1_name.c:interpret_branch_name() to look for \"@{\", not '@'\n(since '@' is a valid symbolic ref).\n\nThese two changes together make the failing test in t1508\n(at-combinations) pass.  In other words, you can now do:\n\n    $ git symbolic-ref @ HEAD\n\nAnd expect the following to work:\n\n    $ git rev-parse @@{u}\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n remote.c                   | 14 ++++++++++++++\n sha1_name.c                |  2 +-\n t/t1508-at-combinations.sh |  2 +-\n 3 files changed, 16 insertions(+), 2 deletions(-)\n\ndiff --git a/remote.c b/remote.c\nindex 68eb99b..0f44e2e 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -1470,6 +1470,20 @@ struct branch *branch_get(const char *name)\n \t\tret = current_branch;\n \telse\n \t\tret = make_branch(name, 0);\n+\n+\tif (name && *name && (!ret || !ret->remote_name)) {\n+\t\t/* Is this a symref pointing to a valid branch, other\n+\t\t * than HEAD?\n+\t\t */\n+\t\tconst char *this_ref;\n+\t\tunsigned char sha1[20];\n+\t\tint flag;\n+\n+\t\tthis_ref = resolve_ref_unsafe(name, sha1, 0, &flag);\n+\t\tif (this_ref && (flag & REF_ISSYMREF) &&\n+\t\t\t!prefixcmp(this_ref, \"refs/heads/\"))\n+\t\t\tret = make_branch(this_ref + strlen(\"refs/heads/\"), 0);\n+\t}\n \tif (ret && ret->remote_name) {\n \t\tret->remote = remote_get(ret->remote_name);\n \t\tif (ret->merge_nr) {\ndiff --git a/sha1_name.c b/sha1_name.c\nindex f30e344..c4a3a54 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -1060,7 +1060,7 @@ int interpret_branch_name(const char *name, struct strbuf *buf)\n \t\treturn ret - used + len;\n \t}\n \n-\tcp = strchr(name, '@');\n+\tcp = strstr(name, \"@{\");\n \tif (!cp)\n \t\treturn -1;\n \ttmp_len = upstream_mark(cp, namelen - (cp - name));\ndiff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh\nindex efa2a2a..73c457d 100755\n--- a/t/t1508-at-combinations.sh\n+++ b/t/t1508-at-combinations.sh\n@@ -59,7 +59,7 @@ nonsense \"HEAD@{-1}\"\n git symbolic-ref @ HEAD\n check \"@@{1}\" new-one\n check \"@@{now}\" new-two\n-check \"@@{u}\" upstream-two failure\n+check \"@@{u}\" upstream-two\n nonsense \"@@{-1}\"\n \n test_done\n-- \n1.8.3.rc0.24.g6456091\n"},{"id":"216152","messageId":"1367425235-14998-6-git-send-email-artagnon@gmail.com","threadId":"33695","inReplyTo":"1367425235-14998-1-git-send-email-artagnon@gmail.com","subject":"[PATCH 5/5] refs.c: make @ a pseudo-ref alias to HEAD","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T16:20:35Z","receivedAt":"2013-05-01T16:20:35Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"First, make sure that check_refname_format() rejects the a refname\nbeginning with a '@'.  Add a test to t1400 (update-ref) demonstrating\nthat update-ref forbids the user from updating a ref named \"@\".\n\nNow, resolve_ref_unsafe() is built to resolve any refs that have a\ncorresponding file inside $GITDIR.  Our \"@\" ref is a special\npseudo-ref and does not have a filesystem counterpart.  So,\nhard-interpret \"@\" as \"HEAD\" and resolve .git/HEAD as usual.  This\nmeans that we can drop the 'git symbolic-ref @ HEAD' line in t1508\n(at-combinations), and everything will continue working as usual.\n\nIf the user does manage to create a '.git/@' unsafely (via\nsymbolic-ref or otherwise), it will be ignored.\n\nIn practice, this means that you will now be able to do:\n\n    $ git show @~1\n    $ git log @^2\n\nAdvertise these features in the tests and documentation.\n\nSigned-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n---\n Documentation/git-check-ref-format.txt |  2 ++\n Documentation/revisions.txt            |  8 ++++++--\n refs.c                                 | 12 ++++++++++--\n t/t1400-update-ref.sh                  |  3 +++\n t/t1508-at-combinations.sh             |  7 ++++---\n 5 files changed, 25 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt\nindex ec1739a..3de9adc 100644\n--- a/Documentation/git-check-ref-format.txt\n+++ b/Documentation/git-check-ref-format.txt\n@@ -52,6 +52,8 @@ Git imposes the following rules on how references are named:\n \n . They cannot end with a dot `.`.\n \n+. They cannot be the single character `@`.\n+\n . They cannot contain a sequence `@{`.\n \n . They cannot contain a `\\`.\ndiff --git a/Documentation/revisions.txt b/Documentation/revisions.txt\nindex d477b3f..9b2e653 100644\n--- a/Documentation/revisions.txt\n+++ b/Documentation/revisions.txt\n@@ -27,6 +27,10 @@ blobs contained in a commit.\n   When ambiguous, a '<refname>' is disambiguated by taking the\n   first match in the following rules:\n \n+  . '@' is a special pseudo-ref that refers to HEAD.  An '@' followed\n+    by '\\{' has no relationship to this and means something entirely\n+    different (see below).\n+\n   . If '$GIT_DIR/<refname>' exists, that is what you mean (this is usually\n     useful only for 'HEAD', 'FETCH_HEAD', 'ORIG_HEAD', 'MERGE_HEAD'\n     and 'CHERRY_PICK_HEAD');\n@@ -93,7 +97,7 @@ some output processing may assume ref names in UTF-8.\n   refers to the branch that the branch specified by branchname is set to build on\n   top of.  A missing branchname defaults to the current one.\n \n-'<rev>{caret}', e.g. 'HEAD{caret}, v1.5.1{caret}0'::\n+'<rev>{caret}', e.g. '@{caret}2', 'HEAD{caret}', 'v1.5.1{caret}0'::\n   A suffix '{caret}' to a revision parameter means the first parent of\n   that commit object.  '{caret}<n>' means the <n>th parent (i.e.\n   '<rev>{caret}'\n@@ -101,7 +105,7 @@ some output processing may assume ref names in UTF-8.\n   '<rev>{caret}0' means the commit itself and is used when '<rev>' is the\n   object name of a tag object that refers to a commit object.\n \n-'<rev>{tilde}<n>', e.g. 'master{tilde}3'::\n+'<rev>{tilde}<n>', e.g. '@{tilde}1', 'master{tilde}3'::\n   A suffix '{tilde}<n>' to a revision parameter means the commit\n   object that is the <n>th generation ancestor of the named\n   commit object, following only the first parents.  I.e. '<rev>{tilde}3' is\ndiff --git a/refs.c b/refs.c\nindex de2d8eb..6a75f77 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -72,6 +72,9 @@ int check_refname_format(const char *refname, int flags)\n {\n \tint component_len, component_count = 0;\n \n+\tif (!strcmp(refname, \"@\"))\n+\t\treturn -1;\n+\n \twhile (1) {\n \t\t/* We are at the start of a path component. */\n \t\tcomponent_len = check_refname_component(refname, flags);\n@@ -1093,8 +1096,13 @@ const char *resolve_ref_unsafe(const char *refname, unsigned char *sha1, int rea\n \tif (flag)\n \t\t*flag = 0;\n \n-\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL))\n-\t\treturn NULL;\n+\tif (check_refname_format(refname, REFNAME_ALLOW_ONELEVEL) < 0) {\n+\t\t/* Handle the pseudo-ref @ */\n+\t\tif (!strcmp(refname, \"@\"))\n+\t\t\trefname = \"HEAD\";\n+\t\telse\n+\t\t\treturn NULL;\n+\t}\n \n \tfor (;;) {\n \t\tchar path[PATH_MAX];\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex e415ee0..ee93979 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -127,6 +127,9 @@ test_expect_success '(not) change HEAD with wrong SHA1' \"\n test_expect_success \"(not) changed .git/$m\" \"\n \t! test $B\"' = $(cat .git/'\"$m\"')\n '\n+test_expect_success 'disallow creating a ref with name @' '\n+\ttest_must_fail git update-ref @ HEAD\n+'\n rm -f .git/$m\n \n : a repository with working tree always has reflog these days...\ndiff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh\nindex 73c457d..8f8e32d 100755\n--- a/t/t1508-at-combinations.sh\n+++ b/t/t1508-at-combinations.sh\n@@ -54,12 +54,13 @@ nonsense \"@{0}@{0}\"\n nonsense \"@{1}@{u}\"\n nonsense \"HEAD@{-1}\"\n \n-# Make sure that the @-parser isn't buggy; check things with\n-# symbolic-ref @ pointing to HEAD.\n-git symbolic-ref @ HEAD\n+# Check everything with the pseudo-ref @\n check \"@@{1}\" new-one\n check \"@@{now}\" new-two\n check \"@@{u}\" upstream-two\n nonsense \"@@{-1}\"\n \n+check \"@~1\" new-one\n+check \"@^0\" new-two\n+\n test_done\n-- \n1.8.3.rc0.24.g6456091\n"},{"id":"216160","messageId":"CAMP44s1j6GdQBjgCdBrG3xvXr_XifCTRZWJAQUrzts-MzFg--g@mail.gmail.com","threadId":"33695","inReplyTo":"1367425235-14998-3-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 2/5] sha1_name.c: don't waste cycles in the @-parsing loop","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-01T17:57:50Z","receivedAt":"2013-05-01T17:57:50Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 1, 2013 at 11:20 AM, Ramkumar Ramachandra\n<artagnon@gmail.com> wrote:\n> The @-parsing loop unnecessarily checks for the sequence \"@{\" from\n> len - 2 unnecessarily.  We can safely check from len - 4: write out a\n> comment justifying this.\n>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n>  sha1_name.c | 18 +++++++++++++++++-\n>  1 file changed, 17 insertions(+), 1 deletion(-)\n>\n> diff --git a/sha1_name.c b/sha1_name.c\n> index 3820f28..be1d12c 100644\n> --- a/sha1_name.c\n> +++ b/sha1_name.c\n> @@ -445,7 +445,23 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n>         /* basic@{time or number or -number} format to query ref-log */\n>         reflog_len = at = 0;\n>         if (len && str[len-1] == '}') {\n> -               for (at = len-2; at >= 0; at--) {\n> +               /* str = @}\n> +                *       ^\n> +                *       len - 2; expression is senseless\n> +                *\n> +                * str = @{}\n> +                *       ^\n> +                *       len - 3; expression is still senseless\n> +                *\n> +                * str = @{.}\n> +                *       ^\n> +                *       len - 4 where . is any character; expression\n> +                *       is worth investigating\n> +                *\n> +                * Therefore, if str ends with }, search three\n> +                * characters earlier for @{\n> +                */\n\nI think this comment is overkill.\n\n> +               for (at = len - 4; at >= 0; at--) {\n\nThe change seems OK to me, but there's no need to explain where you\nare starting, and if there's a need:\n\n/* start from where reflogs can start: @{.} */\n\nDoes the trick nicely.\n\n>                         if (str[at] == '@' && str[at+1] == '{') {\n>                                 if (!upstream_mark(str + at, len - at)) {\n>                                         reflog_len = (len-1) - (at+2);\n> --\n\n-- \nFelipe Contreras\n"},{"id":"216161","messageId":"CAMP44s1tHC+i+Wug_UuPnprZNvaPgLMNBX9MZi49SFv4iO62SQ@mail.gmail.com","threadId":"33695","inReplyTo":"1367425235-14998-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 3/5] sha1_name.c: simplify @-parsing in get_sha1_basic()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-01T18:09:52Z","receivedAt":"2013-05-01T18:09:52Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 1, 2013 at 11:20 AM, Ramkumar Ramachandra\n<artagnon@gmail.com> wrote:\n> Presently, the logic for @-parsing is horribly convoluted.  Prove that\n> it is very straightforward by laying out the three cases (@{N},\n> @{u[upstream], and @{-N}) and explaining how each case should be\n> handled in comments.  All tests pass, and no functional changes have\n> been introduced.\n\n> @@ -463,9 +463,26 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n>                  */\n>                 for (at = len - 4; at >= 0; at--) {\n>                         if (str[at] == '@' && str[at+1] == '{') {\n> +                               /* Set reflog_len only if we\n> +                                * don't see a @{u[pstream]}.  The\n> +                                * @{N} and @{-N} forms have to do\n> +                                * with reflog digging.\n> +                                */\n> +\n> +                               /* Setting len to at means that we are\n> +                                * only going to process the part\n> +                                * before the @ until we reach \"if\n> +                                * (reflog)\" at the end of the\n> +                                * function.  That is only applicable\n> +                                * in the @{N} case; in the @{-N} and\n> +                                * @{u[pstream]} cases, we will run it\n> +                                * through interpret_branch_name().\n> +                                */\n> +\n\nOverkill.\n\n/* set reflog_len when using the form: @{N} */\n\n> @@ -476,22 +493,34 @@ 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> +       if (reflog_len) {\n> +               if (!len)\n> +                       /* In the @{N} case where there's nothing\n> +                        * before the @.\n> +                        */\n\nI think !len makes it clear.\n\n> +                       refs_found = dwim_log(\"HEAD\", 4, sha1, &real_ref);\n> +               else {\n> +                       /* The @{N} case where there is something\n> +                        * before the @ for dwim_log to figure out,\n> +                        * and the @{-N} case.\n> +                        */\n\nI think 'else' makes it clear.\n\n> +                       refs_found = dwim_log(str, len, sha1, &real_ref);\n> +\n> +                       if (!refs_found && str[2] == '-') {\n\nYou are changing the behavior, there's no need for that in this\nreorganization patch.\n\n> +                               /* The @{-N} case that resolves to a\n> +                                * detached HEAD (ie. not a ref)\n> +                                */\n\nThis is not true, it resolves to a ref.\n\ngit rev-parse --symbolic-full-name @{-1}\n\nAlso, you removed a useful comment:\n\n/* try the @{-N} syntax for n-th checkout */\n\n> +                               struct strbuf buf = STRBUF_INIT;\n> +                               if (interpret_branch_name(str, &buf) > 0) {\n> +                                       get_sha1_hex(buf.buf, sha1);\n> +                                       refs_found = 1;\n> +                                       reflog_len = 0;\n> +                               }\n> +                               strbuf_release(&buf);\n\nI'm pretty sure this is doing something totally different now. This is\ncertainly not \"no functional changes\".\n\n> +                       }\n>                 }\n> -               /* allow \"@{...}\" to mean the current branch reflog */\n> -               refs_found = dwim_ref(\"HEAD\", 4, sha1, &real_ref);\n> -       } else if (reflog_len)\n> -               refs_found = dwim_log(str, len, sha1, &real_ref);\n> -       else\n> +       } else\n> +               /* The @{u[pstream]} case */\n\nIt's not true, there might not be any @{u} in there.\n\nSome of these changes might be good, but I would do them truly without\nintroducing functional changes, without removing useful comments, and\nwithout adding paragraphs of explanation for what you are doing.\n\nWith the principle of self-documenting code, if you need paragraphs to\nexplain what you are doing, you already lost.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"216162","messageId":"CAMP44s20F-QALd18VPHLF4Fj=eFFvXmkhC4XK__kxNhMoeN=ug@mail.gmail.com","threadId":"33695","inReplyTo":"1367425235-14998-5-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 4/5] remote.c: teach branch_get() to treat symrefs other than HEAD","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-01T18:16:05Z","receivedAt":"2013-05-01T18:16:05Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 1, 2013 at 11:20 AM, Ramkumar Ramachandra\n<artagnon@gmail.com> wrote:\n> Currently, branch_get() either accepts either a branch name, empty\n> string, or the magic four-letter word \"HEAD\".  Make it additionally\n> handle symbolic refs that point to a branch.\n>\n> Update sha1_name.c:interpret_branch_name() to look for \"@{\", not '@'\n> (since '@' is a valid symbolic ref).\n>\n> These two changes together make the failing test in t1508\n> (at-combinations) pass.  In other words, you can now do:\n>\n>     $ git symbolic-ref @ HEAD\n>\n> And expect the following to work:\n>\n>     $ git rev-parse @@{u}\n>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n>  remote.c                   | 14 ++++++++++++++\n>  sha1_name.c                |  2 +-\n>  t/t1508-at-combinations.sh |  2 +-\n>  3 files changed, 16 insertions(+), 2 deletions(-)\n>\n> diff --git a/remote.c b/remote.c\n> index 68eb99b..0f44e2e 100644\n> --- a/remote.c\n> +++ b/remote.c\n> @@ -1470,6 +1470,20 @@ struct branch *branch_get(const char *name)\n>                 ret = current_branch;\n>         else\n>                 ret = make_branch(name, 0);\n> +\n> +       if (name && *name && (!ret || !ret->remote_name)) {\n> +               /* Is this a symref pointing to a valid branch, other\n> +                * than HEAD?\n> +                */\n> +               const char *this_ref;\n> +               unsigned char sha1[20];\n> +               int flag;\n> +\n> +               this_ref = resolve_ref_unsafe(name, sha1, 0, &flag);\n> +               if (this_ref && (flag & REF_ISSYMREF) &&\n> +                       !prefixcmp(this_ref, \"refs/heads/\"))\n> +                       ret = make_branch(this_ref + strlen(\"refs/heads/\"), 0);\n> +       }\n\nBut why? I'm not familiar with branch_get, but my intuition tells me\nyou are changing the behavior, and now branch_get() is doing something\nit wasn't intended to do. And for what?\n\nYour rationale is that it fixes the test cases below, but that's not\nreason enough, since there are other ways to fix them, as my patch\nseries shows.\n\n>         if (ret && ret->remote_name) {\n>                 ret->remote = remote_get(ret->remote_name);\n>                 if (ret->merge_nr) {\n> diff --git a/sha1_name.c b/sha1_name.c\n> index f30e344..c4a3a54 100644\n> --- a/sha1_name.c\n> +++ b/sha1_name.c\n> @@ -1060,7 +1060,7 @@ int interpret_branch_name(const char *name, struct strbuf *buf)\n>                 return ret - used + len;\n>         }\n>\n> -       cp = strchr(name, '@');\n> +       cp = strstr(name, \"@{\");\n\nThis might make sense, but it feels totally sneaked in.\n\n>         if (!cp)\n>                 return -1;\n>         tmp_len = upstream_mark(cp, namelen - (cp - name));\n\nI think these are two patches should be introduced separately, and\nwith a reason for them to exist independent of each other.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"216163","messageId":"CAMP44s3nzuecoM+h+pNknV4A68R1gZ6DZpehp3uKcJhppXo+1w@mail.gmail.com","threadId":"33695","inReplyTo":"1367425235-14998-6-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 5/5] refs.c: make @ a pseudo-ref alias to HEAD","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-01T18:20:38Z","receivedAt":"2013-05-01T18:20:38Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 1, 2013 at 11:20 AM, Ramkumar Ramachandra\n<artagnon@gmail.com> wrote:\n> First, make sure that check_refname_format() rejects the a refname\n> beginning with a '@'.  Add a test to t1400 (update-ref) demonstrating\n> that update-ref forbids the user from updating a ref named \"@\".\n>\n> Now, resolve_ref_unsafe() is built to resolve any refs that have a\n> corresponding file inside $GITDIR.  Our \"@\" ref is a special\n> pseudo-ref and does not have a filesystem counterpart.  So,\n> hard-interpret \"@\" as \"HEAD\" and resolve .git/HEAD as usual.  This\n> means that we can drop the 'git symbolic-ref @ HEAD' line in t1508\n> (at-combinations), and everything will continue working as usual.\n>\n> If the user does manage to create a '.git/@' unsafely (via\n> symbolic-ref or otherwise), it will be ignored.\n>\n> In practice, this means that you will now be able to do:\n>\n>     $ git show @~1\n>     $ git log @^2\n>\n> Advertise these features in the tests and documentation.\n>\n> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n> ---\n>  Documentation/git-check-ref-format.txt |  2 ++\n>  Documentation/revisions.txt            |  8 ++++++--\n>  refs.c                                 | 12 ++++++++++--\n>  t/t1400-update-ref.sh                  |  3 +++\n>  t/t1508-at-combinations.sh             |  7 ++++---\n>  5 files changed, 25 insertions(+), 7 deletions(-)\n>\n> diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt\n> index ec1739a..3de9adc 100644\n> --- a/Documentation/git-check-ref-format.txt\n> +++ b/Documentation/git-check-ref-format.txt\n> @@ -52,6 +52,8 @@ Git imposes the following rules on how references are named:\n>\n>  . They cannot end with a dot `.`.\n>\n> +. They cannot be the single character `@`.\n> +\n>  . They cannot contain a sequence `@{`.\n>\n>  . They cannot contain a `\\`.\n> diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt\n> index d477b3f..9b2e653 100644\n> --- a/Documentation/revisions.txt\n> +++ b/Documentation/revisions.txt\n> @@ -27,6 +27,10 @@ blobs contained in a commit.\n>    When ambiguous, a '<refname>' is disambiguated by taking the\n>    first match in the following rules:\n>\n> +  . '@' is a special pseudo-ref that refers to HEAD.\n\nDoes the user really cares if it's a pseudo-ref or not? Also, what\ndoes it mean that \"refers\" to HEAD?\n\n> An '@' followed\n> +    by '\\{' has no relationship to this and means something entirely\n> +    different (see below).\n\nIf the user cares about that, the user can see that below, otherwise\nthere's no point in mentioning that. Just like there's no point in\nmentioning that @{-N} means something totally different from @{N},\nbecause the user can see that. If it didn't mean something different,\nthis bullet point wouldn't exist.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"216170","messageId":"CALkWK0nTSMYvh8VMgQ6Q0EoPMmRa2vyodz+tDmpPp1d6KYmq8w@mail.gmail.com","threadId":"33695","inReplyTo":"CAMP44s1tHC+i+Wug_UuPnprZNvaPgLMNBX9MZi49SFv4iO62SQ@mail.gmail.com","subject":"Re: [PATCH 3/5] sha1_name.c: simplify @-parsing in get_sha1_basic()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T18:36:27Z","receivedAt":"2013-05-01T18:36:27Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> On Wed, May 1, 2013 at 11:20 AM, Ramkumar Ramachandra\n> <artagnon@gmail.com> wrote:\n>> +                       refs_found = dwim_log(str, len, sha1, &real_ref);\n>> +\n>> +                       if (!refs_found && str[2] == '-') {\n>\n> You are changing the behavior, there's no need for that in this\n> reorganization patch.\n\nThis is not a reorganization patch.  I said \"simplify\": not refactor.\n\n>> +                               /* The @{-N} case that resolves to a\n>> +                                * detached HEAD (ie. not a ref)\n>> +                                */\n>\n> This is not true, it resolves to a ref.\n>\n> git rev-parse --symbolic-full-name @{-1}\n\n> git co @~1; git co -; git rev-parse --symbolic-full-name @{-1}\n\nIf it did resolve to a ref, dwim_log() would have found it.  The\nconstraint guarding this `if (!refs_found && str[2] == '-')` wouldn't\nhave been satisfied, and we wouldn't be here.\n\n> Also, you removed a useful comment:\n>\n> /* try the @{-N} syntax for n-th checkout */\n\nI've changed the entire logic and written expensive comments; and I'm\nnot allowed to remove one comment which I didn't find helpful?\n\n>> +                               struct strbuf buf = STRBUF_INIT;\n>> +                               if (interpret_branch_name(str, &buf) > 0) {\n>> +                                       get_sha1_hex(buf.buf, sha1);\n>> +                                       refs_found = 1;\n>> +                                       reflog_len = 0;\n>> +                               }\n>> +                               strbuf_release(&buf);\n>\n> I'm pretty sure this is doing something totally different now. This is\n> certainly not \"no functional changes\".\n\nI'm claiming that there is no functional change at the program level,\nwith the commenting*.  If you want to disprove my claim, you have to\nwrite a test that breaks after this patch.\n\n* And even if there is, it's probably an accidental corner case with\nthe wrong behavior.  The new logic is a straightforward implementation\nof the rules.\n\n>> +                       }\n>>                 }\n>> -               /* allow \"@{...}\" to mean the current branch reflog */\n>> -               refs_found = dwim_ref(\"HEAD\", 4, sha1, &real_ref);\n>> -       } else if (reflog_len)\n>> -               refs_found = dwim_log(str, len, sha1, &real_ref);\n>> -       else\n>> +       } else\n>> +               /* The @{u[pstream]} case */\n>\n> It's not true, there might not be any @{u} in there.\n\nThis entire structure is a success-filter.  At the end of this, if\n!refs_found, then there has been a failure.\n\n> Some of these changes might be good, but I would do them truly without\n> introducing functional changes, without removing useful comments, and\n> without adding paragraphs of explanation for what you are doing.\n\nThe functional changes part is for you to prove.  And it's not even\nworth proving, because I'm claiming that the new logic is an obviously\ncorrect implementation of the rules.\n\n> With the principle of self-documenting code, if you need paragraphs to\n> explain what you are doing, you already lost.\n\nThe Git project is already suffering from a severe shortage of\ncomments [1], and you're complaining about too many comments?\n\nHave you tried to read and understand the old version?  Some shining\nexample of self-documenting code you've brought up.\n\n[1]: https://www.ohloh.net/p/git/factoids#FactoidCommentsLow\n"},{"id":"216174","messageId":"CALkWK0n2odCn=GnYSXv7g113PFEX42WF1d2GBGV=ye2TuY3CHA@mail.gmail.com","threadId":"33695","inReplyTo":"CAMP44s20F-QALd18VPHLF4Fj=eFFvXmkhC4XK__kxNhMoeN=ug@mail.gmail.com","subject":"Re: [PATCH 4/5] remote.c: teach branch_get() to treat symrefs other than HEAD","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T18:44:44Z","receivedAt":"2013-05-01T18:44:44Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> But why? I'm not familiar with branch_get, but my intuition tells me\n> you are changing the behavior, and now branch_get() is doing something\n> it wasn't intended to do. And for what?\n\nWhy is there a commit message?  I've explained what the behavior change is.\n\n> Your rationale is that it fixes the test cases below, but that's not\n> reason enough, since there are other ways to fix them, as my patch\n> series shows.\n\nFor what exactly.  To fix a real bug: H@{u} and @@{u} don't work where\neither H or @ are symbolic refs.  I want custom symbolic refs, because\nthey are useful.  In other words, \"HEAD\" is not a sacred symbolic ref.\n\n>>         if (ret && ret->remote_name) {\n>>                 ret->remote = remote_get(ret->remote_name);\n>>                 if (ret->merge_nr) {\n>> diff --git a/sha1_name.c b/sha1_name.c\n>> index f30e344..c4a3a54 100644\n>> --- a/sha1_name.c\n>> +++ b/sha1_name.c\n>> @@ -1060,7 +1060,7 @@ int interpret_branch_name(const char *name, struct strbuf *buf)\n>>                 return ret - used + len;\n>>         }\n>>\n>> -       cp = strchr(name, '@');\n>> +       cp = strstr(name, \"@{\");\n>\n> This might make sense, but it feels totally sneaked in.\n\nSneaked in?  I have three paragraphs in my commit message.  The first\ntwo explain two related changes, and the third one shows how they are\nrelated.\n\n>>         if (!cp)\n>>                 return -1;\n>>         tmp_len = upstream_mark(cp, namelen - (cp - name));\n>\n> I think these are two patches should be introduced separately, and\n> with a reason for them to exist independent of each other.\n\nI cannot justify the remote.c patch without the \"@{\" change.\n"},{"id":"216175","messageId":"CALkWK0nERh_PRWgpPEgim_M1=oPXR03f343u1TLwiwqgi4JpSQ@mail.gmail.com","threadId":"33695","inReplyTo":"CAMP44s1j6GdQBjgCdBrG3xvXr_XifCTRZWJAQUrzts-MzFg--g@mail.gmail.com","subject":"Re: [PATCH 2/5] sha1_name.c: don't waste cycles in the @-parsing loop","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T18:48:29Z","receivedAt":"2013-05-01T18:48:29Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> I think this comment is overkill.\n\nThat's not for one line.  It's for the whole logic following it: there\nare things like (len-1) - (at+2) which are easy to visualize with this\npicture.  They're int, not char *.\n"},{"id":"216176","messageId":"7va9oe5y6k.fsf@alter.siamese.dyndns.org","threadId":"33695","inReplyTo":"1367425235-14998-2-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-01T18:53:39Z","receivedAt":"2013-05-01T18:53:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramkumar Ramachandra <artagnon@gmail.com> writes:\n\n> To emphasize what we're testing in @{1}@{u}, document that @{0}@{0} is\n> also nonsense.  This makes it clear that @{<n>} does not resolve to a\n> ref whose upstream we can determine with @{u}/ reflog we can dig with\n> @{0}.\n>\n> Since HEAD is implicit in @{},...\n\nJust making sure.  HEAD@{$n} and @{$n} for non-negative $n mean\ntotally different things.  @{0} and HEAD@{0} are almost always the\nsame, and @{1} and HEAD@{1} may often happen to be the same, but as\na blanket statement, I find \"Since HEAD is implicit in @{}\" very\nmisleading.\n\nAs you and Felipe seem to be aiming for the same \"Let's allow users\nto say '@' when they mean HEAD\", I'll let you two figure the best\napproach out.\n\nOne productive way forward might be to come up with a common test\nscript pieces to document what constructs that spell @ in place of\nHEAD should be supported, and much more importantly, what constructs\nthat happen to have @ in them should not mistakenly trigger the new\nmachinery.\n\nHave fun ;-)\n"},{"id":"216177","messageId":"20130501185414.GN24467@google.com","threadId":"33695","inReplyTo":"CALkWK0nTSMYvh8VMgQ6Q0EoPMmRa2vyodz+tDmpPp1d6KYmq8w@mail.gmail.com","subject":"Re: [PATCH 3/5] sha1_name.c: simplify @-parsing in get_sha1_basic()","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-05-01T18:54:14Z","receivedAt":"2013-05-01T18:54:14Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Ramkumar Ramachandra wrote:\n\n> [1]: https://www.ohloh.net/p/git/factoids#FactoidCommentsLow\n\nSince this has been coming up from time to time:\n\nI have nothing against including helpful comments where appropriate.\nBut one aspect which that factoid misses is that git has some very\ndetailed, very dense documentation available in its commit log.  Tools\nlike \"git gui blame\" and \"git log -S\" can show detailed historical\ninformation about the purpose of every line of code.  A nice feature\nof such documentation is that it is in a context where it cannot fall\nout of date.\n\nSo for example I can do\n\n\t$ git log -S'if (len && ambiguous_path(str, len))' -- sha1_name.c\n\tcommit 11cf8801\n\tAuthor: Nicolas Pitre <nico@cam.org>\n\tDate:   Thu Feb 1 17:29:33 2007 -0500\n\n\t    provide a nice @{...} syntax to always mean the current branch reflog\n\n\t    This is shorter than HEAD@{...} and being nameless it has no semantic\n\t    issues.\n\n\t    Signed-off-by: Nicolas Pitre <nico@cam.org>\n\t    Signed-off-by: Junio C Hamano <junkio@cox.net>\n\nand then \"git show 11cf8801\" will show me exactly what change prompted\nthat \"len\" test.\n\nThe same is true of the Linux kernel, too.\n\nHope that helps,\nJonathan\n"},{"id":"216179","messageId":"CALkWK0=WjctcYv30V9b3NMb5motYz=57OqmLU7pdW_fbOP40MA@mail.gmail.com","threadId":"33695","inReplyTo":"CAMP44s3nzuecoM+h+pNknV4A68R1gZ6DZpehp3uKcJhppXo+1w@mail.gmail.com","subject":"Re: [PATCH 5/5] refs.c: make @ a pseudo-ref alias to HEAD","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T19:00:05Z","receivedAt":"2013-05-01T19:00:05Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> Does the user really cares if it's a pseudo-ref or not? Also, what\n> does it mean that \"refers\" to HEAD?\n\nIt's not about whether the user cares or not; it's about saying it in\na way that doesn't make it less precise.  I'm saying \"@ is like a\nsymbolic-ref .git/@ ref referring to HEAD, except it doesn't sit on\nthe filesystem\".\n\nIt's not important that end users understand it fully.  It's only\nreally useful in two cases: @~ and @^, and I've provided examples\nthere.\n\n>> An '@' followed\n>> +    by '\\{' has no relationship to this and means something entirely\n>> +    different (see below).\n>\n> If the user cares about that, the user can see that below, otherwise\n> there's no point in mentioning that.\n\nA user seeing @{} might vaguely recall @ and scroll-back here.  In\nwhich case, this is useful.\n\n> Just like there's no point in\n> mentioning that @{-N} means something totally different from @{N},\n> because the user can see that. If it didn't mean something different,\n> this bullet point wouldn't exist.\n\nThose two are right next to each other.\n"},{"id":"216180","messageId":"CAMP44s0sRqsmhbv-GA5x0FPc5msouSoGaU_Wg51E0F9bNsmkjg@mail.gmail.com","threadId":"33695","inReplyTo":"CALkWK0nTSMYvh8VMgQ6Q0EoPMmRa2vyodz+tDmpPp1d6KYmq8w@mail.gmail.com","subject":"Re: [PATCH 3/5] sha1_name.c: simplify @-parsing in get_sha1_basic()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-01T19:23:55Z","receivedAt":"2013-05-01T19:23:55Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 1, 2013 at 1:36 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n> Felipe Contreras wrote:\n>> On Wed, May 1, 2013 at 11:20 AM, Ramkumar Ramachandra\n>> <artagnon@gmail.com> wrote:\n>>> +                       refs_found = dwim_log(str, len, sha1, &real_ref);\n>>> +\n>>> +                       if (!refs_found && str[2] == '-') {\n>>\n>> You are changing the behavior, there's no need for that in this\n>> reorganization patch.\n>\n> This is not a reorganization patch.  I said \"simplify\": not refactor.\n\nI'd say you should start with a reorganization, and then a simplification.\n\n>>> +                               /* The @{-N} case that resolves to a\n>>> +                                * detached HEAD (ie. not a ref)\n>>> +                                */\n>>\n>> This is not true, it resolves to a ref.\n>>\n>> git rev-parse --symbolic-full-name @{-1}\n>\n>> git co @~1; git co -; git rev-parse --symbolic-full-name @{-1}\n>\n> If it did resolve to a ref, dwim_log() would have found it.  The\n> constraint guarding this `if (!refs_found && str[2] == '-')` wouldn't\n> have been satisfied, and we wouldn't be here.\n\nI see.\n\n>> Also, you removed a useful comment:\n>>\n>> /* try the @{-N} syntax for n-th checkout */\n>\n> I've changed the entire logic and written expensive comments; and I'm\n> not allowed to remove one comment which I didn't find helpful?\n\nBut it is helpful.\n\n>>> +                               struct strbuf buf = STRBUF_INIT;\n>>> +                               if (interpret_branch_name(str, &buf) > 0) {\n>>> +                                       get_sha1_hex(buf.buf, sha1);\n>>> +                                       refs_found = 1;\n>>> +                                       reflog_len = 0;\n>>> +                               }\n>>> +                               strbuf_release(&buf);\n>>\n>> I'm pretty sure this is doing something totally different now. This is\n>> certainly not \"no functional changes\".\n>\n> I'm claiming that there is no functional change at the program level,\n> with the commenting*.  If you want to disprove my claim, you have to\n> write a test that breaks after this patch.\n\nThe burden of proof resides in the one that makes the claim, I don't\nneed to prove that there are functional changes, merely that you\nhaven't met your burden of proof.\n\nThat being said, perhaps there are no _behavioral_ changes, because\nyou are relegating some of the current functionality to dwim_log, but\nthe code most certainly is doing something completely different.\nBefore, get_sha1_basic recursively called itself when @{-N} resolved\nto a branch name, now, you rely on dwim_log to do this for you without\nrecursion, which is nice, but functionally different.\n\n>>> +                       }\n>>>                 }\n>>> -               /* allow \"@{...}\" to mean the current branch reflog */\n>>> -               refs_found = dwim_ref(\"HEAD\", 4, sha1, &real_ref);\n>>> -       } else if (reflog_len)\n>>> -               refs_found = dwim_log(str, len, sha1, &real_ref);\n>>> -       else\n>>> +       } else\n>>> +               /* The @{u[pstream]} case */\n>>\n>> It's not true, there might not be any @{u} in there.\n>\n> This entire structure is a success-filter.  At the end of this, if\n> !refs_found, then there has been a failure.\n\nThat's irrelevant, this 'else' case is for !reflog_len, there might or\nmight not be @{u} in there.\n\n>> With the principle of self-documenting code, if you need paragraphs to\n>> explain what you are doing, you already lost.\n>\n> The Git project is already suffering from a severe shortage of\n> comments [1], and you're complaining about too many comments?\n\nIrrelevant conclusion fallacy; let's suppose that the Git project is\nindeed suffering from a shortage of comments, that doesn't imply that\n*these* comments in their present form are any good.\n\n-- \nFelipe Contreras\n"},{"id":"216181","messageId":"CAMP44s34X+6c2V0iU_+dfNR9ULXD+Q0CDFk0S9qdK0n8AC81DQ@mail.gmail.com","threadId":"33695","inReplyTo":"CALkWK0n2odCn=GnYSXv7g113PFEX42WF1d2GBGV=ye2TuY3CHA@mail.gmail.com","subject":"Re: [PATCH 4/5] remote.c: teach branch_get() to treat symrefs other than HEAD","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-01T19:28:27Z","receivedAt":"2013-05-01T19:28:27Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 1, 2013 at 1:44 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n> Felipe Contreras wrote:\n>> But why? I'm not familiar with branch_get, but my intuition tells me\n>> you are changing the behavior, and now branch_get() is doing something\n>> it wasn't intended to do. And for what?\n>\n> Why is there a commit message?  I've explained what the behavior change is.\n\nNot good enough.\n\n>> Your rationale is that it fixes the test cases below, but that's not\n>> reason enough, since there are other ways to fix them, as my patch\n>> series shows.\n>\n> For what exactly.  To fix a real bug: H@{u} and @@{u} don't work where\n> either H or @ are symbolic refs.  I want custom symbolic refs, because\n> they are useful.  In other words, \"HEAD\" is not a sacred symbolic ref.\n\nAs I said, the @@{u} thing can be fixed through other ways.\n\nMoreover, \"HEAD\" is still a special case in remote.c::branch_get()\nthat you just modified.\n\n>> I think these are two patches should be introduced separately, and\n>> with a reason for them to exist independent of each other.\n>\n> I cannot justify the remote.c patch without the \"@{\" change.\n\nThat's what I thought.\n\n-- \nFelipe Contreras\n"},{"id":"216182","messageId":"CAMP44s3D3yPx6mfLvnkf3Ef55=kP1Jtx1P947uG7pg8_HrH3Vg@mail.gmail.com","threadId":"33695","inReplyTo":"CALkWK0=WjctcYv30V9b3NMb5motYz=57OqmLU7pdW_fbOP40MA@mail.gmail.com","subject":"Re: [PATCH 5/5] refs.c: make @ a pseudo-ref alias to HEAD","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-01T19:31:29Z","receivedAt":"2013-05-01T19:31:29Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 1, 2013 at 2:00 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n> Felipe Contreras wrote:\n>> Does the user really cares if it's a pseudo-ref or not? Also, what\n>> does it mean that \"refers\" to HEAD?\n>\n> It's not about whether the user cares or not; it's about saying it in\n> a way that doesn't make it less precise.  I'm saying \"@ is like a\n> symbolic-ref .git/@ ref referring to HEAD, except it doesn't sit on\n> the filesystem\".\n\nIf I put my user shoes, I don't care how @ is implemented, I just care\nthat it's a shortcut for HEAD, that's what it means to me, the common\nuser.\n\n-- \nFelipe Contreras\n"},{"id":"216183","messageId":"CALkWK0=Z81f4c1X3uXO4O5q_Dj2hRJjSY-i-aDtZ0KqSyo5Wtg@mail.gmail.com","threadId":"33695","inReplyTo":"CAMP44s0sRqsmhbv-GA5x0FPc5msouSoGaU_Wg51E0F9bNsmkjg@mail.gmail.com","subject":"Re: [PATCH 3/5] sha1_name.c: simplify @-parsing in get_sha1_basic()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T19:40:09Z","receivedAt":"2013-05-01T19:40:09Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> On Wed, May 1, 2013 at 1:36 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n>> This is not a reorganization patch.  I said \"simplify\": not refactor.\n>\n> I'd say you should start with a reorganization, and then a simplification.\n\nYou don't think I already tried that?  There is no way to sensibly\nreorganize the old logic sensibly, in a way that doesn't break\nanything.\n\n>> I've changed the entire logic and written expensive comments; and I'm\n>> not allowed to remove one comment which I didn't find helpful?\n>\n> But it is helpful.\n\nOkay, we'll add it back if you feel strongly about it.  I thought it\nwould be a sore thumb when neither @{N} nor @{upstream} are explained.\n\n>> I'm claiming that there is no functional change at the program level,\n>> with the commenting*.  If you want to disprove my claim, you have to\n>> write a test that breaks after this patch.\n>\n> The burden of proof resides in the one that makes the claim, I don't\n> need to prove that there are functional changes, merely that you\n> haven't met your burden of proof.\n\nOh, but I have.  All the tests (along with the new ones I added in [1/5]) pass.\n\nScientific theories stand until you can find one observation that disproves it.\n\n> That being said, perhaps there are no _behavioral_ changes, because\n> you are relegating some of the current functionality to dwim_log, but\n> the code most certainly is doing something completely different.\n> Before, get_sha1_basic recursively called itself when @{-N} resolved\n> to a branch name, now, you rely on dwim_log to do this for you without\n> recursion, which is nice, but functionally different.\n\nYour point being?  That I shouldn't say \"no functional changes\"\nbecause the logic is changing non-trivially at this level?\n\n>>> It's not true, there might not be any @{u} in there.\n>>\n>> This entire structure is a success-filter.  At the end of this, if\n>> !refs_found, then there has been a failure.\n>\n> That's irrelevant, this 'else' case is for !reflog_len, there might or\n> might not be @{u} in there.\n\nThere's no need to associate one comment with one line of code.\nPeople can see clearly see the failure case following it.\n\n>> The Git project is already suffering from a severe shortage of\n>> comments [1], and you're complaining about too many comments?\n>\n> Irrelevant conclusion fallacy; let's suppose that the Git project is\n> indeed suffering from a shortage of comments, that doesn't imply that\n> *these* comments in their present form are any good.\n\nIs there anything _wrong_ with the comments, apart from the fact that\nthey seem to be too big?  I'm saying things that I cannot say in the\ncommit message.\n"},{"id":"216184","messageId":"CALkWK0mfHVBLPdU7jyiBR=-kSguCBFOhsEsXYYthBuQbtX7MuA@mail.gmail.com","threadId":"33695","inReplyTo":"CAMP44s34X+6c2V0iU_+dfNR9ULXD+Q0CDFk0S9qdK0n8AC81DQ@mail.gmail.com","subject":"Re: [PATCH 4/5] remote.c: teach branch_get() to treat symrefs other than HEAD","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T19:50:51Z","receivedAt":"2013-05-01T19:50:51Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> As I said, the @@{u} thing can be fixed through other ways.\n\nIt's not just @@{u}.  I can have lots of custom symbolic refs working\nproperly; I might choose D for my deleted-reflogs, M for master and so\non.\n\n    $ git log M..\n\nThe point is that my solution solves the problem in the more general case.\n\n> Moreover, \"HEAD\" is still a special case in remote.c::branch_get()\n> that you just modified.\n\nIt's not.  It's just a quick shortcut for the common case because\nread_config() already sets it up.\n"},{"id":"216185","messageId":"CALkWK0=DajZwYuDYoVS7tJwHO276HXXZ2_z69FeY_w=Kk0V3hg@mail.gmail.com","threadId":"33695","inReplyTo":"CAMP44s3D3yPx6mfLvnkf3Ef55=kP1Jtx1P947uG7pg8_HrH3Vg@mail.gmail.com","subject":"Re: [PATCH 5/5] refs.c: make @ a pseudo-ref alias to HEAD","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T19:51:56Z","receivedAt":"2013-05-01T19:51:56Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> If I put my user shoes, I don't care how @ is implemented, I just care\n> that it's a shortcut for HEAD, that's what it means to me, the common\n> user.\n\nOkay, we'll change this.\n"},{"id":"216188","messageId":"CALkWK0=EcKmWm5MTEHO0jXGnLkZ0Hpis__metA7pHOUo+AFJMA@mail.gmail.com","threadId":"33695","inReplyTo":"20130501185414.GN24467@google.com","subject":"Re: [PATCH 3/5] sha1_name.c: simplify @-parsing in get_sha1_basic()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T19:55:25Z","receivedAt":"2013-05-01T19:55:25Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jonathan Nieder wrote:\n> Since this has been coming up from time to time:\n> [...]\n\nThanks, I didn't know about 'git gui blame'.\n\nI think both comments and commit messages have their uses.  One cannot\ndo the job of the other.\n"},{"id":"216194","messageId":"CAMP44s1dWhpVhUar2-u1NAqK-TPssxZYgGMY9_SVxvuKSFT4QA@mail.gmail.com","threadId":"33695","inReplyTo":"CALkWK0mfHVBLPdU7jyiBR=-kSguCBFOhsEsXYYthBuQbtX7MuA@mail.gmail.com","subject":"Re: [PATCH 4/5] remote.c: teach branch_get() to treat symrefs other than HEAD","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-01T20:48:55Z","receivedAt":"2013-05-01T20:48:55Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 1, 2013 at 2:50 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n> Felipe Contreras wrote:\n>> As I said, the @@{u} thing can be fixed through other ways.\n>\n> It's not just @@{u}.  I can have lots of custom symbolic refs working\n> properly; I might choose D for my deleted-reflogs, M for master and so\n> on.\n\nAnd that doesn't require the changes you did to sha1_name.c, or\nt/t1508-at-combinations.sh, does it?\n\nIf you are arguing in favor of arbitrary symbolic refs plus @{u} to\nwork, a patch that allows that, and nothing else, should make sense.\nSuch patch would trigger further discussion, for example, if\nget_branch() is the right place to achieve that.\n\n> The point is that my solution solves the problem in the more general case.\n>\n>> Moreover, \"HEAD\" is still a special case in remote.c::branch_get()\n>> that you just modified.\n>\n> It's not.  It's just a quick shortcut for the common case because\n> read_config() already sets it up.\n\nI still see this in the code:\n\n\tif (!name || !*name || !strcmp(name, \"HEAD\"))\n\t\tret = current_branch;\n\telse\n\t\tret = make_branch(name, 0);\n\n-- \nFelipe Contreras\n"},{"id":"216196","messageId":"CALkWK0mz6+RS-fZAjz=1+kVN6WFTb+cX98V=8AGfzybacf28RQ@mail.gmail.com","threadId":"33695","inReplyTo":"CAMP44s1dWhpVhUar2-u1NAqK-TPssxZYgGMY9_SVxvuKSFT4QA@mail.gmail.com","subject":"Re: [PATCH 4/5] remote.c: teach branch_get() to treat symrefs other than HEAD","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T20:57:12Z","receivedAt":"2013-05-01T20:57:12Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> If you are arguing in favor of arbitrary symbolic refs plus @{u} to\n> work, a patch that allows that, and nothing else, should make sense.\n> Such patch would trigger further discussion, for example, if\n> get_branch() is the right place to achieve that.\n\nWhat kind of absurd argument is this?  I am trying to solve the @ ->\nHEAD problem correctly.  In the process, I fixed general symbolic-ref\nhandling and cleaned up the @-parsing logic.  Does anyone have a\nproblem with the fix, or am I missing something?\n\nSo, start the discussion.  What are you waiting for?\n\n> I still see this in the code:\n>\n>         if (!name || !*name || !strcmp(name, \"HEAD\"))\n\nRemove the !strcmp(name, \"HEAD\") and tell me what you see.  \"HEAD\"\nwill get resolved just like any other symbolic ref, via the hunk I\nadded.\n"},{"id":"216197","messageId":"CALkWK0nL+UvjsUKh6v4x_MaWJmsU6Uh_gh82iE7UT0UE-ZMt0g@mail.gmail.com","threadId":"33695","inReplyTo":"7va9oe5y6k.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T21:04:01Z","receivedAt":"2013-05-01T21:04:01Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Junio C Hamano wrote:\n> Just making sure.  HEAD@{$n} and @{$n} for non-negative $n mean\n> totally different things.  @{0} and HEAD@{0} are almost always the\n> same, and @{1} and HEAD@{1} may often happen to be the same, but as\n> a blanket statement, I find \"Since HEAD is implicit in @{}\" very\n> misleading.\n\nWhen will they be different?  I'm looking at this from the parser's\npoint of view: when the part before @{} is missing, we dwim a \"HEAD\".\n\n> As you and Felipe seem to be aiming for the same \"Let's allow users\n> to say '@' when they mean HEAD\", I'll let you two figure the best\n> approach out.\n\nI've solved the problem in the general case of symbolic-refs and made\n\"@\" a special case of that.\n\n> One productive way forward might be to come up with a common test\n> script pieces to document what constructs that spell @ in place of\n> HEAD should be supported,\n\nIt's sufficient to test that symbolic refs work properly.  @ is a\ntrivial implementation of a pseudo symbolic-ref (see [5/5]).\n\n> and much more importantly, what constructs\n> that happen to have @ in them should not mistakenly trigger the new\n> machinery.\n\nAt the parsing level, @ can only ever interfere with @{}; I've added\ntests for those.\n"},{"id":"216198","messageId":"20130501211613.GA15392@sigill.intra.peff.net","threadId":"33695","inReplyTo":"CALkWK0nL+UvjsUKh6v4x_MaWJmsU6Uh_gh82iE7UT0UE-ZMt0g@mail.gmail.com","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-05-01T21:16:13Z","receivedAt":"2013-05-01T21:16:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, May 02, 2013 at 02:34:01AM +0530, Ramkumar Ramachandra wrote:\n\n> Junio C Hamano wrote:\n> > Just making sure.  HEAD@{$n} and @{$n} for non-negative $n mean\n> > totally different things.  @{0} and HEAD@{0} are almost always the\n> > same, and @{1} and HEAD@{1} may often happen to be the same, but as\n> > a blanket statement, I find \"Since HEAD is implicit in @{}\" very\n> > misleading.\n> \n> When will they be different?  I'm looking at this from the parser's\n> point of view: when the part before @{} is missing, we dwim a \"HEAD\".\n\nThe difference is that HEAD@{} refers to HEAD's reflog, but @{} refers\nto the reflog of the branch pointed to by HEAD. For example, try:\n\n  git init repo && cd repo\n  git commit --allow-empty -m one &&\n  git commit --allow-empty -m two &&\n  git checkout HEAD^ &&\n  git commit --allow-empty -m three &&\n  git checkout master &&\n  for i in 0 1 2; do\n    echo \"HEAD@{$i}: $(git log -1 --oneline HEAD@{$i} 2>&1)\"\n    echo \"    @{$i}: $(git log -1 --oneline     @{$i} 2>&1)\"\n  done\n\nwhich produces:\n\n  HEAD@{0}: 1fbb097 two\n      @{0}: 1fbb097 two\n  HEAD@{1}: 42f3f4d three\n      @{1}: 1089d0e one\n  HEAD@{2}: 1089d0e one\n      @{2}: fatal: Log for '' only has 2 entries.\n\nUnless your reflogs are screwed up, the 0th reflog should always point\nto the same commit (since you just moved HEAD there), but beyond that\nthere is not necessarily any relation. And even for the 0th reflog\nentry, the reflog information is not the same. Try this on the repo\nabove:\n\n  echo \"HEAD@{0}: $(git log -g -1 --format='%gd %gs' HEAD@{0})\"\n  echo \"    @{0}: $(git log -g -1 --format='%gd %gs'     @{0})\"\n\nwhich yields:\n\n  HEAD@{0}: HEAD@{0} checkout: moving from ... to master\n      @{0}: master@{0} commit: two\n\n-Peff\n"},{"id":"216201","messageId":"CALkWK0nYPe2hXLhAqK-GnMZ64N43cTCmdfFhvYdvm92nrQKRNQ@mail.gmail.com","threadId":"33695","inReplyTo":"1367425235-14998-1-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 0/5] A natural solution to the @ -> HEAD problem","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T21:49:35Z","receivedAt":"2013-05-01T21:49:35Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> Ramkumar Ramachandra (5):\n>   t1508 (at-combinations): more tests; document failures\n>   sha1_name.c: don't waste cycles in the @-parsing loop\n>   sha1_name.c: simplify @-parsing in get_sha1_basic()\n>   remote.c: teach branch_get() to treat symrefs other than HEAD\n>   refs.c: make @ a pseudo-ref alias to HEAD\n\nFor your convenience, pull https://github.com/artagnon/git/tree/implicit-head\n"},{"id":"216204","messageId":"CALkWK0nwzCVTjOufkdboGO-09MqBKFnYPJq_KFxKpVD4QYjo-Q@mail.gmail.com","threadId":"33695","inReplyTo":"20130501211613.GA15392@sigill.intra.peff.net","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T22:01:13Z","receivedAt":"2013-05-01T22:01:13Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Jeff King wrote:\n> The difference is that HEAD@{} refers to HEAD's reflog, but @{} refers\n> to the reflog of the branch pointed to by HEAD.\n\nAh, I missed this.  Thanks for the testcase.  My patch changes this\nbehavior, and I'm looking into the problem.\n"},{"id":"216205","messageId":"CALkWK0kCcaQHFjR_j+=SK=ni5ZkXua6akJDYw_U9teS1u22HkQ@mail.gmail.com","threadId":"33695","inReplyTo":"1367425235-14998-4-git-send-email-artagnon@gmail.com","subject":"Re: [PATCH 3/5] sha1_name.c: simplify @-parsing in get_sha1_basic()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T22:06:07Z","receivedAt":"2013-05-01T22:06:07Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> +               if (!len)\n> +                       /* In the @{N} case where there's nothing\n> +                        * before the @.\n> +                        */\n> +                       refs_found = dwim_log(\"HEAD\", 4, sha1, &real_ref);\n\n\nMinor mistake here: it should be dwim_ref, not dwim_log.  Thanks to\nJunio/ Jeff for poking.\n"},{"id":"216208","messageId":"CAMP44s2sRsJR9xxty9F4c7-G3HQrK3N=-o7KBSpG5TYpdhu-9w@mail.gmail.com","threadId":"33695","inReplyTo":"CALkWK0=Z81f4c1X3uXO4O5q_Dj2hRJjSY-i-aDtZ0KqSyo5Wtg@mail.gmail.com","subject":"Re: [PATCH 3/5] sha1_name.c: simplify @-parsing in get_sha1_basic()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-01T22:18:47Z","receivedAt":"2013-05-01T22:18:47Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 1, 2013 at 2:40 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n> Felipe Contreras wrote:\n>> On Wed, May 1, 2013 at 1:36 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n>>> This is not a reorganization patch.  I said \"simplify\": not refactor.\n>>\n>> I'd say you should start with a reorganization, and then a simplification.\n>\n> You don't think I already tried that?  There is no way to sensibly\n> reorganize the old logic sensibly, in a way that doesn't break\n> anything.\n\nThere's always a way.\n\nSee, I tried to split your patch into logical changes, so I started with this:\n\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -460,23 +460,26 @@ 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 (!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+       if (reflog_len) {\n+               if (!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+                       /* allow \"@{...}\" to mean the current branch reflog */\n+                       refs_found = dwim_ref(\"HEAD\", 4, sha1, &real_ref);\n+               } else {\n+                       refs_found = dwim_log(str, len, sha1, &real_ref);\n                }\n-               /* allow \"@{...}\" to mean the current branch reflog */\n-               refs_found = dwim_ref(\"HEAD\", 4, sha1, &real_ref);\n-       } else if (reflog_len)\n-               refs_found = dwim_log(str, len, sha1, &real_ref);\n-       else\n+       } else {\n                refs_found = dwim_ref(str, len, sha1, &real_ref);\n+       }\n\n        if (!refs_found)\n                return -1;\n\nTruly no functional changes at this point, but then this:\n\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -473,7 +473,7 @@ static int get_sha1_basic(const char *str, int\nlen, unsigned char *sha1)\n                                return -1;\n                        }\n                        /* allow \"@{...}\" to mean the current branch reflog */\n-                       refs_found = dwim_ref(\"HEAD\", 4, sha1, &real_ref);\n+                       refs_found = dwim_log(\"HEAD\", 4, sha1, &real_ref);\n                } else {\n                        refs_found = dwim_log(str, len, sha1, &real_ref);\n                }\n\nBam! Now we have a functional change. @{1} is different from HEAD@{1},\nbut this change makes them the same. Not only is this a functional\nchange, it's a behavioral change.\n\nOf course, this would be easy to see if you had bothered to split your\npatch into logical changes, but you didn't, so the change is lost in a\nmess. This is why it's not recommended to do that.\n\n>>> I'm claiming that there is no functional change at the program level,\n>>> with the commenting*.  If you want to disprove my claim, you have to\n>>> write a test that breaks after this patch.\n>>\n>> The burden of proof resides in the one that makes the claim, I don't\n>> need to prove that there are functional changes, merely that you\n>> haven't met your burden of proof.\n>\n> Oh, but I have.  All the tests (along with the new ones I added in [1/5]) pass.\n\nThat is only proof that those tests pass.\n\n> Scientific theories stand until you can find one observation that disproves it.\n\nYeah, I would like to see a scientist claiming \"There are exactly\nthree multiverses. You don't think so? Prove me wrong. Na na na na!\".\nNot going to happen.\n\nYou are the one making a claim, you are the one that has the burden of\nproof, and you haven't met it.\n\nAnd even though I don't have to prove your claim is false, I already\ndid; @{1} is different now. If you want more, see below.\n\n>> That being said, perhaps there are no _behavioral_ changes, because\n>> you are relegating some of the current functionality to dwim_log, but\n>> the code most certainly is doing something completely different.\n>> Before, get_sha1_basic recursively called itself when @{-N} resolved\n>> to a branch name, now, you rely on dwim_log to do this for you without\n>> recursion, which is nice, but functionally different.\n>\n> Your point being?  That I shouldn't say \"no functional changes\"\n> because the logic is changing non-trivially at this level?\n\nExactly. You shouldn't say there are no functional changes, if, in\nfact, there are functional changes.\n\n>>>> It's not true, there might not be any @{u} in there.\n>>>\n>>> This entire structure is a success-filter.  At the end of this, if\n>>> !refs_found, then there has been a failure.\n>>\n>> That's irrelevant, this 'else' case is for !reflog_len, there might or\n>> might not be @{u} in there.\n>\n> There's no need to associate one comment with one line of code.\n> People can see clearly see the failure case following it.\n\nIs that the way you defend your comments? People can see that the\ncomment is wrong?\n\nWhat is the point of the comment then? People can see the context, so\nthere's no need for a comment, specially one that is wrong.\n\n>>> The Git project is already suffering from a severe shortage of\n>>> comments [1], and you're complaining about too many comments?\n>>\n>> Irrelevant conclusion fallacy; let's suppose that the Git project is\n>> indeed suffering from a shortage of comments, that doesn't imply that\n>> *these* comments in their present form are any good.\n>\n> Is there anything _wrong_ with the comments, apart from the fact that\n> they seem to be too big?  I'm saying things that I cannot say in the\n> commit message.\n\nI just pointed to one comment being wrong. Other than that, yeah, they\nare unnecessary.\n\nAside from the fact that there are wrong and unnecessary comments,\nunmentioned functionality changes, there are behavioral changes:\n\n1) @{1} has changed\n\n2) @{-1}@{-1} now doesn't return an error\n\n3) @{-1}{0} returns an invalid object\n\nI wrote a test to show these changes:\n\n*** t1513-at-combinations-strict.sh ***\nok 1 - setup\nok 2 - HEAD = refs/heads/new-branch\nok 3 - @{1} = commit\nok 4 - HEAD@{3} = commit\nnot ok 5 - @{3} is nonsensical\n#\t\n#\t\ttest_must_fail git rev-parse --symbolic '@{3}'\n#\t\nok 6 - @{-1} = refs/heads/old-branch\nok 7 - @{-1}@{1} = commit\nnot ok 8 - @{-1}{0} = commit\n#\t\n#\t\tif [ 'commit' == 'commit' ]; then\n#\t\t\techo 'old-two' >expect &&\n#\t\t\tgit log -1 --format=%s '@{-1}{0}' >actual\n#\t\telse\n#\t\t\techo 'commit' >expect &&\n#\t\t\tgit rev-parse --symbolic-full-name '@{-1}{0}' >actual\n#\t\tfi &&\n#\t\ttest_cmp expect actual\n#\t\nok 9 - @{u} = refs/heads/upstream-branch\nok 10 - @{u}@{1} = commit\nok 11 - @{-1}@{u} = refs/heads/master\nok 12 - @{-1}@{u}@{1} = commit\nok 13 - @{u}@{-1} is nonsensical\nok 14 - @{1}@{u} is nonsensical\nnot ok 15 - @{-1}@{-1} is nonsensical\n#\t\n#\t\ttest_must_fail git rev-parse --symbolic '@{-1}@{-1}'\n#\t\n# failed 3 among 15 test(s)\n1..15\n\nThey all pass without your patch. So yeah, there's a reason you were\nnot able to show fulfill your burden of proof; your claim wasn't true.\n\nHere's the test:\n\n#!/bin/sh\n\ntest_description='test various @{X} syntax combinations together'\n. ./test-lib.sh\n\ncheck() {\ntest_expect_${4:-success} \"$1 = $2\" \"\n\tif [ '$2' == 'commit' ]; then\n\t\techo '$3' >expect &&\n\t\tgit log -1 --format=%s '$1' >actual\n\telse\n\t\techo '$2' >expect &&\n\t\tgit rev-parse --symbolic-full-name '$1' >actual\n\tfi &&\n\ttest_cmp expect actual\n\"\n}\n\nnonsense() {\ntest_expect_${2:-success} \"$1 is nonsensical\" \"\n\ttest_must_fail git rev-parse --symbolic '$1'\n\"\n}\n\ntest_expect_success 'setup' '\n\ttest_commit master-one &&\n\ttest_commit master-two &&\n\tgit checkout -b upstream-branch &&\n\ttest_commit upstream-one &&\n\ttest_commit upstream-two &&\n\tgit checkout -b old-branch master &&\n\ttest_commit old-one &&\n\ttest_commit old-two &&\n\tgit checkout -b new-branch &&\n\ttest_commit new-one &&\n\ttest_commit new-two &&\n\tgit branch -u master old-branch &&\n\tgit branch -u upstream-branch new-branch\n'\n\ncheck HEAD refs/heads/new-branch\ncheck \"@{1}\" commit new-one\ncheck \"HEAD@{3}\" commit old-two\nnonsense \"@{3}\"\ncheck \"@{-1}\" refs/heads/old-branch\ncheck \"@{-1}@{1}\" commit old-one\ncheck \"@{-1}{0}\" commit old-two\ncheck \"@{u}\" refs/heads/upstream-branch\ncheck \"@{u}@{1}\" commit upstream-one\ncheck \"@{-1}@{u}\" refs/heads/master\ncheck \"@{-1}@{u}@{1}\" commit master-one\nnonsense \"@{u}@{-1}\"\nnonsense \"@{1}@{u}\"\nnonsense \"@{-1}@{-1}\"\n\ntest_done\n\n-- \nFelipe Contreras\n"},{"id":"216209","messageId":"CAMP44s2_ccoYaaEZ_qGQ-fOb4aY4UVtqYzDjo8OXmvC-EgYSvQ@mail.gmail.com","threadId":"33695","inReplyTo":"CALkWK0mz6+RS-fZAjz=1+kVN6WFTb+cX98V=8AGfzybacf28RQ@mail.gmail.com","subject":"Re: [PATCH 4/5] remote.c: teach branch_get() to treat symrefs other than HEAD","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-01T22:23:15Z","receivedAt":"2013-05-01T22:23:15Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 1, 2013 at 3:57 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n> Felipe Contreras wrote:\n>> If you are arguing in favor of arbitrary symbolic refs plus @{u} to\n>> work, a patch that allows that, and nothing else, should make sense.\n>> Such patch would trigger further discussion, for example, if\n>> get_branch() is the right place to achieve that.\n>\n> What kind of absurd argument is this?  I am trying to solve the @ ->\n> HEAD problem correctly.  In the process, I fixed general symbolic-ref\n> handling and cleaned up the @-parsing logic.  Does anyone have a\n> problem with the fix, or am I missing something?\n\nThis is not a fix, this is multiple changes blobbed into one.\n\n> So, start the discussion.  What are you waiting for?\n>\n>> I still see this in the code:\n>>\n>>         if (!name || !*name || !strcmp(name, \"HEAD\"))\n>\n> Remove the !strcmp(name, \"HEAD\") and tell me what you see.  \"HEAD\"\n> will get resolved just like any other symbolic ref, via the hunk I\n> added.\n\nIf you are so certain why don't you remove that code then? I wouldn't\nbe so sure.\n\n-- \nFelipe Contreras\n"},{"id":"216211","messageId":"CALkWK0kJj2J6gg-M+CcLdaM1cXhnLGUWCYwUqc+XGdDDyhwo2Q@mail.gmail.com","threadId":"33695","inReplyTo":"CAMP44s2sRsJR9xxty9F4c7-G3HQrK3N=-o7KBSpG5TYpdhu-9w@mail.gmail.com","subject":"Re: [PATCH 3/5] sha1_name.c: simplify @-parsing in get_sha1_basic()","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-01T22:26:26Z","receivedAt":"2013-05-01T22:26:26Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> On Wed, May 1, 2013 at 2:40 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n>> You don't think I already tried that?  There is no way to sensibly\n>> reorganize the old logic sensibly, in a way that doesn't break\n>> anything.\n>\n> See, I tried to split your patch into logical changes, so I started with this:\n>\n> --- a/sha1_name.c\n> +++ b/sha1_name.c\n\nThanks; I was finding this hard to do.  I'll try to continue from here.\n\n> --- a/sha1_name.c\n> +++ b/sha1_name.c\n> @@ -473,7 +473,7 @@ static int get_sha1_basic(const char *str, int\n> len, unsigned char *sha1)\n>                                 return -1;\n>                         }\n>                         /* allow \"@{...}\" to mean the current branch reflog */\n> -                       refs_found = dwim_ref(\"HEAD\", 4, sha1, &real_ref);\n> +                       refs_found = dwim_log(\"HEAD\", 4, sha1, &real_ref);\n\nYeah, I noticed this just a few minutes ago.  We really should have\ntests testing @{N} against HEAD@{N}.\n\n> Of course, this would be easy to see if you had bothered to split your\n> patch into logical changes, but you didn't, so the change is lost in a\n> mess. This is why it's not recommended to do that.\n\nRight.  I'll try to redo this as multiple parts.\n\n>> There's no need to associate one comment with one line of code.\n>> People can see clearly see the failure case following it.\n>\n> Is that the way you defend your comments? People can see that the\n> comment is wrong?\n\nIn that case, all the comments are wrong.  Even the ones about @{N}\nand @{-N}, because we never really check @{\\d+} or @{-\\d+}.  Would you\nlike to make the comments more painful to read and write?\n\n> 2) @{-1}@{-1} now doesn't return an error\n>\n> 3) @{-1}{0} returns an invalid object\n\nThanks for the tests!  I'll look into the problem.\n"},{"id":"216213","messageId":"CAMP44s2rQ1ZQJ2X6HJ6+SkFiA7ip5vh_-n=nNNNs+AFt2Dhbbw@mail.gmail.com","threadId":"33695","inReplyTo":"CALkWK0kJj2J6gg-M+CcLdaM1cXhnLGUWCYwUqc+XGdDDyhwo2Q@mail.gmail.com","subject":"Re: [PATCH 3/5] sha1_name.c: simplify @-parsing in get_sha1_basic()","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-01T22:39:09Z","receivedAt":"2013-05-01T22:39:09Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 1, 2013 at 5:26 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n> Felipe Contreras wrote:\n>> On Wed, May 1, 2013 at 2:40 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n\n>>> There's no need to associate one comment with one line of code.\n>>> People can see clearly see the failure case following it.\n>>\n>> Is that the way you defend your comments? People can see that the\n>> comment is wrong?\n>\n> In that case, all the comments are wrong.  Even the ones about @{N}\n> and @{-N}, because we never really check @{\\d+} or @{-\\d+}.  Would you\n> like to make the comments more painful to read and write?\n\nIf what I see in the code and what I read in the comments tell me\nconflicting stories, I'd say the comments are not fulfilling their\npurpose. Either add comments that explain what the code is _actually_\ndoing, or don't bother with them at all.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"216214","messageId":"7vfvy648g3.fsf@alter.siamese.dyndns.org","threadId":"33695","inReplyTo":"20130501211613.GA15392@sigill.intra.peff.net","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-01T22:54:52Z","receivedAt":"2013-05-01T22:54:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> On Thu, May 02, 2013 at 02:34:01AM +0530, Ramkumar Ramachandra wrote:\n>\n>> Junio C Hamano wrote:\n>> > Just making sure.  HEAD@{$n} and @{$n} for non-negative $n mean\n>> > totally different things.  @{0} and HEAD@{0} are almost always the\n>> > same, and @{1} and HEAD@{1} may often happen to be the same, but as\n>> > a blanket statement, I find \"Since HEAD is implicit in @{}\" very\n>> > misleading.\n>> \n>> When will they be different?  I'm looking at this from the parser's\n>> point of view: when the part before @{} is missing, we dwim a \"HEAD\".\n>\n> The difference is that HEAD@{} refers to HEAD's reflog, but @{} refers\n> to the reflog of the branch pointed to by HEAD. For example, try:\n>\n>   git init repo && cd repo\n>   git commit --allow-empty -m one &&\n>   git commit --allow-empty -m two &&\n>   git checkout HEAD^ &&\n>   git commit --allow-empty -m three &&\n>   git checkout master &&\n>   for i in 0 1 2; do\n>     echo \"HEAD@{$i}: $(git log -1 --oneline HEAD@{$i} 2>&1)\"\n>     echo \"    @{$i}: $(git log -1 --oneline     @{$i} 2>&1)\"\n>   done\n>\n> which produces:\n>\n>   HEAD@{0}: 1fbb097 two\n>       @{0}: 1fbb097 two\n>   HEAD@{1}: 42f3f4d three\n>       @{1}: 1089d0e one\n>   HEAD@{2}: 1089d0e one\n>       @{2}: fatal: Log for '' only has 2 entries.\n>\n> Unless your reflogs are screwed up, the 0th reflog should always point\n> to the same commit (since you just moved HEAD there), but beyond that\n> there is not necessarily any relation. And even for the 0th reflog\n> entry, the reflog information is not the same. Try this on the repo\n> above:\n>\n>   echo \"HEAD@{0}: $(git log -g -1 --format='%gd %gs' HEAD@{0})\"\n>   echo \"    @{0}: $(git log -g -1 --format='%gd %gs'     @{0})\"\n>\n> which yields:\n>\n>   HEAD@{0}: HEAD@{0} checkout: moving from ... to master\n>       @{0}: master@{0} commit: two\n>\n> -Peff\n\nThanks for helping with a basic education I found no time for\nmyself today.\n"},{"id":"216221","messageId":"7vmwse2qmt.fsf@alter.siamese.dyndns.org","threadId":"33695","inReplyTo":"CAMP44s1j6GdQBjgCdBrG3xvXr_XifCTRZWJAQUrzts-MzFg--g@mail.gmail.com","subject":"Re: [PATCH 2/5] sha1_name.c: don't waste cycles in the @-parsing loop","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-02T00:04:58Z","receivedAt":"2013-05-02T00:04:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> On Wed, May 1, 2013 at 11:20 AM, Ramkumar Ramachandra\n> <artagnon@gmail.com> wrote:\n>> The @-parsing loop unnecessarily checks for the sequence \"@{\" from\n>> len - 2 unnecessarily.  We can safely check from len - 4: write out a\n>> comment justifying this.\n>>\n>> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>\n>> ---\n>>  sha1_name.c | 18 +++++++++++++++++-\n>>  1 file changed, 17 insertions(+), 1 deletion(-)\n>>\n>> diff --git a/sha1_name.c b/sha1_name.c\n>> index 3820f28..be1d12c 100644\n>> --- a/sha1_name.c\n>> +++ b/sha1_name.c\n>> @@ -445,7 +445,23 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)\n>>         /* basic@{time or number or -number} format to query ref-log */\n>>         reflog_len = at = 0;\n>>         if (len && str[len-1] == '}') {\n>> -               for (at = len-2; at >= 0; at--) {\n>> +               /* str = @}\n>> +                *       ^\n>> +                *       len - 2; expression is senseless\n>> +                *\n>> +                * str = @{}\n>> +                *       ^\n>> +                *       len - 3; expression is still senseless\n>> +                *\n>> +                * str = @{.}\n>> +                *       ^\n>> +                *       len - 4 where . is any character; expression\n>> +                *       is worth investigating\n>> +                *\n>> +                * Therefore, if str ends with }, search three\n>> +                * characters earlier for @{\n>> +                */\n>\n> I think this comment is overkill.\n>\n>> +               for (at = len - 4; at >= 0; at--) {\n>\n> The change seems OK to me, but there's no need to explain where you\n> are starting, and if there's a need:\n>\n> /* start from where reflogs can start: @{.} */\n>\n> Does the trick nicely.\n\nAs the fact that nobody noticed nor bothered with the two-byte\noptimization opportunity shows that this is trickier than trivial, I\nagree with both of you that this change deserves an in-code comment.\n\n    Start checking at len - 4, because there has to be at least one\n    byte inside \"@{.}\" for it to be worth checking.\n\nwould be sufficient.  The 16-line comment is way overkill.\n\nNot that I think this change really matters, though.\n\n>>                         if (str[at] == '@' && str[at+1] == '{') {\n>>                                 if (!upstream_mark(str + at, len - at)) {\n>>                                         reflog_len = (len-1) - (at+2);\n>> --\n"},{"id":"216226","messageId":"CAMP44s31jyuGGG1Wn9D=D9udkJJdXQfwVAnHuOg02FWinpmn7Q@mail.gmail.com","threadId":"33695","inReplyTo":"7va9oe5y6k.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-02T02:22:28Z","receivedAt":"2013-05-02T02:22:28Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Wed, May 1, 2013 at 1:53 PM, Junio C Hamano <gitster@pobox.com> wrote:\n\n> As you and Felipe seem to be aiming for the same \"Let's allow users\n> to say '@' when they mean HEAD\", I'll let you two figure the best\n> approach out.\n>\n> One productive way forward might be to come up with a common test\n> script pieces to document what constructs that spell @ in place of\n> HEAD should be supported, and much more importantly, what constructs\n> that happen to have @ in them should not mistakenly trigger the new\n> machinery.\n\nThe difference is basically that my approach is less intrusive; it\nonly touches code that is relevant for revision parsing. This means\nthe scope and side-effects are known; the same as @{-N}.\n\nRamkumar's approach has no precedent, so it's not clear what\nside-effects could there be. His approach relies on modifying\nbranch_get() a function that barely has been touched since 2007, and\nhas other side-effects, plus there isn't even a proper patch with the\nisolated change, and an explanation why it makes sense.\n\nWhile it might or might not make sense to teach branch_get() to accept\nsymbolic refs, it's not needed to achieve the desired functionality,\neither on my approach, or his.\n\nMoreover, the symbolic-ref 'HEAD' is quite special, it's mentioned\neverywhere in the documentation, and the code has special cases for\nit. It's not reasonable to expect all relevant places to be updated\nfor this functionality, and certainly 'Documentation/revisions.txt' is\nnot the only one.\n\nFor example, in Ramkumar's approach:\n\n % git branch -u master @\n\nWould replace '@' with HEAD, however:\n\n % git branch --edit-description @\n\nWould not. These inconsistencies are not due to Ramkumar's code, but\nwhy would the user expect '@' to be replaced with anything if 'git\nbranch' documentation doesn't mention any revision parsing, which is\nthe only place where the '@' shortcut is documented.\n\nIn my opinion, if 'git branch X @{-1}' doesn't work, neither should\n'git branch X @', and that's what my approach does.\n\nMy approach is isolated to revision parsing, which means we minimize\nthe potential for surprises and unintended consequences.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"216236","messageId":"CALkWK0=B_Ym_ei181eV-WeqJZJyHPv-AmLzYwajm13wftJkB6g@mail.gmail.com","threadId":"33695","inReplyTo":"CAMP44s31jyuGGG1Wn9D=D9udkJJdXQfwVAnHuOg02FWinpmn7Q@mail.gmail.com","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-02T09:07:46Z","receivedAt":"2013-05-02T09:07:46Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> [...]\n\nYes, I'm working on a re-roll.\n\n> Moreover, the symbolic-ref 'HEAD' is quite special, it's mentioned\n> everywhere in the documentation, and the code has special cases for\n> it. It's not reasonable to expect all relevant places to be updated\n> for this functionality, and certainly 'Documentation/revisions.txt' is\n> not the only one.\n\nI'm not denying that HEAD is special.  I'm just improving the general\nsupport for symbolic-refs, so that the difference between HEAD and any\nother symbolic-ref is smaller.\n\n> For example, in Ramkumar's approach:\n>\n>  % git branch -u master @\n>\n> Would replace '@' with HEAD, however:\n>\n>  % git branch --edit-description @\n>\n> Would not.\n\ngit branch -u master <any symbolic ref> will work just fine.  It has\nnothing to do with my series.\n\nDoes git branch --edit-description HEAD work?  Then why do you expect\ngit branch --edit-description <any other symbolic ref> to work?  git\nbranch --edit-description operates on non-symbolic refs.\n\nLet me make this clear: @ is just another symbolic-ref that always\npoints to the same thing as HEAD.  Nothing less, nothing more.\n\n> In my opinion, if 'git branch X @{-1}' doesn't work, neither should\n> 'git branch X @', and that's what my approach does.\n\nWhy shouldn't (doesn't) git branch X @{-1} work?  git branch X <any\nexpression with or without a symbolic ref> works fine, and it has\nnothing to do with my series.\n"},{"id":"216238","messageId":"CAMP44s1x8pu2B2h3wKmuBDXz3Tqyk9qyijshcD6iN+NpYDCQdQ@mail.gmail.com","threadId":"33695","inReplyTo":"CALkWK0=B_Ym_ei181eV-WeqJZJyHPv-AmLzYwajm13wftJkB6g@mail.gmail.com","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-02T09:45:00Z","receivedAt":"2013-05-02T09:45:00Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, May 2, 2013 at 4:07 AM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n> Felipe Contreras wrote:\n>> [...]\n>\n> Yes, I'm working on a re-roll.\n>\n>> Moreover, the symbolic-ref 'HEAD' is quite special, it's mentioned\n>> everywhere in the documentation, and the code has special cases for\n>> it. It's not reasonable to expect all relevant places to be updated\n>> for this functionality, and certainly 'Documentation/revisions.txt' is\n>> not the only one.\n>\n> I'm not denying that HEAD is special.  I'm just improving the general\n> support for symbolic-refs, so that the difference between HEAD and any\n> other symbolic-ref is smaller.\n\nThat is orthogonal to the issue at hand.\n\n>> For example, in Ramkumar's approach:\n>>\n>>  % git branch -u master @\n>>\n>> Would replace '@' with HEAD, however:\n>>\n>>  % git branch --edit-description @\n>>\n>> Would not.\n>\n> git branch -u master <any symbolic ref> will work just fine.  It has\n> nothing to do with my series.\n\n% git symbolic-ref TEST refs/heads/master\n% git branch -u origin/master TEST\nfatal: branch 'TEST' does not exist\n\nYou mean it will work just fine _after_ your patch series. So it has\neverything to do with your series.\n\n> Does git branch --edit-description HEAD work?  Then why do you expect\n> git branch --edit-description <any other symbolic ref> to work?  git\n> branch --edit-description operates on non-symbolic refs.\n>\n> Let me make this clear: @ is just another symbolic-ref that always\n> points to the same thing as HEAD.  Nothing less, nothing more.\n\nBut HEAD is special, @ is not. HEAD is documented, @ is not.\n\nWhere is it documented that @ points to HEAD? Where is it documented\nthat 'branch -u foo @' would replace @ with HEAD?\nDocumentation/revisions.txt? Sorry, 'git branch -u foo' does not parse\nrevisions, so that's not the answer. And there's many other places\nthat don't do revision parsing either.\n\nYour approach is more like a hack, it has the consequences we want,\nbut it has many other unintentional and undocumented consequences.\n\nIf I find a single place where 'HEAD' is hard-coded, and your patch\ndoesn't replace '@' correctly, would you then accept that there are\nunintentional consequences, and that this approach is no the best\nprecisely for that reason?\n\n>> In my opinion, if 'git branch X @{-1}' doesn't work, neither should\n>> 'git branch X @', and that's what my approach does.\n>\n> Why shouldn't (doesn't) git branch X @{-1} work?\n\nThat's a red herring. The fact is that right now it doesn't work.\n\nMy patch does what it says it does, and documents what it does, and\ndoes not mess with anything else.\n\n> git branch X <any\n> expression with or without a symbolic ref> works fine, and it has\n> nothing to do with my series.\n\nNo, it doesn't.\n\n% git symbolic-ref TEST refs/heads/master\n% git branch -u origin/master TEST\nfatal: branch 'TEST' does not exist\n% git branch --edit-description TEST\nerror: No branch named 'TEST'.\n\n-- \nFelipe Contreras\n"},{"id":"216245","messageId":"CALkWK0k5zyGwZaP84YMON8VZbfjRxQ3PG2ML2ds8GdQY38s7SA@mail.gmail.com","threadId":"33695","inReplyTo":"CAMP44s1x8pu2B2h3wKmuBDXz3Tqyk9qyijshcD6iN+NpYDCQdQ@mail.gmail.com","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-02T11:03:21Z","receivedAt":"2013-05-02T11:03:21Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> But HEAD is special, @ is not. HEAD is documented, @ is not.\n\nYour point being?  That we should document @?  Yes, I agree.\n\n> Where is it documented that @ points to HEAD? Where is it documented\n> that 'branch -u foo @' would replace @ with HEAD?\n> Documentation/revisions.txt? Sorry, 'git branch -u foo' does not parse\n> revisions, so that's not the answer. And there's many other places\n> that don't do revision parsing either.\n\nYou're confusing parsing revisions with parsing symbolic-refs.  I've\ndocumented @ right next to HEAD, FETCH_HEAD and the other symbolic\nrefs in revisions.txt.  Yes, we have to update the documentation of\ncommands like 'git branch -u foo' to make it clear that they can\noperate on symbolic-refs (not just \"HEAD\") that point to branches, not\njust plain branch refs.\n\nMaybe even a fresh page on symbolic refs?\n\n> Your approach is more like a hack,\n\nNow you're just saying rubbish.  Neither of the approaches is more of\na \"hack\" than the other.  You've implemented @ as a revision, while\nI've implemented it as a symbolic-ref.\n\nYour approach requires less effort to document, and my approach yields\nan implementation that is almost trivial.  That is not the basis for\ndetermining which approach is \"better\".\n\n> it has the consequences we want,\n> but it has many other unintentional and undocumented consequences.\n\nWho said it wasn't intentional?  Yes, I agree with your criticism\nabout it being undocumented: please help fixing this.\n\nIt's very much intentional.  I _want_ these to work:\n\n% git symbolic-ref M refs/heads/master\n% git show M@{u}\n% git branch -u ram/master M\n\nIn other words, I want commands that operate on \"HEAD\" to also operate\non other symbolic refs similarly.  Is this an unreasonable request?\n\n> If I find a single place where 'HEAD' is hard-coded, and your patch\n> doesn't replace '@' correctly, would you then accept that there are\n> unintentional consequences, and that this approach is no the best\n> precisely for that reason?\n\nYou'd have found a bug then, and we must fix it.  Why are you throwing\nuseful features out the window simply because of difficulty of\ndocumentation/ historical inertia?\n\n> Ramkumar Ramachandra wrote:\n>> git branch X <any\n>> expression with or without a symbolic ref> works fine, and it has\n>> nothing to do with my series.\n>\n> No, it doesn't.\n>\n> % git symbolic-ref TEST refs/heads/master\n> % git branch -u origin/master TEST\n> fatal: branch 'TEST' does not exist\n> % git branch --edit-description TEST\n> error: No branch named 'TEST'.\n\nAre you reading what you're responding to?  I said:\n\n% git branch X @{-1}\n% git branch X HEAD\n% git symbolic-ref M refs/heads/master\n% git branch X M\n\nWill work with or without my patch.  This is because git branch <1>\n<2> runs <2> through the revision parser.\n\nThis will work with my patch:\n\n% git symbolic-ref M refs/heads/master\n% git branch -u origin/master M\n\nprecisely because:\n\n% git branch -u origin/master HEAD\n\nworks.  And precisely because this does not:\n\n% git branch -u origin/master @{-1}\n\nIn other words, git branch -u <1> <2> expects <2> to be a ref or\nsymbolic-ref (currently limited to \"HEAD\"), not a revision.  It\ndoesn't run <2> through the revision parser to check if it resolves to\na ref.\n\nThe following will not work:\n\n% git symbolic-ref M refs/heads/master\n% git branch --edit-description M\n\nprecisely because:\n\n% git branch --edit-description HEAD\n\ndoes not work.  This is because git branch --edit-description <1>\nexpects <1> to be a non-symbolic ref.  It doesn't even hard-code\n\"HEAD\".\n\nWhy are you blaming my patch for existing inconsistencies in the UI?\n\nThere is one limitation worth nothing: a symbolic-ref can only point\nto a ref (or another symbolic ref).  The revision parser doesn't kick\nin at the resolve_ref_unsafe() stage.  So, it's quite non-trivial to\nimplement what Thomas asked for (git symbolic-ref U @{u}).  However, I\nthink my series is one step in the right direction.  I'd really love\nsymbolic refs I can take along with me (so M -> master can be in my\n.gitconfig): we have to hook resolve_ref_unsafe() to the config API to\nachieve this.\n\nIn other words, I'm thinking about the future of symbolic refs.\n"},{"id":"216246","messageId":"CALkWK0n0e563Bjw=tXww9hDckv3Rekvp6u0m2uJ8UMrqsFOKMQ@mail.gmail.com","threadId":"33695","inReplyTo":"CALkWK0k5zyGwZaP84YMON8VZbfjRxQ3PG2ML2ds8GdQY38s7SA@mail.gmail.com","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-02T11:36:50Z","receivedAt":"2013-05-02T11:36:50Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> [...]\n\nDisclaimer: I'm not saying that my implementation is Correct and\nFinal.  I will be more thorough in my re-roll about justifying my\nchanges.\n\nWhat I am saying is that we should fix symbolic refs, and that @\nshould be implemented at the ref-level to maximize usefulness.\n"},{"id":"216272","messageId":"CAMP44s2nqLrLRFM1bH028FpV+tPYUWYDz_AffaBhVfwwm4h=kg@mail.gmail.com","threadId":"33695","inReplyTo":"CALkWK0k5zyGwZaP84YMON8VZbfjRxQ3PG2ML2ds8GdQY38s7SA@mail.gmail.com","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-02T16:45:57Z","receivedAt":"2013-05-02T16:45:57Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, May 2, 2013 at 6:03 AM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:\n> Felipe Contreras wrote:\n>> But HEAD is special, @ is not. HEAD is documented, @ is not.\n>\n> Your point being?  That we should document @?  Yes, I agree.\n\nWhere?\n\n>> Where is it documented that @ points to HEAD? Where is it documented\n>> that 'branch -u foo @' would replace @ with HEAD?\n>> Documentation/revisions.txt? Sorry, 'git branch -u foo' does not parse\n>> revisions, so that's not the answer. And there's many other places\n>> that don't do revision parsing either.\n>\n> You're confusing parsing revisions with parsing symbolic-refs.  I've\n> documented @ right next to HEAD, FETCH_HEAD and the other symbolic\n> refs in revisions.txt.  Yes, we have to update the documentation of\n> commands like 'git branch -u foo' to make it clear that they can\n> operate on symbolic-refs (not just \"HEAD\") that point to branches, not\n> just plain branch refs.\n\nFETCH_HEAD is not a symbolic ref, there's no other special symbolic\nref, HEAD is the only one.\n\n> Maybe even a fresh page on symbolic refs?\n\nJust for '@'? Then what? Are you going to manually inspect the whole\ncode-base to find in which instances is '@' going to be resoled, and\nlink the documentation of that command to this new symbolic ref?\n\nGo ahead, let's see what you come up with.\n\n>> Your approach is more like a hack,\n>\n> Now you're just saying rubbish.  Neither of the approaches is more of\n> a \"hack\" than the other.  You've implemented @ as a revision, while\n> I've implemented it as a symbolic-ref.\n\nThe former has precedents, the later doesn't. The former can be easily\ndocumented, the later can't. The former is a simple single patch, the\nlater is bound to dubious changes. The former has no unintended\nconsequences, the later can have many.\n\nSo yeah, I'd say the later is a hack.\n\n> Your approach requires less effort to document,\n\nYour approach can NOT be documented.\n\n> and my approach yields\n> an implementation that is almost trivial.\n\nNo, it requires multiple changes, THIS is trivial:\n\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -1025,6 +1025,13 @@ int interpret_branch_name(const char *name,\nstruct strbuf *buf)\n        cp = strchr(name, '@');\n        if (!cp)\n                return -1;\n+\n+       if (cp == name && name[1] != '{') {\n+               strbuf_reset(buf);\n+               strbuf_add(buf, \"HEAD\", 4);\n+               return reinterpret(name, namelen, 1, buf);\n+       }\n+\n        tmp_len = upstream_mark(cp, namelen - (cp - name));\n        if (!tmp_len)\n                return -1;\n\nThat's it, that's the whole patch that implements my approach.\n\n> That is not the basis for\n> determining which approach is \"better\".\n\nYes it is. If it can be easily documented it's a sign that you are\ndoing it in the right place. If it's *impossible* to document, it's a\nsign that what you are doing is a hack.\n\nIf you need multiple changes all over the place it's a sign that what\nyou are doing is not a natural extension of anything. If it follows\nnaturally from what people have done in the past, it's a sign that it\nis a natural extension.\n\n>> it has the consequences we want,\n>> but it has many other unintentional and undocumented consequences.\n>\n> Who said it wasn't intentional?  Yes, I agree with your criticism\n> about it being undocumented: please help fixing this.\n\nWhy? Why document a hack all over the place? Especially when we do NOT\nhave to. We don't need this hack.\n\n> It's very much intentional.  I _want_ these to work:\n>\n> % git symbolic-ref M refs/heads/master\n> % git show M@{u}\n> % git branch -u ram/master M\n>\n> In other words, I want commands that operate on \"HEAD\" to also operate\n> on other symbolic refs similarly.  Is this an unreasonable request?\n\nThat has absolutely nothing to do with the '@' shortcut. Nothing at all.\n\nThe mere fact that we are discussing about random symbolic refs is\nproof that you can't focus on the issue at hand.\n\n>> If I find a single place where 'HEAD' is hard-coded, and your patch\n>> doesn't replace '@' correctly, would you then accept that there are\n>> unintentional consequences, and that this approach is no the best\n>> precisely for that reason?\n>\n> You'd have found a bug then, and we must fix it.\n\nSo that's it. There's absolutely no way to convince you that your\napproach is the wrong one. No amount of evidence to the contrary will\nchange your mind.\n\nThere's no point in discussing then.\n\n> Why are you throwing\n> useful features out the window simply because of difficulty of\n> documentation/ historical inertia?\n\nI'm not throwing any useful features, I'm implementing them correctly.\n\n>> Ramkumar Ramachandra wrote:\n>>> git branch X <any\n>>> expression with or without a symbolic ref> works fine, and it has\n>>> nothing to do with my series.\n>>\n>> No, it doesn't.\n>>\n>> % git symbolic-ref TEST refs/heads/master\n>> % git branch -u origin/master TEST\n>> fatal: branch 'TEST' does not exist\n>> % git branch --edit-description TEST\n>> error: No branch named 'TEST'.\n>\n> Are you reading what you're responding to?  I said:\n>\n> % git branch X @{-1}\n> % git branch X HEAD\n> % git symbolic-ref M refs/heads/master\n> % git branch X M\n>\n> Will work with or without my patch.  This is because git branch <1>\n> <2> runs <2> through the revision parser.\n\nThe branch commands that run through the revision parser are\nirrelevant. 'git branch foo @' will work with my patch too.\n\n> This will work with my patch:\n>\n> % git symbolic-ref M refs/heads/master\n> % git branch -u origin/master M\n\nWhich is irrelevant to the discussion about '@'.\n\n> precisely because:\n>\n> % git branch -u origin/master HEAD\n>\n> works.  And precisely because this does not:\n>\n> % git branch -u origin/master @{-1}\n>\n> In other words, git branch -u <1> <2> expects <2> to be a ref or\n> symbolic-ref (currently limited to \"HEAD\"), not a revision.  It\n> doesn't run <2> through the revision parser to check if it resolves to\n> a ref.\n>\n> The following will not work:\n>\n> % git symbolic-ref M refs/heads/master\n> % git branch --edit-description M\n>\n> precisely because:\n>\n> % git branch --edit-description HEAD\n>\n> does not work.  This is because git branch --edit-description <1>\n> expects <1> to be a non-symbolic ref.  It doesn't even hard-code\n> \"HEAD\".\n>\n> Why are you blaming my patch for existing inconsistencies in the UI?\n\nYour patch is introducing more, and you can NOT document all the\nchanges you are introducing.\n\n> There is one limitation worth nothing: a symbolic-ref can only point\n> to a ref (or another symbolic ref).  The revision parser doesn't kick\n> in at the resolve_ref_unsafe() stage.  So, it's quite non-trivial to\n> implement what Thomas asked for (git symbolic-ref U @{u}).  However, I\n> think my series is one step in the right direction.  I'd really love\n> symbolic refs I can take along with me (so M -> master can be in my\n> .gitconfig): we have to hook resolve_ref_unsafe() to the config API to\n> achieve this.\n>\n> In other words, I'm thinking about the future of symbolic refs.\n\nThat has absolutely nothing to do with the '@' shortcut.\n\nSince you can't talk about the issue of the '@' without mentioning\nother random symlinks, it's basically impossible to have a fruitful\ndiscussion.\n\nAnd since there's absolutely no evidence or logical argument that\nwould convince you that your approach is not the right one, there is\nno point in discussing.\n\n-- \nFelipe Contreras\n"},{"id":"216275","messageId":"CALkWK0m=gmf9g_O3uHmpGkSa165G-5VGROTgjQx87XwvDDQKSg@mail.gmail.com","threadId":"33695","inReplyTo":"CAMP44s2nqLrLRFM1bH028FpV+tPYUWYDz_AffaBhVfwwm4h=kg@mail.gmail.com","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-02T16:56:55Z","receivedAt":"2013-05-02T16:56:55Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> Your approach can NOT be documented.\n\nAh, I missed that.  The explanation I was looking for is:\n\nHEAD has been special right from the start, and we cannot elevate\nanything else to its status now.\n\nThanks.  And sorry it took me so long.\n"},{"id":"216277","messageId":"CAMP44s1yMR5zrWycdfrToTUQ1j_aTFbySgn_gvROAvjUH1eN-g@mail.gmail.com","threadId":"33695","inReplyTo":"CALkWK0m=gmf9g_O3uHmpGkSa165G-5VGROTgjQx87XwvDDQKSg@mail.gmail.com","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-02T17:01:34Z","receivedAt":"2013-05-02T17:01:34Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, May 2, 2013 at 11:56 AM, Ramkumar Ramachandra\n<artagnon@gmail.com> wrote:\n> Felipe Contreras wrote:\n>> Your approach can NOT be documented.\n>\n> Ah, I missed that.  The explanation I was looking for is:\n>\n> HEAD has been special right from the start, and we cannot elevate\n> anything else to its status now.\n\nWe can, but do we _need_ to? Adding another magical symbolic ref that\nhas limited scope should not be a huge task, as the effects can be\ndocumented, but another magical symbolic ref that does *exactly* the\nsame as HEAD would require too many changes in the code and\ndocumentation it's not even worth considering, especially if we don't\n_need_ to.\n\n-- \nFelipe Contreras\n"},{"id":"216278","messageId":"CALkWK0nqoffHD3mfRqzanEuh4yG-cCO-YBGw8ApnG2uLKJaenw@mail.gmail.com","threadId":"33695","inReplyTo":"CALkWK0m=gmf9g_O3uHmpGkSa165G-5VGROTgjQx87XwvDDQKSg@mail.gmail.com","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-02T17:02:41Z","receivedAt":"2013-05-02T17:02:41Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Ramkumar Ramachandra wrote:\n> HEAD has been special right from the start, and we cannot elevate\n> anything else to its status now.\n\nAs to why I agree with you: I audited the callers of branch_get() and\nfound out there are some things that cannot be fixed just by fixing\nbranch_get():\n\nFor instance, 'git fetch origin M' will not work, and it's not\nbranch_get()'s fault.  And that is just one instance: there are tons\nof other instances; and going and changing each one of them will take\nforever, and totally not be worth the pain.\n\nIn other words; symbolic refs are a dead end.  The future is in\ngetting everything to use the standard revision parser.\n"},{"id":"216280","messageId":"CAMP44s2suBKMxKTPsOC15PxNrVZKCxDVz=0aaYc2v5h-DMHKRg@mail.gmail.com","threadId":"33695","inReplyTo":"CALkWK0nqoffHD3mfRqzanEuh4yG-cCO-YBGw8ApnG2uLKJaenw@mail.gmail.com","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2013-05-02T17:08:44Z","receivedAt":"2013-05-02T17:08:44Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Thu, May 2, 2013 at 12:02 PM, Ramkumar Ramachandra\n<artagnon@gmail.com> wrote:\n> Ramkumar Ramachandra wrote:\n>> HEAD has been special right from the start, and we cannot elevate\n>> anything else to its status now.\n>\n> As to why I agree with you: I audited the callers of branch_get() and\n> found out there are some things that cannot be fixed just by fixing\n> branch_get():\n>\n> For instance, 'git fetch origin M' will not work, and it's not\n> branch_get()'s fault.  And that is just one instance: there are tons\n> of other instances; and going and changing each one of them will take\n> forever, and totally not be worth the pain.\n\nWe probably should fix those, but that is orthogonal to the '@' shortcut.\n\nWe can have the '@' shortcut *today*, with minimal changes to the code\nand the documentation, in a limited and understood scope, with no\nsurprises.\n\nWe can fix the symbolic ref stuff slowly, step by step, no need to\ndelay the '@' shortcut for that.\n\n-- \nFelipe Contreras\n"},{"id":"216281","messageId":"CALkWK0mhHDN3fAydTQN_wDhVzfiqo-UiRFG3eajAWk+S2dHGSw@mail.gmail.com","threadId":"33695","inReplyTo":"CAMP44s2suBKMxKTPsOC15PxNrVZKCxDVz=0aaYc2v5h-DMHKRg@mail.gmail.com","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"Ramkumar Ramachandra","fromEmail":"artagnon@gmail.com","sentAt":"2013-05-02T17:09:46Z","receivedAt":"2013-05-02T17:09:46Z","isPatch":true,"sender":{"key":"r@artagnon.com","avatar":"https://avatars.githubusercontent.com/u/37226?v=4"},"body":"Felipe Contreras wrote:\n> We probably should fix those, but that is orthogonal to the '@' shortcut.\n>\n> We can have the '@' shortcut *today*, with minimal changes to the code\n> and the documentation, in a limited and understood scope, with no\n> surprises.\n>\n> We can fix the symbolic ref stuff slowly, step by step, no need to\n> delay the '@' shortcut for that.\n\nAgreed.\n"},{"id":"216406","messageId":"CAJDDKr7JxmfCDSdPCFhK87g8AuuBexLn-v6SkQh6k0uAbH7j0w@mail.gmail.com","threadId":"33695","inReplyTo":"CALkWK0mhHDN3fAydTQN_wDhVzfiqo-UiRFG3eajAWk+S2dHGSw@mail.gmail.com","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2013-05-04T08:10:10Z","receivedAt":"2013-05-04T08:10:10Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Thu, May 2, 2013 at 10:09 AM, Ramkumar Ramachandra\n<artagnon@gmail.com> wrote:\n> Felipe Contreras wrote:\n>> We probably should fix those, but that is orthogonal to the '@' shortcut.\n>>\n>> We can have the '@' shortcut *today*, with minimal changes to the code\n>> and the documentation, in a limited and understood scope, with no\n>> surprises.\n>>\n>> We can fix the symbolic ref stuff slowly, step by step, no need to\n>> delay the '@' shortcut for that.\n>\n> Agreed.\n\nIf only we didn't care about windows then we could have this feature today:\n\n    cd .git\n    ln -s HEAD @\n\n...and everything works, in the most natural way.\nHas anyone else tweaked their repo this way?\n\nIs an alternative implementation to change the template repo to ship\nwith a symlink?\n\nFor windows, perhaps we can use this new code behind an #ifdef?\n\nAnyways, it's just a crazy idea.  I very much like this feature,\nand in a tweaked repo @{0}{1} actually works.\n\nIs there no way to tweak this at some really low level to trick git\ninto believing the link exists (even when it doesn't)?\n\nI guess that's what these patches do, but the limitations seem unfortunate.\n--\nDavid\n"},{"id":"216407","messageId":"CAJDDKr4CQ9D8RewcR4uMd4iGS3HtKo+-knex+NshVxmc_4517Q@mail.gmail.com","threadId":"33695","inReplyTo":"CAJDDKr7JxmfCDSdPCFhK87g8AuuBexLn-v6SkQh6k0uAbH7j0w@mail.gmail.com","subject":"Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2013-05-04T08:16:51Z","receivedAt":"2013-05-04T08:16:51Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sat, May 4, 2013 at 1:10 AM, David Aguilar <davvid@gmail.com> wrote:\n> On Thu, May 2, 2013 at 10:09 AM, Ramkumar Ramachandra\n> <artagnon@gmail.com> wrote:\n>> Felipe Contreras wrote:\n>>> We probably should fix those, but that is orthogonal to the '@' shortcut.\n>>>\n>>> We can have the '@' shortcut *today*, with minimal changes to the code\n>>> and the documentation, in a limited and understood scope, with no\n>>> surprises.\n>>>\n>>> We can fix the symbolic ref stuff slowly, step by step, no need to\n>>> delay the '@' shortcut for that.\n>>\n>> Agreed.\n>\n> If only we didn't care about windows then we could have this feature today:\n>\n>     cd .git\n>     ln -s HEAD @\n>\n> ...and everything works, in the most natural way.\n> Has anyone else tweaked their repo this way?\n>\n> Is an alternative implementation to change the template repo to ship\n> with a symlink?\n>\n> For windows, perhaps we can use this new code behind an #ifdef?\n>\n> Anyways, it's just a crazy idea.  I very much like this feature,\n> and in a tweaked repo @{0}{1} actually works.\n>\n> Is there no way to tweak this at some really low level to trick git\n> into believing the link exists (even when it doesn't)?\n>\n> I guess that's what these patches do, but the limitations seem unfortunate.\n\nNevermind.  @{0}{1} is nothing special and symbolic-ref happily\nreplaces symlinks with a file, so new code is needed regardless.  I'll\nstart testing these patches instead.\n--\nDavid\n"}]}