{"thread":{"id":"38946","subject":"[PATCH/RFC 1/4] Add \"-\" as @{-1} support for the rev-parse command","startedAt":"2015-03-30T17:41:51Z","lastAt":"2015-03-31T04:55:54Z","messageCount":7,"participants":["Kenny Lee Sin Cheong","Junio C Hamano","Torsten Bögershausen"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"258681","messageId":"1427737315-7229-1-git-send-email-kenny.lee28@gmail.com","threadId":"38946","inReplyTo":null,"subject":"[PATCH/RFC 0/4] Adding '-' notation as @{-1} (pu, d40f108)","fromName":"Kenny Lee Sin Cheong","fromEmail":"kenny.lee28@gmail.com","sentAt":"2015-03-30T17:41:51Z","receivedAt":"2015-03-30T17:41:51Z","isPatch":true,"sender":{"key":"kenny.lee28@gmail.com","avatar":null},"body":"This is an attempt to allow '-' everywhere a revision is normally allowed.\nI previously attempted this  as a microproject and the subject was disscussed at : http://article.gmane.org/gmane.comp.version-control.git/265672\n\nCurrently, something like '-~2' does not work. I tried tracing the execution of, say 'log -~2' vs 'log master -~2' and noticed when calling dwim_ref() with '-~2', it returns 0 (no refs found) whereas when given 'master~2', it returned non-zero. However I'm not sure how exactly dwim_ref() works.\n\nKenny Lee Sin Cheong (4):\n  Add \"-\" as @{-1} support for the rev-parse command\n  t1505: add tests for '-' notation in rev-parse\n  Handle arg as revision first, then option.\n  t0102: add tests for '-' notation\n\n builtin/rev-parse.c           | 37 +++++++++++++-------------\n revision.c                    | 61 +++++++++++++++++++++++--------------------\n sha1_name.c                   |  2 +-\n t/t0102-previous-shorthand.sh | 40 ++++++++++++++++++++++++++++\n t/t1505-rev-parse-last.sh     | 12 ++++++---\n 5 files changed, 101 insertions(+), 51 deletions(-)\n create mode 100644 t/t0102-previous-shorthand.sh\n\n-- \n2.3.3.203.g8ffb468.dirty\n"},{"id":"258680","messageId":"1427737315-7229-2-git-send-email-kenny.lee28@gmail.com","threadId":"38946","inReplyTo":"1427737315-7229-1-git-send-email-kenny.lee28@gmail.com","subject":"[PATCH/RFC 1/4] Add \"-\" as @{-1} support for the rev-parse command","fromName":"Kenny Lee Sin Cheong","fromEmail":"kenny.lee28@gmail.com","sentAt":"2015-03-30T17:41:52Z","receivedAt":"2015-03-30T17:41:52Z","isPatch":true,"sender":{"key":"kenny.lee28@gmail.com","avatar":null},"body":"Allows the use of the \"-\" shorthand notation, including\nuse with revision ranges. If we plan to allow \"-\" as a stand in every\nwhere a revision is allowed, then \"-\" would also need to be usable in\nplumbing commands, for writing tests, for example.\n\nChecks if the argument can be interpreted as a revision range first\nbefore checking for flags. This saves us from having to check that\nsomething that begins with \"-\" does not get checked as a possible flag.\n\nSigned-off-by: Kenny Lee Sin Cheong <kenny.lee28@gmail.com>\n---\n builtin/rev-parse.c | 37 +++++++++++++++++++------------------\n 1 file changed, 19 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 3626c61..8da95b5 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -553,6 +553,25 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \n+\t\t/* Not a flag argument */\n+\t\tif (try_difference(arg))\n+\t\t\tcontinue;\n+\t\tif (try_parent_shorthands(arg))\n+\t\t\tcontinue;\n+\t\tname = arg;\n+\t\ttype = NORMAL;\n+\t\tif (*arg == '^') {\n+\t\t\tname++;\n+\t\t\ttype = REVERSED;\n+\t\t}\n+\t\tif (!get_sha1_with_context(name, flags, sha1, &unused)) {\n+\t\t\tif (verify)\n+\t\t\t\trevs_count++;\n+\t\t\telse\n+\t\t\t\tshow_rev(type, sha1, name);\n+\t\t\tcontinue;\n+\t\t}\n+\n \t\tif (*arg == '-') {\n \t\t\tif (!strcmp(arg, \"--\")) {\n \t\t\t\tas_is = 2;\n@@ -810,24 +829,6 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\tcontinue;\n \t\t}\n \n-\t\t/* Not a flag argument */\n-\t\tif (try_difference(arg))\n-\t\t\tcontinue;\n-\t\tif (try_parent_shorthands(arg))\n-\t\t\tcontinue;\n-\t\tname = arg;\n-\t\ttype = NORMAL;\n-\t\tif (*arg == '^') {\n-\t\t\tname++;\n-\t\t\ttype = REVERSED;\n-\t\t}\n-\t\tif (!get_sha1_with_context(name, flags, sha1, &unused)) {\n-\t\t\tif (verify)\n-\t\t\t\trevs_count++;\n-\t\t\telse\n-\t\t\t\tshow_rev(type, sha1, name);\n-\t\t\tcontinue;\n-\t\t}\n \t\tif (verify)\n \t\t\tdie_no_single_rev(quiet);\n \t\tif (has_dashdash)\n-- \n2.3.3.203.g8ffb468.dirty\n"},{"id":"258682","messageId":"1427737315-7229-3-git-send-email-kenny.lee28@gmail.com","threadId":"38946","inReplyTo":"1427737315-7229-1-git-send-email-kenny.lee28@gmail.com","subject":"[PATCH/RFC 2/4] t1505: add tests for '-' notation in rev-parse","fromName":"Kenny Lee Sin Cheong","fromEmail":"kenny.lee28@gmail.com","sentAt":"2015-03-30T17:41:53Z","receivedAt":"2015-03-30T17:41:53Z","isPatch":true,"sender":{"key":"kenny.lee28@gmail.com","avatar":null},"body":"Signed-off-by: Kenny Lee Sin Cheong <kenny.lee28@gmail.com>\n---\n t/t1505-rev-parse-last.sh | 12 ++++++++----\n 1 file changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh\nindex 4969edb..a1976ad 100755\n--- a/t/t1505-rev-parse-last.sh\n+++ b/t/t1505-rev-parse-last.sh\n@@ -33,19 +33,23 @@ test_expect_success 'setup' '\n # and 'side' should be the last branch\n \n test_expect_success '@{-1} works' '\n-\ttest_cmp_rev side @{-1}\n+\ttest_cmp_rev side @{-1} &&\n+\ttest_cmp_rev side -\n '\n \n test_expect_success '@{-1}~2 works' '\n-\ttest_cmp_rev side~2 @{-1}~2\n+\ttest_cmp_rev side~2 @{-1}~2 &&\n+\ttest_cmp_rev side~2 -~2\n '\n \n test_expect_success '@{-1}^2 works' '\n-\ttest_cmp_rev side^2 @{-1}^2\n+\ttest_cmp_rev side^2 @{-1}^2 &&\n+\ttest_cmp_rev side^2 -^2\n '\n \n test_expect_success '@{-1}@{1} works' '\n-\ttest_cmp_rev side@{1} @{-1}@{1}\n+\ttest_cmp_rev side@{1} @{-1}@{1} &&\n+\ttest_cmp_rev side@{1} -@{1}\n '\n \n test_expect_success '@{-2} works' '\n-- \n2.3.3.203.g8ffb468.dirty\n"},{"id":"258684","messageId":"1427737315-7229-4-git-send-email-kenny.lee28@gmail.com","threadId":"38946","inReplyTo":"1427737315-7229-1-git-send-email-kenny.lee28@gmail.com","subject":"[PATCH/RFC 3/4] Handle arg as revision first, then option.","fromName":"Kenny Lee Sin Cheong","fromEmail":"kenny.lee28@gmail.com","sentAt":"2015-03-30T17:41:54Z","receivedAt":"2015-03-30T17:41:54Z","isPatch":true,"sender":{"key":"kenny.lee28@gmail.com","avatar":null},"body":"Check the argument as a revision at first. If it fails, then tries to\ncheck it as an option, and finally as a pathspec.\n\nReturns -1 when we have an ambiguous revision range, such as\n\"master..next\", to allow the argument to get checked as an option before\ncalling die() from verify_non_filename(). This is because we are\nallowing \"-\" to be given in a revision range, but making the revision\ncheck first. Otherwise, an ambiguous argument that starts with\n\"-\" (let's say an option) would die even though its normal behaviour is\nto silently return. Instead we check for ambiguity in a revision after\nmaking sure that the argument cannot be parsed as an option.\n\nThis problem is discussed in:\nhttp://article.gmane.org/gmane.comp.version-control.git/265672\n\nSigned-off-by: Kenny Lee Sin Cheong <kenny.lee28@gmail.com>\n---\n revision.c  | 61 +++++++++++++++++++++++++++++++++----------------------------\n sha1_name.c |  2 +-\n 2 files changed, 34 insertions(+), 29 deletions(-)\n\ndiff --git a/revision.c b/revision.c\nindex 570945a..1ea290f 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1516,7 +1516,10 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi\n \n \t\t\tif (!cant_be_filename) {\n \t\t\t\t*dotdot = '.';\n-\t\t\t\tverify_non_filename(revs->prefix, arg);\n+\t\t\t\tif (is_inside_work_tree() && !is_inside_git_dir() &&\n+\t\t\t\t    check_filename(revs->prefix, arg)) {\n+\t\t\t\t\treturn -1;\n+\t\t\t\t}\n \t\t\t}\n \n \t\t\ta_obj = parse_object(from_sha1);\n@@ -2198,40 +2201,39 @@ 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[0] == '-' && arg[1] && !starts_with(arg + 1, \"..\")) {\n-\t\t\tint opts;\n-\n-\t\t\topts = handle_revision_pseudo_opt(submodule,\n-\t\t\t\t\t\trevs, argc - i, argv + i,\n-\t\t\t\t\t\t&flags);\n-\t\t\tif (opts > 0) {\n-\t\t\t\ti += opts - 1;\n-\t\t\t\tcontinue;\n-\t\t\t}\n+\t\tif (handle_revision_arg(arg, revs, flags, revarg_opt)) {\n+\t\t\tif (arg[0] == '-' && arg[1] && !starts_with(arg + 1, \"..\")) {\n+\t\t\t\tint opts;\n+\n+\t\t\t\topts = handle_revision_pseudo_opt(submodule,\n+\t\t\t\t\t\t\t\t  revs, argc - i, argv + i,\n+\t\t\t\t\t\t\t\t  &flags);\n+\t\t\t\tif (opts > 0) {\n+\t\t\t\t\ti += opts - 1;\n+\t\t\t\t\tcontinue;\n+\t\t\t\t}\n \n-\t\t\tif (!strcmp(arg, \"--stdin\")) {\n-\t\t\t\tif (revs->disable_stdin) {\n-\t\t\t\t\targv[left++] = arg;\n+\t\t\t\tif (!strcmp(arg, \"--stdin\")) {\n+\t\t\t\t\tif (revs->disable_stdin) {\n+\t\t\t\t\t\targv[left++] = arg;\n+\t\t\t\t\t\tcontinue;\n+\t\t\t\t\t}\n+\t\t\t\t\tif (read_from_stdin++)\n+\t\t\t\t\t\tdie(\"--stdin given twice?\");\n+\t\t\t\t\tread_revisions_from_stdin(revs, &prune_data);\n \t\t\t\t\tcontinue;\n \t\t\t\t}\n-\t\t\t\tif (read_from_stdin++)\n-\t\t\t\t\tdie(\"--stdin given twice?\");\n-\t\t\t\tread_revisions_from_stdin(revs, &prune_data);\n-\t\t\t\tcontinue;\n-\t\t\t}\n \n-\t\t\topts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);\n-\t\t\tif (opts > 0) {\n-\t\t\t\ti += opts - 1;\n+\t\t\t\topts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);\n+\t\t\t\tif (opts > 0) {\n+\t\t\t\t\ti += opts - 1;\n+\t\t\t\t\tcontinue;\n+\t\t\t\t}\n+\t\t\t\tif (opts < 0)\n+\t\t\t\t\texit(128);\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t\tif (opts < 0)\n-\t\t\t\texit(128);\n-\t\t\tcontinue;\n-\t\t}\n-\n \n-\t\tif (handle_revision_arg(arg, revs, flags, revarg_opt)) {\n \t\t\tint j;\n \t\t\tif (seen_dashdash || *arg == '^')\n \t\t\t\tdie(\"bad revision '%s'\", arg);\n@@ -2249,6 +2251,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s\n \t\t\tbreak;\n \t\t}\n \t\telse\n+\t\t\t/* Make sure that a filename doesn't get interpreted as a revision */\n+\t\t\tif (!seen_dashdash)\n+\t\t\t\tverify_non_filename(revs->prefix, arg);\n \t\t\tgot_rev_arg = 1;\n \t}\n \ndiff --git a/sha1_name.c b/sha1_name.c\nindex 7a621ba..b99b1dc 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -483,7 +483,7 @@ 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} else if (len == 1 && str[0] == '-' && !str[1]) {\n \t\tnth_prior = 1;\n \t}\n \n-- \n2.3.3.203.g8ffb468.dirty\n"},{"id":"258685","messageId":"1427737315-7229-5-git-send-email-kenny.lee28@gmail.com","threadId":"38946","inReplyTo":"1427737315-7229-1-git-send-email-kenny.lee28@gmail.com","subject":"[PATCH/RFC 4/4] t0102: add tests for '-' notation","fromName":"Kenny Lee Sin Cheong","fromEmail":"kenny.lee28@gmail.com","sentAt":"2015-03-30T17:41:55Z","receivedAt":"2015-03-30T17:41:55Z","isPatch":true,"sender":{"key":"kenny.lee28@gmail.com","avatar":null},"body":"Signed-off-by: Kenny Lee Sin Cheong <kenny.lee28@gmail.com>\n---\n t/t0102-previous-shorthand.sh | 40 ++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 40 insertions(+)\n create mode 100644 t/t0102-previous-shorthand.sh\n\ndiff --git a/t/t0102-previous-shorthand.sh b/t/t0102-previous-shorthand.sh\nnew file mode 100644\nindex 0000000..919b055\n--- /dev/null\n+++ b/t/t0102-previous-shorthand.sh\n@@ -0,0 +1,40 @@\n+#!/bin/sh\n+\n+test_description='previous branch syntax @{-n}'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'branch -d -' '\n+\ttest_commit A &&\n+\tgit checkout -b junk2 &&\n+\tgit checkout - &&\n+\ttest \"$(git symbolic-ref HEAD)\" = refs/heads/master &&\n+\tgit branch -d - &&\n+\ttest_must_fail git rev-parse --verify refs/heads/junk2\n+'\n+\n+test_expect_success 'merge -' '\n+\tgit checkout A &&\n+\ttest_commit B &&\n+\tgit checkout A &&\n+\ttest_commit C &&\n+\ttest_commit D &&\n+\tgit branch -f master B &&\n+\tgit branch -f other &&\n+\tgit checkout other &&\n+\tgit checkout master &&\n+\tgit merge - &&\n+\tgit cat-file commit HEAD | grep \"Merge branch '\\''other'\\''\"\n+'\n+\n+test_expect_success 'merge -~1' '\n+\tgit checkout master &&\n+\tgit reset --hard B &&\n+\tgit checkout other &&\n+\tgit checkout master &&\n+\tgit merge -~1 &&\n+\tgit cat-file commit HEAD >actual &&\n+\tgrep \"Merge branch '\\''other'\\''\" actual\n+'\n+\n+test_done\n-- \n2.3.3.203.g8ffb468.dirty\n"},{"id":"258688","messageId":"xmqqy4mew9n3.fsf@gitster.dls.corp.google.com","threadId":"38946","inReplyTo":"1427737315-7229-2-git-send-email-kenny.lee28@gmail.com","subject":"Re: [PATCH/RFC 1/4] Add \"-\" as @{-1} support for the rev-parse command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-30T19:46:56Z","receivedAt":"2015-03-30T19:46:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kenny Lee Sin Cheong <kenny.lee28@gmail.com> writes:\n\n> Allows the use of the \"-\" shorthand notation, including\n> use with revision ranges. If we plan to allow \"-\" as a stand in every\n> where a revision is allowed, then \"-\" would also need to be usable in\n> plumbing commands, for writing tests, for example.\n>\n> Checks if the argument can be interpreted as a revision range first\n> before checking for flags. This saves us from having to check that\n> something that begins with \"-\" does not get checked as a possible flag.\n\nDoesn't that mean -<something> that is a valid flag can no longer be\nrecognised as a flag if the same string can be an extended SHA-1\nwhose formulation starts from \"the previous branch\"?  It sounds like\na regression to me.\n\nHmmm.\n\nAfter all, \"we often call for the previous branch, so let's give a\nshort-and-sweet '-' as an even shorter short-hand than '@{-1}'\" and\n\"allow '-' anywhere\" are two quite different things.  We may do \"git\ncheckout -\" very often to go back to what we were working on, but I\ndo not think \"git log -..\" or \"git log ..-\" are something we want to\ndo very often.\n\nI think what I am saying is that it may be perfectly fine if we said\n\"'-' can be used for '@{-1}' only by itself; no ranges, no\nparent-traversals, no other uses\", if it makes it less likely for\nmistakes and confusions to happen.\n"},{"id":"258730","messageId":"551A28DA.2050402@web.de","threadId":"38946","inReplyTo":"1427737315-7229-3-git-send-email-kenny.lee28@gmail.com","subject":"Re: [PATCH/RFC 2/4] t1505: add tests for '-' notation in rev-parse","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-03-31T04:55:54Z","receivedAt":"2015-03-31T04:55:54Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 03/30/2015 07:41 PM, Kenny Lee Sin Cheong wrote:\n> Signed-off-by: Kenny Lee Sin Cheong <kenny.lee28@gmail.com>\n> ---\n>   t/t1505-rev-parse-last.sh | 12 ++++++++----\n>   1 file changed, 8 insertions(+), 4 deletions(-)\n>\n> diff --git a/t/t1505-rev-parse-last.sh b/t/t1505-rev-parse-last.sh\n> index 4969edb..a1976ad 100755\n> --- a/t/t1505-rev-parse-last.sh\n> +++ b/t/t1505-rev-parse-last.sh\n> @@ -33,19 +33,23 @@ test_expect_success 'setup' '\n>   # and 'side' should be the last branch\n>   \n>   test_expect_success '@{-1} works' '\n> -\ttest_cmp_rev side @{-1}\n> +\ttest_cmp_rev side @{-1} &&\n> +\ttest_cmp_rev side -\n>   '\n(Beside that \"-\" is often used for \"stdin\" in many unix-like tools,\nand my favorite would be \"-1\" ):\n\nI think the test heading should be updated as well:\n\ntest_expect_success '@{-1} or - works' '\n\ttest_cmp_rev side @{-1} &&\n\ttest_cmp_rev side -\n  '\n\n\n>   \n>   test_expect_success '@{-1}~2 works' '\n> -\ttest_cmp_rev side~2 @{-1}~2\n> +\ttest_cmp_rev side~2 @{-1}~2 &&\n> +\ttest_cmp_rev side~2 -~2\n>   '\n>   \n>   test_expect_success '@{-1}^2 works' '\n> -\ttest_cmp_rev side^2 @{-1}^2\n> +\ttest_cmp_rev side^2 @{-1}^2 &&\n> +\ttest_cmp_rev side^2 -^2\n>   '\n>   \n>   test_expect_success '@{-1}@{1} works' '\n> -\ttest_cmp_rev side@{1} @{-1}@{1}\n> +\ttest_cmp_rev side@{1} @{-1}@{1} &&\n> +\ttest_cmp_rev side@{1} -@{1}\n>   '\n>   \n>   test_expect_success '@{-2} works' '\n"}]}