{"thread":{"id":"38776","subject":"[JFF] \"-\" and \"@{-1}\" on various programs","startedAt":"2015-03-10T20:20:18Z","lastAt":"2015-03-11T20:16:43Z","messageCount":3,"participants":["Junio C Hamano","Michael J Gruber"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"257494","messageId":"xmqqy4n4zjst.fsf@gitster.dls.corp.google.com","threadId":"38776","inReplyTo":null,"subject":"[JFF] \"-\" and \"@{-1}\" on various programs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-10T20:20:18Z","receivedAt":"2015-03-10T20:20:18Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"JFF stands for just for fun.\n\nThis is not meant to give out a model answer and is known to be\nincomplete, but I was wondering if it would be a better direction to\nallow \"-\" as a stand-in for \"@{-1}\" everywhere we allow a branch\nname, losing workarounds at the surface level we have for checkout,\nmerge and revert.\n\nThe first three paths are to remove the surface workarounds that\nbecome unnecessary.  The one in sha1_name.c is the central change.\n\nThe change in revision.c is to allow a single \"-\" to be recognized\nas a potential revision name (without this change, what begins with\n\"-\" is either an option or an unknown option).\n\nSo you could do things like \"git reset - $path\" but also things like\n\"git log -\" after switching out of a branch.\n\nWhat does not work are what needs further tweaking in revision.c\nparser.  \"git checkout master && git checkout next && git log -..\"\nshould show what next has on top of master but I didn't touch the\nrange notation so it does not work, for example.\n\n builtin/checkout.c |  3 ---\n builtin/merge.c    |  3 +--\n builtin/revert.c   |  2 --\n revision.c         |  2 +-\n sha1_name.c        | 57 +++++++++++++++++++++++++++++++++---------------------\n 5 files changed, 37 insertions(+), 30 deletions(-)\n\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex 7e2d144..8f52a92 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -1169,9 +1169,6 @@ static int parse_branchname_arg(int argc, const char **argv,\n \telse if (dash_dash_pos >= 2)\n \t\tdie(_(\"only one reference expected, %d given.\"), dash_dash_pos);\n \n-\tif (!strcmp(arg, \"-\"))\n-\t\targ = \"@{-1}\";\n-\n \tif (get_sha1_mb(arg, rev)) {\n \t\t/*\n \t\t * Either case (3) or (4), with <something> not being\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 90dd5e6..569dda3 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -1164,8 +1164,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)\n \t\t\t\targc = setup_with_upstream(&argv);\n \t\t\telse\n \t\t\t\tdie(_(\"No commit specified and merge.defaultToUpstream not set.\"));\n-\t\t} else if (argc == 1 && !strcmp(argv[0], \"-\"))\n-\t\t\targv[0] = \"@{-1}\";\n+\t\t}\n \t}\n \tif (!argc)\n \t\tusage_with_options(builtin_merge_usage,\ndiff --git a/builtin/revert.c b/builtin/revert.c\nindex 56a2c36..dc98b4e 100644\n--- a/builtin/revert.c\n+++ b/builtin/revert.c\n@@ -170,8 +170,6 @@ static void parse_args(int argc, const char **argv, struct replay_opts *opts)\n \t\topts->revs->no_walk = REVISION_WALK_NO_WALK_UNSORTED;\n \t\tif (argc < 2)\n \t\t\tusage_with_options(usage_str, options);\n-\t\tif (!strcmp(argv[1], \"-\"))\n-\t\t\targv[1] = \"@{-1}\";\n \t\tmemset(&s_r_opt, 0, sizeof(s_r_opt));\n \t\ts_r_opt.assume_dashdash = 1;\n \t\targc = setup_revisions(argc, argv, opts->revs, &s_r_opt);\ndiff --git a/revision.c b/revision.c\nindex e1321d5..17709a1 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -2195,7 +2195,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \tread_from_stdin = 0;\n \tfor (left = i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n-\t\tif (*arg == '-') {\n+\t\tif (arg[0] == '-' && arg[1]) {\n \t\t\tint opts;\n \n \t\t\topts = handle_revision_pseudo_opt(submodule,\ndiff --git a/sha1_name.c b/sha1_name.c\nindex 95f9f8f..7a621ba 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -483,6 +483,8 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1,\n \t\t\t\tbreak;\n \t\t\t}\n \t\t}\n+\t} else if (len == 1 && str[0] == '-') {\n+\t\tnth_prior = 1;\n \t}\n \n \t/* Accept only unambiguous ref paths. */\n@@ -491,13 +493,16 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1,\n \n \tif (nth_prior) {\n \t\tstruct strbuf buf = STRBUF_INIT;\n-\t\tint detached;\n+\t\tint status;\n \n \t\tif (interpret_nth_prior_checkout(str, len, &buf) > 0) {\n-\t\t\tdetached = (buf.len == 40 && !get_sha1_hex(buf.buf, sha1));\n+\t\t\tif (get_sha1(buf.buf, sha1))\n+\t\t\t\t/* bad---the previous branch no longer exists? */\n+\t\t\t\tstatus = -1;\n+\t\t\telse\n+\t\t\t\tstatus = 0; /* detached */\n \t\t\tstrbuf_release(&buf);\n-\t\t\tif (detached)\n-\t\t\t\treturn 0;\n+\t\t\treturn status;\n \t\t}\n \t}\n \n@@ -931,35 +936,43 @@ static int interpret_nth_prior_checkout(const char *name, int namelen,\n \t\t\t\t\tstruct strbuf *buf)\n {\n \tlong nth;\n-\tint retval;\n+\tint consumed;\n \tstruct grab_nth_branch_switch_cbdata cb;\n-\tconst char *brace;\n-\tchar *num_end;\n \n-\tif (namelen < 4)\n-\t\treturn -1;\n-\tif (name[0] != '@' || name[1] != '{' || name[2] != '-')\n-\t\treturn -1;\n-\tbrace = memchr(name, '}', namelen);\n-\tif (!brace)\n-\t\treturn -1;\n-\tnth = strtol(name + 3, &num_end, 10);\n-\tif (num_end != brace)\n-\t\treturn -1;\n-\tif (nth <= 0)\n-\t\treturn -1;\n+\tif (namelen == 1 && name[0] == '-') {\n+\t\tnth = 1;\n+\t\tconsumed = 1;\n+\t} else {\n+\t\tconst char *brace;\n+\t\tchar *num_end;\n+\n+\t\tif (namelen < 4)\n+\t\t\treturn -1;\n+\t\tif (name[0] != '@' || name[1] != '{' || name[2] != '-')\n+\t\t\treturn -1;\n+\t\tbrace = memchr(name, '}', namelen);\n+\t\tif (!brace)\n+\t\t\treturn -1;\n+\t\tnth = strtol(name + 3, &num_end, 10);\n+\t\tif (num_end != brace)\n+\t\t\treturn -1;\n+\t\tif (nth <= 0)\n+\t\t\treturn -1;\n+\t\tconsumed = brace - name + 1;\n+\t}\n+\n \tcb.remaining = nth;\n \tstrbuf_init(&cb.buf, 20);\n \n-\tretval = 0;\n \tif (0 < for_each_reflog_ent_reverse(\"HEAD\", grab_nth_branch_switch, &cb)) {\n \t\tstrbuf_reset(buf);\n \t\tstrbuf_addbuf(buf, &cb.buf);\n-\t\tretval = brace - name + 1;\n+\t} else {\n+\t\tconsumed = 0;\n \t}\n \n \tstrbuf_release(&cb.buf);\n-\treturn retval;\n+\treturn consumed;\n }\n \n int get_sha1_mb(const char *name, unsigned char *sha1)\n"},{"id":"257554","messageId":"55003757.8090004@drmicha.warpmail.net","threadId":"38776","inReplyTo":"xmqqy4n4zjst.fsf@gitster.dls.corp.google.com","subject":"Re: [JFF] \"-\" and \"@{-1}\" on various programs","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2015-03-11T12:38:47Z","receivedAt":"2015-03-11T12:38:47Z","isPatch":false,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Junio C Hamano venit, vidit, dixit 10.03.2015 21:20:\n> JFF stands for just for fun.\n> \n> This is not meant to give out a model answer and is known to be\n> incomplete, but I was wondering if it would be a better direction to\n> allow \"-\" as a stand-in for \"@{-1}\" everywhere we allow a branch\n> name, losing workarounds at the surface level we have for checkout,\n> merge and revert.\n> \n> The first three paths are to remove the surface workarounds that\n> become unnecessary.  The one in sha1_name.c is the central change.\n> \n> The change in revision.c is to allow a single \"-\" to be recognized\n> as a potential revision name (without this change, what begins with\n> \"-\" is either an option or an unknown option).\n> \n> So you could do things like \"git reset - $path\" but also things like\n> \"git log -\" after switching out of a branch.\n> \n> What does not work are what needs further tweaking in revision.c\n> parser.  \"git checkout master && git checkout next && git log -..\"\n> should show what next has on top of master but I didn't touch the\n> range notation so it does not work, for example.\n> \n>  builtin/checkout.c |  3 ---\n>  builtin/merge.c    |  3 +--\n>  builtin/revert.c   |  2 --\n>  revision.c         |  2 +-\n>  sha1_name.c        | 57 +++++++++++++++++++++++++++++++++---------------------\n>  5 files changed, 37 insertions(+), 30 deletions(-)\n\nLike it :)\n\nIt removes the special casing and makes a shorthand available\nsystematically. I'd say it's useful even without extending it to ranges.\n\nMichae\n"},{"id":"257565","messageId":"xmqqoanz5lxw.fsf@gitster.dls.corp.google.com","threadId":"38776","inReplyTo":"55003757.8090004@drmicha.warpmail.net","subject":"Re: [JFF] \"-\" and \"@{-1}\" on various programs","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-11T20:16:43Z","receivedAt":"2015-03-11T20:16:43Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael J Gruber <git@drmicha.warpmail.net> writes:\n\n> Junio C Hamano venit, vidit, dixit 10.03.2015 21:20:\n>> JFF stands for just for fun.\n>> ...\n>> What does not work are what needs further tweaking in revision.c\n>> parser.  \"git checkout master && git checkout next && git log -..\"\n>> should show what next has on top of master but I didn't touch the\n>> range notation so it does not work, for example.\n>> \n>>  builtin/checkout.c |  3 ---\n>>  builtin/merge.c    |  3 +--\n>>  builtin/revert.c   |  2 --\n>>  revision.c         |  2 +-\n>>  sha1_name.c        | 57 +++++++++++++++++++++++++++++++++---------------------\n>>  5 files changed, 37 insertions(+), 30 deletions(-)\n>\n> Like it :)\n>\n> It removes the special casing and makes a shorthand available\n> systematically. I'd say it's useful even without extending it to ranges.\n\nI wouldn't have done this if I thought it will lead to an unusable\nblind alley ;-) But I won't be hacking on this any further for now.\n\nHint hint...\n"}]}