{"thread":{"id":"38696","subject":"[PATCH] reset: allow \"-\" short hand for previous commit","startedAt":"2015-03-03T20:51:51Z","lastAt":"2015-03-08T18:09:37Z","messageCount":12,"participants":["Sudhanshu Shekhar","Matthieu Moy","Junio C Hamano","Eric Sunshine","David Aguilar"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"256929","messageId":"1425415911-496-1-git-send-email-sudshekhar02@gmail.com","threadId":"38696","inReplyTo":null,"subject":"[PATCH] reset: allow \"-\" short hand for previous commit","fromName":"Sudhanshu Shekhar","fromEmail":"sudshekhar02@gmail.com","sentAt":"2015-03-03T20:51:51Z","receivedAt":"2015-03-03T20:51:51Z","isPatch":true,"sender":{"key":"sudshekhar02@gmail.com","avatar":null},"body":"From: SudShekhar \n\nTeach reset the same shorthand as checkout and merge. \"-\" means the\n\"previous commit\".\n\nSigned-off-by: Sudhanshu Shekhar \n---\nThis is done as a microproject for gsoc purposes. I am looking forward to your feedback/comments. Thanks\nbuiltin/reset.c | 2 ++\n 1 file changed, 2 insertions(+)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 4c08ddc..3e0378b 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -205,6 +205,8 @@ static void parse_args(struct pathspec *pathspec,\n \t */\n \n \tif (argv[0]) {\n+\t\tif(!strcmp(argv[0],\"-\"))\n+\t\t\targv[0]=\"@{-1}\";\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-- \n2.3.1\n"},{"id":"256939","messageId":"vpq4mq13f69.fsf@anie.imag.fr","threadId":"38696","inReplyTo":"1425415911-496-1-git-send-email-sudshekhar02@gmail.com","subject":"Re: [PATCH] reset: allow \"-\" short hand for previous commit","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-03-03T22:10:54Z","receivedAt":"2015-03-03T22:10:54Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Sudhanshu Shekhar writes:\n\n> From: SudShekhar \n\nPlease, set your configuration to have the same identity for commit and\nsend-email. It seems your commiter ID (user.name) does not contain your\nlast name.\n\n> builtin/reset.c | 2 ++\n\nDoesn't this deserve a test?\n\n+\t\tif(!strcmp(argv[0],\"-\"))\n+\t\t\targv[0]=\"@{-1}\";\n\nWrong spacing (around = and after ,).\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"256948","messageId":"xmqq1tl5acwu.fsf@gitster.dls.corp.google.com","threadId":"38696","inReplyTo":"vpq4mq13f69.fsf@anie.imag.fr","subject":"Re: [PATCH] reset: allow \"-\" short hand for previous commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-03T23:17:53Z","receivedAt":"2015-03-03T23:17:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthieu Moy writes:\n\n> Sudhanshu Shekhar writes:\n>\n>> From: SudShekhar \n>\n> Please, set your configuration to have the same identity for commit and\n> send-email. It seems your commiter ID (user.name) does not contain your\n> last name.\n\nActually, the token does not match either of the two names; it looks\nlike two names smashed together into a single nickname token.\n\n>> builtin/reset.c | 2 ++\n>\n> Doesn't this deserve a test?\n>\n> +\t\tif(!strcmp(argv[0],\"-\"))\n> +\t\t\targv[0]=\"@{-1}\";\n>\n> Wrong spacing (around = and after ,).\n\nWhat should worry us even more is what the user would get when @{-1}\ndoes not resolve to something the command can use. It would be bad\nif we give an error message with @{-1} in it that the user never\ntyped (and may not even understand what it means).\n"},{"id":"256964","messageId":"CAODo60qimjiMfWY_FNuOcVaYc=JdbTv_4SJ3BROC-9Jo5qWkdA@mail.gmail.com","threadId":"38696","inReplyTo":"xmqq1tl5acwu.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH] reset: allow \"-\" short hand for previous commit","fromName":"Sudhanshu Shekhar","fromEmail":"sudshekhar02@gmail.com","sentAt":"2015-03-04T07:07:03Z","receivedAt":"2015-03-04T07:07:03Z","isPatch":true,"sender":{"key":"sudshekhar02@gmail.com","avatar":null},"body":"Hi,\n\n> Matthieu Moy writes:\n>\n>> Sudhanshu Shekhar writes:\n>>\n>>> From: SudShekhar \n>>\n>> Please, set your configuration to have the same identity for commit and\n>> send-email. It seems your commiter ID (user.name) does not contain your\n>> last name.\n>\n> Actually, the token does not match either of the two names; it looks\n> like two names smashed together into a single nickname token.\n\nSorry, I had set my irc nick as the commiter id. I have changed this to my name.\n\n>>> builtin/reset.c | 2 ++\n>>\n>> Doesn't this deserve a test?\n>>\n>> + if(!strcmp(argv[0],\"-\"))\n>> + argv[0]=\"@{-1}\";\n>>\n>> Wrong spacing (around = and after ,).\n\nThanks for pointing this out. I have corrected it.\n\n> What should worry us even more is what the user would get when @{-1}\n> does not resolve to something the command can use. It would be bad\n> if we give an error message with @{-1} in it that the user never\n> typed (and may not even understand what it means).\n\n\nI apologize for having overlooked this use case. I have made some\nchanges and I think this should work now. I will email the updated\npatch in the next email (I hope this is the correct procedure). Please\nlet me know your thoughts.\nAlso, this use case hasn't been handled in the case of git merge.\n\nAnother thing, can someone guide me regarding the right place to add a\ntest case? Should it be t7102-reset.sh or some other file?\n"},{"id":"256965","messageId":"1425452996-11682-1-git-send-email-sudshekhar02@gmail.com","threadId":"38696","inReplyTo":"CAODo60qimjiMfWY_FNuOcVaYc=JdbTv_4SJ3BROC-9Jo5qWkdA@mail.gmail.com","subject":"[PATCH] reset: allow \"-\" short hand for previous commit","fromName":"Sudhanshu Shekhar","fromEmail":"sudshekhar02@gmail.com","sentAt":"2015-03-04T07:09:56Z","receivedAt":"2015-03-04T07:09:56Z","isPatch":true,"sender":{"key":"sudshekhar02@gmail.com","avatar":null},"body":"Teach reset the same shorthand as checkout and merge. \"-\" means the\n\"previous commit\".\n\nSigned-off-by: Sudhanshu Shekhar \n---\n builtin/reset.c | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 4c08ddc..9f8967d 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 substituted_minus = 0;\n \t/*\n \t * Possible arguments are:\n \t *\n@@ -205,6 +206,10 @@ static void parse_args(struct pathspec *pathspec,\n \t */\n \n \tif (argv[0]) {\n+\t\tif(!strcmp(argv[0], \"-\")) {\n+\t\t\targv[0] = \"@{-1}\";\n+\t\t\tsubstituted_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@@ -225,12 +230,14 @@ static void parse_args(struct pathspec *pathspec,\n \t\t\tverify_non_filename(prefix, argv[0]);\n \t\t\trev = *argv++;\n \t\t} else {\n+\t\t\t/* We were treating \"-\" as a commit and not a file */\n+\t\t\tif(substituted_minus)\n+\t\t\t\targv[0] = \"-\";\n \t\t\t/* Otherwise we treat this as a filename */\n \t\t\tverify_filename(prefix, argv[0], 1);\n \t\t}\n \t}\n \t*rev_ret = rev;\n-\n \tif (read_cache() < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n-- \n2.3.1.168.gfd4bc34.dirty\n"},{"id":"256966","messageId":"CAPig+cRXZ+QRqxGPCkHTbW7ZbuPVpJndZU-7mS7GdtnqScfwfg@mail.gmail.com","threadId":"38696","inReplyTo":"CAODo60qimjiMfWY_FNuOcVaYc=JdbTv_4SJ3BROC-9Jo5qWkdA@mail.gmail.com","subject":"Re: [PATCH] reset: allow \"-\" short hand for previous commit","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-03-04T07:10:55Z","receivedAt":"2015-03-04T07:10:55Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Wed, Mar 4, 2015 at 2:07 AM, Sudhanshu Shekhar\n wrote:\n>> Matthieu Moy writes:\n>>> Sudhanshu Shekhar writes:\n>>>> From: SudShekhar \n>>>\n>>> + if(!strcmp(argv[0],\"-\"))\n>>> + argv[0]=\"@{-1}\";\n>>>\n>>> Wrong spacing (around = and after ,).\n>\n> Thanks for pointing this out. I have corrected it.\n\nAlso, add a space after 'if'.\n"},{"id":"257043","messageId":"xmqq1tl4705j.fsf@gitster.dls.corp.google.com","threadId":"38696","inReplyTo":"CAODo60qimjiMfWY_FNuOcVaYc=JdbTv_4SJ3BROC-9Jo5qWkdA@mail.gmail.com","subject":"Re: [PATCH] reset: allow \"-\" short hand for previous commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-03-05T00:34:00Z","receivedAt":"2015-03-05T00:34:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Sudhanshu Shekhar writes:\n\n>> What should worry us even more is what the user would get when @{-1}\n>> does not resolve to something the command can use. It would be bad\n>> if we give an error message with @{-1} in it that the user never\n>> typed (and may not even understand what it means).\n>\n> I apologize for having overlooked this use case. \n\nThanking is fine, but there is no need to apologize in response to\nreview comments. We are imperfect humans and review exchanges are\ndesigned to allow us cover points each of us missed in our\ncollective effort to make Git better.\n\n> Another thing, can someone guide me regarding the right place to add a\n> test case? Should it be t7102-reset.sh or some other file?\n\nAt the end of that file sounds like a reasonable choice. You would\nwant to test various cases, such as (1) what happens when there is\nno @{-1} at all (you would need a newly initialied test repository,\njust like the last test in that file creates mixed_worktree\nrepository for its own use), (2) what happens when there is, split\ninto two , i.e. (1-a) @{-1} does not exist but there is a file whose\nname is \"-\"; (1-b) @{-1} does not exist and there is no file whose\nname is \"-\"; (2-a) @{-1} exists but there is a file whose name is\n\"-\"; and (2-b) @{-1} exists and there is no file whose name is \"-\".\n\nDo not just test (2-b) and declare victory---making sure that a\nfeature does not trigger when it should not is also important.\n\nThanks.\n"},{"id":"257237","messageId":"1425762244-8629-1-git-send-email-sudshekhar02@gmail.com","threadId":"38696","inReplyTo":"xmqq1tl4705j.fsf@gitster.dls.corp.google.com","subject":"[PATCH 1/2] reset: allow \"-\" short hand for previous commit","fromName":"Sudhanshu Shekhar","fromEmail":"sudshekhar02@gmail.com","sentAt":"2015-03-07T21:04:04Z","receivedAt":"2015-03-07T21:04:04Z","isPatch":true,"sender":{"key":"sudshekhar02@gmail.com","avatar":null},"body":"Teach reset the same shorthand as checkout and merge. \"-\" means the\n\"previous commit\".\n\nSigned-off-by: Sudhanshu Shekhar \n---\n builtin/reset.c | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 4c08ddc..9f8967d 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 substituted_minus = 0;\n \t/*\n \t * Possible arguments are:\n \t *\n@@ -205,6 +206,10 @@ static void parse_args(struct pathspec *pathspec,\n \t */\n \n \tif (argv[0]) {\n+\t\tif(!strcmp(argv[0], \"-\")) {\n+\t\t\targv[0] = \"@{-1}\";\n+\t\t\tsubstituted_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@@ -225,12 +230,14 @@ static void parse_args(struct pathspec *pathspec,\n \t\t\tverify_non_filename(prefix, argv[0]);\n \t\t\trev = *argv++;\n \t\t} else {\n+\t\t\t/* We were treating \"-\" as a commit and not a file */\n+\t\t\tif(substituted_minus)\n+\t\t\t\targv[0] = \"-\";\n \t\t\t/* Otherwise we treat this as a filename */\n \t\t\tverify_filename(prefix, argv[0], 1);\n \t\t}\n \t}\n \t*rev_ret = rev;\n-\n \tif (read_cache() < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n-- \n2.3.1.168.g0c82976.dirty\n\n\n>From 21f0298c17768aaa11ff0a677cdefc8f54ac9515 Mon Sep 17 00:00:00 2001\nFrom: Sudhanshu Shekhar \nDate: Sun, 8 Mar 2015 02:13:57 +0530\nSubject: [PATCH 2/2] Added test cases for reset -\n\nFour test cases have been added\n\n1) when user does reset - without any previous branch => Leads to error\n2) when user does reset - with a previous branch => Ensure it\nbehaves like @{-1}\n\nOther two deal with the situation when we have a file named '-'. We\nignore such a file and - is always treated either as a previous branch\nor a bad filename. Users who wish to reset a file named '-' should specify\nit as './-'\n---\n builtin/reset.c | 4 ++--\n t/t7102-reset.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 64 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 9f8967d..02f33ef 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -206,7 +206,7 @@ static void parse_args(struct pathspec *pathspec,\n \t */\n \n \tif (argv[0]) {\n-\t\tif(!strcmp(argv[0], \"-\")) {\n+\t\tif (!strcmp(argv[0], \"-\")) {\n \t\t\targv[0] = \"@{-1}\";\n \t\t\tsubstituted_minus = 1;\n \t\t}\n@@ -231,7 +231,7 @@ static void parse_args(struct pathspec *pathspec,\n \t\t\trev = *argv++;\n \t\t} else {\n \t\t\t/* We were treating \"-\" as a commit and not a file */\n-\t\t\tif(substituted_minus)\n+\t\t\tif (substituted_minus)\n \t\t\t\targv[0] = \"-\";\n \t\t\t/* Otherwise we treat this as a filename */\n \t\t\tverify_filename(prefix, argv[0], 1);\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex 98bcfe2..4b8d7f5 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -568,4 +568,66 @@ test_expect_success 'reset --mixed sets up work tree' '\n \ttest_cmp expect actual\n '\n \n+cat > expect << EOF\n+fatal: bad flag '-' used after filename\n+EOF\n+\n+test_expect_success 'reset - with no previous branch' '\n+\tgit init no_previous --quiet &&\n+\t(\n+\tcd no_previous\n+\t) &&\n+\ttest_must_fail git reset - 2> output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success 'reset - while having file named - and no previous branch' '\n+\tgit init no_previous --quiet &&\n+\t(\n+\tcd no_previous &&\n+\ttouch ./-\n+\t) &&\n+\ttest_must_fail git reset - 2> output &&\n+\ttest_cmp expect output\n+'\n+\n+cat > expect << EOF\n+Unstaged changes after reset:\n+M\t-\n+M\t1\n+EOF\n+\n+test_expect_success 'reset - in the prescence of file named - with previou branch' '\n+\tgit init no_previous --quiet &&\n+\tcd no_previous &&\n+\ttouch ./- 1 &&\n+\tgit add 1 - &&\n+\tgit commit -m \"add base files\" &&\n+\tgit checkout -b new_branch &&\n+\techo \"random\" > ./- &&\n+\techo \"wow\" > 1 &&\n+\tgit add 1 - &&\n+\tgit reset - > output &&\n+\ttest_cmp output ../expect\n+'\n+test_expect_success 'reset - works same as reset @{-1}' '\n+\tgit init no_previous --quiet &&\n+\tcd no_previous &&\n+\techo \"random\" > random &&\n+\tgit add random &&\n+\tgit commit -m \"base commit\" &&\n+\tgit checkout -b temp &&\n+\techo new-file > new-file &&\n+\tgit add new-file &&\n+\tgit commit -m \"added new-file\" &&\n+\tgit reset - &&\n+\n+\tgit status > ../first &&\n+\tgit add new-file &&\n+\tgit commit -m \"added new-file\" &&\n+\tgit reset @{-1} &&\n+\tgit status > ../second &&\n+\ttest_cmp ../first ../second\n+'\n+\n test_done\n-- \n2.3.1.168.g0c82976.dirty\n"},{"id":"257316","messageId":"vpqioeb22y8.fsf@anie.imag.fr","threadId":"38696","inReplyTo":"1425762244-8629-1-git-send-email-sudshekhar02@gmail.com","subject":"Re: [PATCH 1/2] reset: allow \"-\" short hand for previous commit","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2015-03-08T10:33:51Z","receivedAt":"2015-03-08T10:33:51Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"Sudhanshu Shekhar writes:\n\n> +\t\tif(!strcmp(argv[0], \"-\")) {\n[...]\n> diff --git a/builtin/reset.c b/builtin/reset.c\n> index 9f8967d..02f33ef 100644\n> --- a/builtin/reset.c\n> +++ b/builtin/reset.c\n> @@ -206,7 +206,7 @@ static void parse_args(struct pathspec *pathspec,\n> \t */\n> \n> \tif (argv[0]) {\n> -\t\tif(!strcmp(argv[0], \"-\")) {\n> +\t\tif (!strcmp(argv[0], \"-\")) {\n\nPlease, squash this hunk into the previous patch, so that reviewers get\nthe right version right away.\n\nAlso, send each patch as a separate email (git send-email can do that\nfor you).\n\n> +\ttest_must_fail git reset - 2> output &&\n\nHere and elsewhere: no space after >.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"},{"id":"257324","messageId":"1425826720-5899-1-git-send-email-sudshekhar02@gmail.com","threadId":"38696","inReplyTo":"vpqioeb22y8.fsf@anie.imag.fr","subject":"[PATCH 1/2] Teach reset the same short-hand as checkout","fromName":"Sudhanshu Shekhar","fromEmail":"sudshekhar02@gmail.com","sentAt":"2015-03-08T14:58:39Z","receivedAt":"2015-03-08T14:58:39Z","isPatch":true,"sender":{"key":"sudshekhar02@gmail.com","avatar":null},"body":"\"-\" now means the previous branch.\n\nSigned-off-by: Sudhanshu Shekhar \nThanks-to: Junio C Hamano, Matthieu Moy, Eric Sunshine\n---\nThank you all for your feedback. Please let me know if I am missing out on anything else.\n\n\n builtin/reset.c | 9 ++++++++-\n 1 file changed, 8 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 4c08ddc..02f33ef 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 substituted_minus = 0;\n \t/*\n \t * Possible arguments are:\n \t *\n@@ -205,6 +206,10 @@ static void parse_args(struct pathspec *pathspec,\n \t */\n \n \tif (argv[0]) {\n+\t\tif (!strcmp(argv[0], \"-\")) {\n+\t\t\targv[0] = \"@{-1}\";\n+\t\t\tsubstituted_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@@ -225,12 +230,14 @@ static void parse_args(struct pathspec *pathspec,\n \t\t\tverify_non_filename(prefix, argv[0]);\n \t\t\trev = *argv++;\n \t\t} else {\n+\t\t\t/* We were treating \"-\" as a commit and not a file */\n+\t\t\tif (substituted_minus)\n+\t\t\t\targv[0] = \"-\";\n \t\t\t/* Otherwise we treat this as a filename */\n \t\t\tverify_filename(prefix, argv[0], 1);\n \t\t}\n \t}\n \t*rev_ret = rev;\n-\n \tif (read_cache() < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n-- \n2.3.1.168.g0c82976.dirty\n"},{"id":"257325","messageId":"1425826720-5899-2-git-send-email-sudshekhar02@gmail.com","threadId":"38696","inReplyTo":"1425826720-5899-1-git-send-email-sudshekhar02@gmail.com","subject":"[PATCH 2/2] Added test cases for git reset -","fromName":"Sudhanshu Shekhar","fromEmail":"sudshekhar02@gmail.com","sentAt":"2015-03-08T14:58:40Z","receivedAt":"2015-03-08T14:58:40Z","isPatch":true,"sender":{"key":"sudshekhar02@gmail.com","avatar":null},"body":"Four test cases have been added\n\n1) when user does reset - without any previous branch => Leads to error\n2) when user does reset - with a previous branch => Ensure it\nbehaves like {-1}\n\nOther two deal with the situation when we have a file named '-'. We\nignore such a file and - is always treated either as a previous branch\nor a bad filename. Users who wish to reset a file named '-' should\nspecify\nit as './-'\n\nSigned-off-by: Sudhanshu Shekhar \n---\nI have created test cases for git reset -. @Junio, I tried incorporating your suggestions while developing these test cases.\nHowever, since the verify_filename function ignores files starting with \"-\", git reset - will always refer to the branch only. Kindly let me know your thoughts and views on this and also your reviews about the test cases I have created.\n\nRegards,\nSudhanshu\n\n t/t7102-reset.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n 1 file changed, 62 insertions(+)\n\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex 98bcfe2..ade3d6a 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -568,4 +568,66 @@ test_expect_success 'reset --mixed sets up work tree' '\n \ttest_cmp expect actual\n '\n \n+cat > expect << EOF\n+fatal: bad flag '-' used after filename\n+EOF\n+\n+test_expect_success 'reset - with no previous branch' '\n+\tgit init no_previous --quiet &&\n+\t(\n+\tcd no_previous\n+\t) &&\n+\ttest_must_fail git reset - 2>output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success 'reset - while having file named - and no previous branch' '\n+\tgit init no_previous --quiet &&\n+\t(\n+\tcd no_previous &&\n+\ttouch ./-\n+\t) &&\n+\ttest_must_fail git reset - 2>output &&\n+\ttest_cmp expect output\n+'\n+\n+cat > expect << EOF\n+Unstaged changes after reset:\n+M\t-\n+M\t1\n+EOF\n+\n+test_expect_success 'reset - in the prescence of file named - with previou branch' '\n+\tgit init no_previous --quiet &&\n+\tcd no_previous &&\n+\ttouch ./- 1 &&\n+\tgit add 1 - &&\n+\tgit commit -m \"add base files\" &&\n+\tgit checkout -b new_branch &&\n+\techo \"random\" >./- &&\n+\techo \"wow\" >1 &&\n+\tgit add 1 - &&\n+\tgit reset - >output &&\n+\ttest_cmp output ../expect\n+'\n+test_expect_success 'reset - works same as reset @{-1}' '\n+\tgit init no_previous --quiet &&\n+\tcd no_previous &&\n+\techo \"random\" >random &&\n+\tgit add random &&\n+\tgit commit -m \"base commit\" &&\n+\tgit checkout -b temp &&\n+\techo new-file >new-file &&\n+\tgit add new-file &&\n+\tgit commit -m \"added new-file\" &&\n+\tgit reset - &&\n+\n+\tgit status >../first &&\n+\tgit add new-file &&\n+\tgit commit -m \"added new-file\" &&\n+\tgit reset @{-1} &&\n+\tgit status >../second &&\n+\ttest_cmp ../first ../second\n+'\n+\n test_done\n-- \n2.3.1.168.g0c82976.dirty\n"},{"id":"257334","messageId":"20150308180934.GA18215@gmail.com","threadId":"38696","inReplyTo":"1425826720-5899-2-git-send-email-sudshekhar02@gmail.com","subject":"Re: [PATCH 2/2] Added test cases for git reset -","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2015-03-08T18:09:37Z","receivedAt":"2015-03-08T18:09:37Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"Hi,\n\nOn Sun, Mar 08, 2015 at 08:28:40PM +0530, Sudhanshu Shekhar wrote:\n> Four test cases have been added\n> \n> 1) when user does reset - without any previous branch => Leads to error\n> 2) when user does reset - with a previous branch => Ensure it\n> behaves like {-1}\n> \n> Other two deal with the situation when we have a file named '-'. We\n> ignore such a file and - is always treated either as a previous branch\n> or a bad filename. Users who wish to reset a file named '-' should\n> specify\n> it as './-'\n> \n> Signed-off-by: Sudhanshu Shekhar \n> ---\n\n\nPlease reword the commit message so that it uses an imperative\nmood; see ~line 112 in Documentation/SubmittingPatches for an\nexplanation.\n\n\n> t/t7102-reset.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> 1 file changed, 62 insertions(+)\n> \n> diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\n> index 98bcfe2..ade3d6a 100755\n> --- a/t/t7102-reset.sh\n> +++ b/t/t7102-reset.sh\n> @@ -568,4 +568,66 @@ test_expect_success 'reset --mixed sets up work tree' '\n> \ttest_cmp expect actual\n> '\n> \n> +cat > expect << EOF\n\n\nPlease drop the space after \">\" and \"<<\".\n\n\n> +fatal: bad flag '-' used after filename\n> +EOF\n> +\n> +test_expect_success 'reset - with no previous branch' '\n> +\tgit init no_previous --quiet &&\n> +\t(\n> +\tcd no_previous\n> +\t) &&\n> +\ttest_must_fail git reset - 2>output &&\n> +\ttest_cmp expect output\n> +'\n\n\nPlease indent lines inside the (...) parens so that it's easier\nto notice that they are executing within a subshell, e.g.\n\n\tgit init no_previous --quiet &&\n\t(\n\t\tcd no_previous &&\n\t\t...\n\t) &&\n\t...\n\nAs written, the above test verifies that we can \"cd\" into\n\"no_previous\", but because it's a subshell the subsequent\ncommands after the parens will not be run within that\nsubdirectory.\n\nThus, the \"git reset -\" that we assert must fail is happening in\nthe outer directory, not the inner no_previous repo.\n\nIf that's all we wanted to verify then the (...) sub-shelled cd\ncommand could be replaced entirely by \"test -d no_previous\", but\nI don't think that's the intention of this test.\n\nI believe you may have intended to write the following instead:\n\ntest_expect_success 'reset - with no previous branch' '\n\tgit init no_previous --quiet &&\n\t(\n\t\tcd no_previous &&\n\t\ttest_must_fail git reset - 2>../output\n\t) &&\n\ttest_cmp expect output\n'\n\n\"output\" becomes \"../output\" because we're one directory down.\n\n\n> +\n> +test_expect_success 'reset - while having file named - and no previous branch' '\n> +\tgit init no_previous --quiet &&\n> +\t(\n> +\tcd no_previous &&\n> +\ttouch ./-\n> +\t) &&\n> +\ttest_must_fail git reset - 2>output &&\n> +\ttest_cmp expect output\n> +'\n\n\nDitto; please indent (...) subshells and move the \"git reset\"\ninvocation into the subshell so that it runs in the context of\nthe no_previous repo. The output path will need the same\nadjustment as above.\n\n\n> +\n> +cat > expect << EOF\n\n\nDitto; no space after > and <<.\n\n\n> +Unstaged changes after reset:\n> +M\t-\n> +M\t1\n> +EOF\n> +\n> +test_expect_success 'reset - in the prescence of file named - with previou branch' '\n> +\tgit init no_previous --quiet &&\n> +\tcd no_previous &&\n\n\nTests that need to change the current directory with \"cd\" should\ngenerally always use a (...) subshell. It prevents them from\nneeding to manually undo the \"cd\" before running subsequent\ncommands that need to be in the original, parent directory.\n\n\n> +\ttouch ./- 1 &&\n> +\tgit add 1 - &&\n> +\tgit commit -m \"add base files\" &&\n> +\tgit checkout -b new_branch &&\n> +\techo \"random\" >./- &&\n> +\techo \"wow\" >1 &&\n> +\tgit add 1 - &&\n> +\tgit reset - >output &&\n> +\ttest_cmp output ../expect\n\n\nWhen applying the previous note, if we keep the test_cmp line\noutside of the (...) subshell then we won't need to use \"../\"\nwhen referring to \"expect\" here, but we will need it for\n\"../output\" file on the line above it.\n\n\n> +'\n> +test_expect_success 'reset - works same as reset @{-1}' '\n> +\tgit init no_previous --quiet &&\n> +\tcd no_previous &&\n\n\nDitto; please use a subshell when entering \"no_previous\".\n\n\n> +\techo \"random\" >random &&\n> +\tgit add random &&\n> +\tgit commit -m \"base commit\" &&\n> +\tgit checkout -b temp &&\n> +\techo new-file >new-file &&\n> +\tgit add new-file &&\n> +\tgit commit -m \"added new-file\" &&\n> +\tgit reset - &&\n> +\n> +\tgit status >../first &&\n> +\tgit add new-file &&\n> +\tgit commit -m \"added new-file\" &&\n> +\tgit reset @{-1} &&\n> +\tgit status >../second &&\n> +\ttest_cmp ../first ../second\n> +'\n> +\n> test_done\n\n\nThis test uses \"git status\" to capture the worktree state so\nthat we can verify that calling \"reset -\" and \"reset @{-1}\" are\nequivalent.\n\nBare \"git status\" is not a plumbing command. This doesn't make a\npractical difference for the purpose of this test, but it's\nprobably a good idea to use the plumbing form of \"git status\" by\npassing the \"--porcelain\" flag when calling it here.\n\n\nIn addition to these tests, it might also be worth adding test\ncases to ensure that we unambiguously differentiate the \"-\"\nshortcut from a file when the \"--\" marker is used in a repo that\ncontains a \"-\" file. When running the following two commands,\n\n\tgit reset - --\n\tgit reset -- -\n\nwe should test that these are unambiguous because of the \"--\".\n\nThe first command should notice the dash-dash and treat \"-\" like\na shortcut despite the existence of a file named \"-\".\n\nThe second command should operate on the \"-\" file only and\notherwise leave the repo state as-is.\n\nIf we want to be especially thorough then we should also test\nthis invocation:\n\n\tgit reset - -- -\n\nThis invocation should reset the index to the previous commit\nfor the \"-\" file only.\n\nI don't want to increase the scope of this commit so it might\nnot hurt to add these additional tests as a separate patch.\n-- \nDavid\n"}]}