{"thread":{"id":"38767","subject":"[PATCH 1/2] Adding - shorthand for @{-1} in RESET command","startedAt":"2015-03-09T20:46:49Z","lastAt":"2015-03-10T17:43:57Z","messageCount":7,"participants":["Sundararajan R","Torsten Bögershausen","Eric Sunshine","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"257428","messageId":"1425934010-8780-1-git-send-email-dyoucme@gmail.com","threadId":"38767","inReplyTo":null,"subject":"[PATCH 1/2] Adding - shorthand for @{-1} in RESET command","fromName":"Sundararajan R","fromEmail":"dyoucme@gmail.com","sentAt":"2015-03-09T20:46:49Z","receivedAt":"2015-03-09T20:46:49Z","isPatch":true,"sender":{"key":"dyoucme@gmail.com","avatar":null},"body":"Please give feedback and suggest things I may have missed out on. \nI hope I have incorporated all the suggestions.\n\nSigned-off-by: Sundararajan R <dyoucme@gmail.com>\nThanks-to: Junio C Hamano\n---\nI have attempted to resolve the ambiguity when there exists a file named -\nby communicating to the user that he/she can use ./- when he/she wants to refer\nto the - file. I perform this check using the check_filename() function.\n\n builtin/reset.c | 16 +++++++++++++++-\n 1 file changed, 15 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 4c08ddc..2bdd5cd 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -192,6 +192,7 @@ static void parse_args(struct pathspec *pathspec,\n {\n \tconst char *rev = \"HEAD\";\n \tunsigned char unused[20];\n+\tint file_named_minus=0;\n \t/*\n \t * Possible arguments are:\n \t *\n@@ -205,6 +206,12 @@ static void parse_args(struct pathspec *pathspec,\n \t */\n \n \tif (argv[0]) {\n+\t\tif (!strcmp(argv[0], \"-\") && !argv[1]) {\n+\t\t\tif(!check_filename(prefix,\"-\"))\n+\t\t\t\targv[0]=\"@{-1}\";\n+\t\t\telse \n+\t\t\t\tfile_named_minus=1;\n+\t\t}\n \t\tif (!strcmp(argv[0], \"--\")) {\n \t\t\targv++; /* reset to HEAD, possibly with paths */\n \t\t} else if (argv[1] && !strcmp(argv[1], \"--\")) {\n@@ -226,7 +233,14 @@ static void parse_args(struct pathspec *pathspec,\n \t\t\trev = *argv++;\n \t\t} else {\n \t\t\t/* Otherwise we treat this as a filename */\n-\t\t\tverify_filename(prefix, argv[0], 1);\n+\t\t\tif(file_named_minus) {\n+\t\t\t\tdie(_(\"ambiguous argument '-': both revision and filename\\n\"\n+\t\t\t\t\t\"Use ./- for file named -\\n\"\n+\t\t\t\t\t\"Use '--' to separate paths from revisions, like this:\\n\"\n+\t\t\t\t\t\"'git <command> [<revision>...] -- [<file>...]'\"));\n+\t\t\t}\n+\t\t\telse\n+\t\t\t\tverify_filename(prefix, argv[0], 1);\n \t\t}\n \t}\n \t*rev_ret = rev;\n-- \n2.1.0\n"},{"id":"257429","messageId":"1425934010-8780-2-git-send-email-dyoucme@gmail.com","threadId":"38767","inReplyTo":"1425934010-8780-1-git-send-email-dyoucme@gmail.com","subject":"[PATCH 2/2] Added tests for git reset -","fromName":"Sundararajan R","fromEmail":"dyoucme@gmail.com","sentAt":"2015-03-09T20:46:50Z","receivedAt":"2015-03-09T20:46:50Z","isPatch":true,"sender":{"key":"dyoucme@gmail.com","avatar":null},"body":"As you had suggested @Junio, I have added the required tests.\nPlease let me know if there is something is I should add.\n\nSigned-off-by: Sundararajan R <dyoucme@gmail.com>\nThanks-to: Junio C Hamano\n---\nI have added 6 tests to check for the following cases:\ngit reset - with no @{-1}\ngit reset - with no @{-1} and file named -\ngit reset - with @{-1} and file named @{-1}\ngit reset - with @{-1} and file named - \ngit reset - with @{-1} and file named @{-1} and - \ngit reset - with @{-1} and no file named - or @{-1} \nThe 1st test with no previous branch results in the error\nThe 2nd,3rd,4th and 5th result in the ambiguous argument error \nThe 6th test has - working like @{-1}\n\n t/t7102-reset.sh | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 107 insertions(+)\n\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex 98bcfe2..a670938 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -568,4 +568,111 @@ test_expect_success 'reset --mixed sets up work tree' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'reset - with no @{-1}' '\n+\tgit init new --quiet &&\n+\tcd new &&\n+\ttest_must_fail git reset - >actual &&\n+\ttouch expect &&\n+\ttest_cmp expect actual\n+'\n+\n+rm -rf new\n+\n+cat >expect <<EOF\n+fatal: ambiguous argument '-': both revision and filename\n+Use ./- for file named -\n+Use '--' to separate paths from revisions, like this:\n+'git <command> [<revision>...] -- [<file>...]'\n+EOF\n+\n+test_expect_success 'reset - with no @{-1} and file named -' '\n+\tgit init new --quiet &&\n+\tcd new &&\n+\techo \"Hello\" > - &&\n+\tgit add -\n+\ttest_must_fail git reset - 2>actual &&\n+\ttest_cmp ../expect actual\n+'\n+\n+cd ..\n+rm -rf new\n+\n+cat >expect <<EOF\n+fatal: ambiguous argument '@{-1}': both revision and filename\n+Use '--' to separate paths from revisions, like this:\n+'git <command> [<revision>...] -- [<file>...]'\n+EOF\n+\n+test_expect_success 'reset - with @{-1} and file named @{-1}' '\n+\tgit init new --quiet &&\n+\tcd new && \n+\techo \"Hello\" >@{-1} &&\n+\tgit add @{-1} &&\n+\tgit commit -m \"first_commit\" &&\n+\tgit checkout -b new_branch &&\n+\ttouch @{-1} &&\n+\tgit add @{-1} &&\n+\ttest_must_fail git reset - 2>actual &&\n+\ttest_cmp ../expect actual\n+'\n+\n+cd ..\n+rm -rf new\n+\n+cat >expect <<EOF\n+fatal: ambiguous argument '-': both revision and filename\n+Use ./- for file named -\n+Use '--' to separate paths from revisions, like this:\n+'git <command> [<revision>...] -- [<file>...]'\n+EOF\n+\n+test_expect_success 'reset - with @{-1} and file named - ' '\n+\tgit init new --quiet &&\n+\tcd new && \n+\techo \"Hello\" > - &&\n+\tgit add - &&\n+\tgit commit -m \"first_commit\" &&\n+\tgit checkout -b new_branch &&\n+\ttouch - &&\n+\tgit add - &&\n+\ttest_must_fail git reset - 2>actual &&\n+\ttest_cmp ../expect actual\n+'\n+\n+cd ..\n+rm -rf new\n+\n+test_expect_success 'reset - with @{-1} and file named @{-1} and - ' '\n+\tgit init new --quiet &&\n+\tcd new &&\n+\techo \"Hello\" > - &&\n+\tgit add - &&\n+\tgit commit -m \"first_commit\" &&\n+\tgit checkout -b new_branch\n+\techo \"Hello\" >@{-1} &&\n+\tgit add @{-1} &&\n+\ttest_must_fail git reset - 2>actual &&\n+\ttest_cmp ../expect actual\n+'\n+\n+cd ..\n+rm -rf new\n+\n+test_expect_success 'reset - with @{-1} and no file named - or @{-1} ' '\n+\tgit init new --quiet &&\n+\tcd new &&\n+\techo \"Hello\" >new_file &&\n+\tgit add new_file &&\n+\tgit commit -m \"first_commit\" &&\n+\tgit checkout -b new_branch &&\n+\techo \"Hey\" >new_file &&\n+\tgit add new_file &&\n+\tgit reset - &&\n+\tgit status -uno >file1 &&\n+\tgit add new_file &&\n+\tgit reset @{-1} &&\n+\tgit status -uno >file2 &&\n+\ttest_cmp file1 file2\n+'\n+\n test_done\n-- \n2.1.0\n"},{"id":"257443","messageId":"54FE8599.7000403@web.de","threadId":"38767","inReplyTo":"1425934010-8780-2-git-send-email-dyoucme@gmail.com","subject":"Re: [PATCH 2/2] Added tests for git reset -","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-03-10T05:48:09Z","receivedAt":"2015-03-10T05:48:09Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 03/09/2015 09:46 PM, Sundararajan R wrote:\n> As you had suggested @Junio, I have added the required tests.\n> Please let me know if there is something is I should add.\n>\n> Signed-off-by: Sundararajan R <dyoucme@gmail.com>\n> Thanks-to: Junio C Hamano\n> ---\n> I have added 6 tests to check for the following cases:\n> git reset - with no @{-1}\n> git reset - with no @{-1} and file named -\n> git reset - with @{-1} and file named @{-1}\n> git reset - with @{-1} and file named -\n> git reset - with @{-1} and file named @{-1} and -\n> git reset - with @{-1} and no file named - or @{-1}\n> The 1st test with no previous branch results in the error\n> The 2nd,3rd,4th and 5th result in the ambiguous argument error\n> The 6th test has - working like @{-1}\n>\n>   t/t7102-reset.sh | 107 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n>   1 file changed, 107 insertions(+)\n>\n> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\n> index 98bcfe2..a670938 100755\n> --- a/t/t7102-reset.sh\n> +++ b/t/t7102-reset.sh\n> @@ -568,4 +568,111 @@ test_expect_success 'reset --mixed sets up work tree' '\n>   \ttest_cmp expect actual\n>   '\n>   \n> +test_expect_success 'reset - with no @{-1}' '\n> +\tgit init new --quiet &&\n> +\tcd new &&\n> +\ttest_must_fail git reset - >actual &&\n> +\ttouch expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +rm -rf new\n> +\n> +cat >expect <<EOF\n> +fatal: ambiguous argument '-': both revision and filename\n> +Use ./- for file named -\n> +Use '--' to separate paths from revisions, like this:\n> +'git <command> [<revision>...] -- [<file>...]'\n> +EOF\n> +\n> +test_expect_success 'reset - with no @{-1} and file named -' '\n> +\tgit init new --quiet &&\n> +\tcd new &&\n> +\techo \"Hello\" > - &&\n> +\tgit add -\n> +\ttest_must_fail git reset - 2>actual &&\n> +\ttest_cmp ../expect actual\n> +'\n> +\n> +cd ..\n> +rm -rf new\n> +\n> +cat >expect <<EOF\n> +fatal: ambiguous argument '@{-1}': both revision and filename\n> +Use '--' to separate paths from revisions, like this:\n> +'git <command> [<revision>...] -- [<file>...]'\n> +EOF\n> +\n> +test_expect_success 'reset - with @{-1} and file named @{-1}' '\n> +\tgit init new --quiet &&\n> +\tcd new &&\nIf the shell changes the directory, this should be done in a subshell\n\n+\tgit init new --quiet &&\n+\t(\n\t\tcd new &&\n\n                # All the stuff\n             )\n\n+'\n\n+cd ..\n\nAnd the the .. should  be removed\n(Same problem further down)\n"},{"id":"257447","messageId":"CAPig+cRAB-LQctj6UOKUXps-MEh2C_EbSp_3=wfgxtWx6xCbhw@mail.gmail.com","threadId":"38767","inReplyTo":"1425934010-8780-1-git-send-email-dyoucme@gmail.com","subject":"Re: [PATCH 1/2] Adding - shorthand for @{-1} in RESET command","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-03-10T06:54:52Z","receivedAt":"2015-03-10T06:54:52Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 9, 2015 at 4:46 PM, Sundararajan R <dyoucme@gmail.com> wrote:\n> Please give feedback and suggest things I may have missed out on.\n> I hope I have incorporated all the suggestions.\n\nIf you haven't already, read Documentation/SubmittingPatches. Pay\nparticular attention to section #2 which explains how to write a good\ncommit message, and to section #4 to learn where to place patch\ncommentary not intended as part of the permanent commit history. The\nabove lines are commentary, not meant as part of the commit message.\nPlace such commentary below the '---' line just before the diffstat.\n\n> Subject: Adding - shorthand for @{-1} in RESET command\n\nPrefix the first line of the commit message with the module or command\nyou re changing. Drop capitalization. Write in imperative mood. For\ninstance:\n\n    reset: add '-' shorthand for '@{-1}'\n\n> Signed-off-by: Sundararajan R <dyoucme@gmail.com>\n> Thanks-to: Junio C Hamano\n\nPlace your sign-off last. Use Helped-by: rather than Thanks-to: and\ninclude the person's full name and email address.\n\n> ---\n\nHere, just below the '---' line is where you should place commentary\nnot intended for the permanent commit record.\n\n> I have attempted to resolve the ambiguity when there exists a file named -\n> by communicating to the user that he/she can use ./- when he/she wants to refer\n> to the - file. I perform this check using the check_filename() function.\n\nThis is important information for the commit message itself above the\n'---' line, though you would want to rephrase it just to state the\nfacts. No need to mention \"I did this\" or \"I did that\" since the patch\nitself implies that you made the changes.\n\n>  builtin/reset.c | 16 +++++++++++++++-\n>  1 file changed, 15 insertions(+), 1 deletion(-)\n>\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 4c08ddc..2bdd5cd 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -192,6 +192,7 @@ static void parse_args(struct pathspec *pathspec,\n>  {\n>         const char *rev = \"HEAD\";\n>         unsigned char unused[20];\n> +       int file_named_minus=0;\n\nStyle: Here and elsewhere, add a space around '='.\n\n>         /*\n>          * Possible arguments are:\n>          *\n> @@ -205,6 +206,12 @@ static void parse_args(struct pathspec *pathspec,\n>          */\n>\n>         if (argv[0]) {\n> +               if (!strcmp(argv[0], \"-\") && !argv[1]) {\n> +                       if(!check_filename(prefix,\"-\"))\n\nStyle: Here and elsewhere, add space after 'if'.\nStyle: Add space after comma.\n\n> +                               argv[0]=\"@{-1}\";\n> +                       else\n> +                               file_named_minus=1;\n> +               }\n>                 if (!strcmp(argv[0], \"--\")) {\n>                         argv++; /* reset to HEAD, possibly with paths */\n>                 } else if (argv[1] && !strcmp(argv[1], \"--\")) {\n> @@ -226,7 +233,14 @@ static void parse_args(struct pathspec *pathspec,\n>                         rev = *argv++;\n>                 } else {\n>                         /* Otherwise we treat this as a filename */\n> -                       verify_filename(prefix, argv[0], 1);\n> +                       if(file_named_minus) {\n> +                               die(_(\"ambiguous argument '-': both revision and filename\\n\"\n> +                                       \"Use ./- for file named -\\n\"\n> +                                       \"Use '--' to separate paths from revisions, like this:\\n\"\n> +                                       \"'git <command> [<revision>...] -- [<file>...]'\"));\n\nThis seems odd. If arguments following '--' are unconditionally\ntreated as paths, why is it be necessary to tell the user to spell out\nfile '-' as './-'? Shouldn't \"git reset -- -\" be sufficient?\n\n> +                       }\n> +                       else\n> +                               verify_filename(prefix, argv[0], 1);\n>                 }\n>         }\n>         *rev_ret = rev;\n> --\n> 2.1.0\n"},{"id":"257450","messageId":"CAPig+cRgA5ZMM8d+ep8cyoZpK8FubzDBNVACgGWyWdRgF+nq7w@mail.gmail.com","threadId":"38767","inReplyTo":"1425934010-8780-2-git-send-email-dyoucme@gmail.com","subject":"Re: [PATCH 2/2] Added tests for git reset -","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-03-10T07:49:06Z","receivedAt":"2015-03-10T07:49:06Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 9, 2015 at 4:46 PM, Sundararajan R <dyoucme@gmail.com> wrote:\n> As you had suggested @Junio, I have added the required tests.\n> Please let me know if there is something is I should add.\n>\n> Signed-off-by: Sundararajan R <dyoucme@gmail.com>\n> Thanks-to: Junio C Hamano\n> ---\n> I have added 6 tests to check for the following cases:\n> git reset - with no @{-1}\n> git reset - with no @{-1} and file named -\n> git reset - with @{-1} and file named @{-1}\n> git reset - with @{-1} and file named -\n> git reset - with @{-1} and file named @{-1} and -\n> git reset - with @{-1} and no file named - or @{-1}\n> The 1st test with no previous branch results in the error\n> The 2nd,3rd,4th and 5th result in the ambiguous argument error\n> The 6th test has - working like @{-1}\n\nSee my review of patch 1 regarding where to place commentary, how to\nconstruct a good commit message, and how to formulate the trailers\n(Helped-by:, Signed-off-by:).\n\n> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\n> index 98bcfe2..a670938 100755\n> --- a/t/t7102-reset.sh\n> +++ b/t/t7102-reset.sh\n> @@ -568,4 +568,111 @@ test_expect_success 'reset --mixed sets up work tree' '\n>         test_cmp expect actual\n>  '\n>\n> +test_expect_success 'reset - with no @{-1}' '\n> +       git init new --quiet &&\n\nWhy --quiet?\n\n> +       cd new &&\n\nAs Torsten mentioned already, wrap a subshell (via '(' and ')') around\nthe 'cd' and commands which should be invoked within the subdirectory.\nDoing so ensures that tests following this one aren't incorrectly run\nwithin the working directory set by this test.\n\n> +       test_must_fail git reset - >actual &&\n> +       touch expect &&\n\nUnless the timestamp of 'expect' is significant, don't use \"touch\" to\ncreate the file. Instead, just say \">expect &&\" on a line by itself.\n\n> +       test_cmp expect actual\n\nWhat is the intention here? Rather than git-reset's stderr, you've\ncaptured stdout, which doesn't seem particularly interesting.\n\n> +'\n> +\n> +rm -rf new\n\nThis is ineffective since the test just above left you inside the\n'new' subdirectory, and there is no 'new' subdirectory within that one\nto clean up.\n\n> +cat >expect <<EOF\n> +fatal: ambiguous argument '-': both revision and filename\n> +Use ./- for file named -\n> +Use '--' to separate paths from revisions, like this:\n> +'git <command> [<revision>...] -- [<file>...]'\n> +EOF\n\nReproducing the error message verbatim is fragile. Any minor change to\nthe text will cause the test to fail. Generally speaking, it is\nsufficient just to use test_must_fail to ensure that the command fails\nas expected. If you feel particularly strongly about checking that the\ncorrect error condition occurred, then just check the error output\nusing test_i18ngrep() for a significant phrase, such as \"ambiguous\nargument\".\n\n> +test_expect_success 'reset - with no @{-1} and file named -' '\n> +       git init new --quiet &&\n> +       cd new &&\n> +       echo \"Hello\" > - &&\n\nDrop the space after the redirection operator.\n\n> +       git add -\n\nBroken &&-chain.\n\n> +       test_must_fail git reset - 2>actual &&\n> +       test_cmp ../expect actual\n> +'\n> +\n> +cd ..\n\nThis is problematic. If the preceding test fails in git-init, before\nthe \"cd new\", then this \"cd ..\" will change to the wrong place. Use of\na subshell, as explained above, avoids such problems.\n\n> +rm -rf new\n> +\n> +cat >expect <<EOF\n> +fatal: ambiguous argument '@{-1}': both revision and filename\n> +Use '--' to separate paths from revisions, like this:\n> +'git <command> [<revision>...] -- [<file>...]'\n> +EOF\n\nRather than doing cleanup (\"rm\") and preparation (\"cat >expect\") at\nthe top-level, place them within the test which requires them, thus\nmaking each test self-contained.\n\nI'll stop reviewing here since the above comments also apply to the\nrest of the tests.\n\n> +test_expect_success 'reset - with @{-1} and file named @{-1}' '\n> +       git init new --quiet &&\n> +       cd new &&\n> +       echo \"Hello\" >@{-1} &&\n> +       git add @{-1} &&\n> +       git commit -m \"first_commit\" &&\n> +       git checkout -b new_branch &&\n> +       touch @{-1} &&\n> +       git add @{-1} &&\n> +       test_must_fail git reset - 2>actual &&\n> +       test_cmp ../expect actual\n> +'\n> +\n> +cd ..\n> +rm -rf new\n> +\n> +cat >expect <<EOF\n> +fatal: ambiguous argument '-': both revision and filename\n> +Use ./- for file named -\n> +Use '--' to separate paths from revisions, like this:\n> +'git <command> [<revision>...] -- [<file>...]'\n> +EOF\n> +\n> +test_expect_success 'reset - with @{-1} and file named - ' '\n> +       git init new --quiet &&\n> +       cd new &&\n> +       echo \"Hello\" > - &&\n> +       git add - &&\n> +       git commit -m \"first_commit\" &&\n> +       git checkout -b new_branch &&\n> +       touch - &&\n> +       git add - &&\n> +       test_must_fail git reset - 2>actual &&\n> +       test_cmp ../expect actual\n> +'\n> +\n> +cd ..\n> +rm -rf new\n> +\n> +test_expect_success 'reset - with @{-1} and file named @{-1} and - ' '\n> +       git init new --quiet &&\n> +       cd new &&\n> +       echo \"Hello\" > - &&\n> +       git add - &&\n> +       git commit -m \"first_commit\" &&\n> +       git checkout -b new_branch\n> +       echo \"Hello\" >@{-1} &&\n> +       git add @{-1} &&\n> +       test_must_fail git reset - 2>actual &&\n> +       test_cmp ../expect actual\n> +'\n> +\n> +cd ..\n> +rm -rf new\n> +\n> +test_expect_success 'reset - with @{-1} and no file named - or @{-1} ' '\n> +       git init new --quiet &&\n> +       cd new &&\n> +       echo \"Hello\" >new_file &&\n> +       git add new_file &&\n> +       git commit -m \"first_commit\" &&\n> +       git checkout -b new_branch &&\n> +       echo \"Hey\" >new_file &&\n> +       git add new_file &&\n> +       git reset - &&\n> +       git status -uno >file1 &&\n> +       git add new_file &&\n> +       git reset @{-1} &&\n> +       git status -uno >file2 &&\n> +       test_cmp file1 file2\n> +'\n> +\n>  test_done\n> --\n> 2.1.0\n"},{"id":"257482","messageId":"xmqqfv9c21qj.fsf@gitster.dls.corp.google.com","threadId":"38767","inReplyTo":"CAPig+cRgA5ZMM8d+ep8cyoZpK8FubzDBNVACgGWyWdRgF+nq7w@mail.gmail.com","subject":"Re: [PATCH 2/2] Added tests for git reset -","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-10T17:36:52Z","receivedAt":"2015-03-10T17:36:52Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>> +test_expect_success 'reset - with no @{-1}' '\n>> +       git init new --quiet &&\n>\n> Why --quiet?\n\nAlso, to make sure tests serve as good examples, tests should stick\nto \"options first and then arguments\", i.e. \"git init --quiet new\",\nif it passes options.\n"},{"id":"257483","messageId":"xmqqbnk021eq.fsf@gitster.dls.corp.google.com","threadId":"38767","inReplyTo":"CAPig+cRAB-LQctj6UOKUXps-MEh2C_EbSp_3=wfgxtWx6xCbhw@mail.gmail.com","subject":"Re: [PATCH 1/2] Adding - shorthand for @{-1} in RESET command","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-10T17:43:57Z","receivedAt":"2015-03-10T17:43:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>> @@ -226,7 +233,14 @@ static void parse_args(struct pathspec *pathspec,\n>>                         rev = *argv++;\n>>                 } else {\n>>                         /* Otherwise we treat this as a filename */\n>> -                       verify_filename(prefix, argv[0], 1);\n>> +                       if(file_named_minus) {\n>> +                               die(_(\"ambiguous argument '-': both revision and filename\\n\"\n>> +                                       \"Use ./- for file named -\\n\"\n>> +                                       \"Use '--' to separate paths from revisions, like this:\\n\"\n>> +                                       \"'git <command> [<revision>...] -- [<file>...]'\"));\n>\n> This seems odd. If arguments following '--' are unconditionally\n> treated as paths, why is it be necessary to tell the user to spell out\n> file '-' as './-'? Shouldn't \"git reset -- -\" be sufficient?\n\nI find that the presense of the if statement itself even odder.\n\n - verify_filename() and verify_non_filename() are designed to check\n   that the string \"-\" given by the end-user is or is not a filename\n   on the filesystem.  Why isn't this caller letting the callee do\n   the job it was designed to do and doing that itself instead?\n\n - we know \"-\" aka \"@{-1}\" does not resolve to a committish at this\n   point, so it must be a filename.  If \"-\" exists, then why should\n   the user even need to differenciate it as ./- (or with \"-- -\")?\n\n   After all, if there is no branch whose name is 'foo' and a file\n   'foo' exists on the filesystem, the user can say \"git reset foo\"\n   without disambiguation to reset that path, no?\n"}]}