{"thread":{"id":"11312","subject":"[PATCH] Add an \"-i\" option to git-reset, to confirm a reset.","startedAt":"2007-12-16T03:26:36Z","lastAt":"2007-12-16T21:26:53Z","messageCount":5,"participants":["Kelvie Wong","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"63287","messageId":"1197775596-14329-1-git-send-email-kelvie@ieee.org","threadId":"11312","inReplyTo":null,"subject":"[PATCH] Add an \"-i\" option to git-reset, to confirm a reset.","fromName":"Kelvie Wong","fromEmail":"kelvie@ieee.org","sentAt":"2007-12-16T03:26:36Z","receivedAt":"2007-12-16T03:26:36Z","isPatch":true,"sender":{"key":"kelvie@ieee.org","avatar":null},"body":"It shows a diffstat, and asks the user if they would like to continue, or\nshow a full diff of the things getting reset.\n\nI know that many times, I do a reset --hard thinking I had commited a file\nalready, but it turns out that I hadn't; and so this makes sure I don't\nlose any work when the caffeine wears off.\n\nMaybe it should also be made that only hard resets take this option, as\nI cannot see this being useful in other places.\n\nSigned-off-by: Kelvie Wong \n---\n Documentation/git-reset.txt | 4 +++\n builtin-reset.c | 46 ++++++++++++++++++++++++++++++++++++++----\n 2 files changed, 45 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt\nindex 050e4ea..0323d9d 100644\n--- a/Documentation/git-reset.txt\n+++ b/Documentation/git-reset.txt\n@@ -48,6 +48,10 @@ OPTIONS\n -q::\n \tBe quiet, only report errors.\n \n+-i::\n+\tShow what is about to be reset, and ask for confirmation before doing\n+\tso.\n+\n ::\n \tCommit to make the current HEAD.\n \ndiff --git a/builtin-reset.c b/builtin-reset.c\nindex 713c2d5..1086817 100644\n--- a/builtin-reset.c\n+++ b/builtin-reset.c\n@@ -18,7 +18,8 @@\n #include \"tree.h\"\n \n static const char builtin_reset_usage[] =\n-\"git-reset [--mixed | --soft | --hard] [-q] [] [ [--] ...]\";\n+\"git-reset [--mixed | --soft | --hard] [-q] [-i] [] [ [--] \"\n+\"...]\";\n \n static char *args_to_str(const char **argv)\n {\n@@ -56,17 +57,47 @@ static int unmerged_files(void)\n \treturn 0;\n }\n \n-static int reset_index_file(const unsigned char *sha1, int is_hard_reset)\n+static int reset_index_file(const unsigned char *sha1, int is_hard_reset,\n+\t\t\t int confirm_reset)\n {\n \tint i = 0;\n \tconst char *args[6];\n+\tstruct strbuf buf;\n+\tchar result = 0;\n+\tconst char *ref = sha1_to_hex(sha1);\n+\tconst char *diffstat_args[] = { \"diff\", \"--stat\", ref, NULL };\n+\tconst char *diff_args[] = { \"diff\", ref, NULL };\n \n \targs[i++] = \"read-tree\";\n \targs[i++] = \"-v\";\n \targs[i++] = \"--reset\";\n+\n+\t/* Show the user what is about to be reset, and in more detail, if they\n+\t * like. */\n+\tif(confirm_reset) {\n+\t\tprintf(\"The following files will be reset:\\n\");\n+\t\trun_command_v_opt(diffstat_args, RUN_GIT_CMD);\n+\t\tstrbuf_init(&buf, 0);\n+\t\twhile(result != 'y') {\n+\t\t\tprintf(\"Continue? ((y)es/(n)o/view (d)iff)\\n\");\n+\t\t\tstrbuf_getline(&buf, stdin, '\\n');\n+\t\t\tresult = tolower(buf.buf[0]);\n+\t\t\tswitch(result) {\n+\t\t\tcase 'd':\n+\t\t\t\trun_command_v_opt(diff_args, RUN_GIT_CMD);\n+\t\t\t\tbreak;\n+\t\t\tcase 'n':\n+\t\t\t\treturn 1;\n+\t\t\t\tbreak;\n+\t\t\t};\n+\t\t}\n+\t\tstrbuf_release(&buf);\n+ }\n+\n+\n \tif (is_hard_reset)\n \t\targs[i++] = \"-u\";\n-\targs[i++] = sha1_to_hex(sha1);\n+\targs[i++] = ref;\n \targs[i] = NULL;\n \n \treturn run_command_v_opt(args, RUN_GIT_CMD);\n@@ -181,7 +212,8 @@ static const char *reset_type_names[] = { \"mixed\", \"soft\", \"hard\", NULL };\n \n int cmd_reset(int argc, const char **argv, const char *prefix)\n {\n-\tint i = 1, reset_type = NONE, update_ref_status = 0, quiet = 0;\n+\tint i = 1, reset_type = NONE, update_ref_status = 0, quiet = 0,\n+\t\tconfirm = 0;\n \tconst char *rev = \"HEAD\";\n \tunsigned char sha1[20], *orig = NULL, sha1_orig[20],\n \t\t\t\t*old_orig = NULL, sha1_old_orig[20];\n@@ -210,6 +242,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\t\tquiet = 1;\n \t\t\ti++;\n \t\t}\n+\t\telse if (!strcmp(argv[i], \"-i\")) {\n+\t\t\tconfirm = 1;\n+\t\t\ti++;\n+\t\t}\n \t\telse\n \t\t\tbreak;\n \t}\n@@ -251,7 +287,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\tif (is_merge() || unmerged_files())\n \t\t\tdie(\"Cannot do a soft reset in the middle of a merge.\");\n \t}\n-\telse if (reset_index_file(sha1, (reset_type == HARD)))\n+\telse if (reset_index_file(sha1, (reset_type == HARD), confirm))\n \t\tdie(\"Could not reset index file to revision '%s'.\", rev);\n \n \t/* Any resets update HEAD to the head being switched to,\n-- \n1.5.4-rc0.GIT\n"},{"id":"63289","messageId":"Pine.LNX.4.64.0712160332140.27959@racer.site","threadId":"11312","inReplyTo":"1197775596-14329-1-git-send-email-kelvie@ieee.org","subject":"Re: [PATCH] Add an \"-i\" option to git-reset, to confirm a reset.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-12-16T03:35:09Z","receivedAt":"2007-12-16T03:35:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 15 Dec 2007, Kelvie Wong wrote:\n\n> It shows a diffstat, and asks the user if they would like to continue, \n> or show a full diff of the things getting reset.\n> \n> I know that many times, I do a reset --hard thinking I had commited a \n> file already, but it turns out that I hadn't; and so this makes sure I \n> don't lose any work when the caffeine wears off.\n> \n> Maybe it should also be made that only hard resets take this option, as \n> I cannot see this being useful in other places.\n\nI am slightly negative on this patch. Not only do I think that it is both \neasier and more natural to run diff/status/an-alias to see what a reset \nwould do, but the patch only handles the index_file part (missing the -- \n part AFAICT).\n\nBesides, the code style is incompatible with the surrounding code.\n\nCiao,\nDscho\n"},{"id":"63291","messageId":"94ccbe710712151946u22f02a8fkbc3c4cbc96ee22f5@mail.gmail.com","threadId":"11312","inReplyTo":"Pine.LNX.4.64.0712160332140.27959@racer.site","subject":"Re: [PATCH] Add an \"-i\" option to git-reset, to confirm a reset.","fromName":"Kelvie Wong","fromEmail":"kelvie@ieee.org","sentAt":"2007-12-16T03:46:10Z","receivedAt":"2007-12-16T03:46:10Z","isPatch":true,"sender":{"key":"kelvie@ieee.org","avatar":null},"body":"On Dec 15, 2007 7:35 PM, Johannes Schindelin wrote:\n> Hi,\n>\n> On Sat, 15 Dec 2007, Kelvie Wong wrote:\n>\n> > It shows a diffstat, and asks the user if they would like to continue,\n> > or show a full diff of the things getting reset.\n> >\n> > I know that many times, I do a reset --hard thinking I had commited a\n> > file already, but it turns out that I hadn't; and so this makes sure I\n> > don't lose any work when the caffeine wears off.\n> >\n> > Maybe it should also be made that only hard resets take this option, as\n> > I cannot see this being useful in other places.\n>\n> I am slightly negative on this patch. Not only do I think that it is both\n> easier and more natural to run diff/status/an-alias to see what a reset\n> would do, but the patch only handles the index_file part (missing the --\n> part AFAICT).\n>\n> Besides, the code style is incompatible with the surrounding code.\n>\n> Ciao,\n> Dscho\n>\n>\n[forgot to hit Reply To All again, sorry!]\n\nAh, you're completely right about the index_file part (this is\nactually the first time I've looked at the git-code :P)\n\nHrm.. I should have just used a shell script wrapper instead it seems.\n\nI do think something like this would be nice though.\n\nw.r.t. the style, you were referring to just the array initializers\nright? Or was there something else I did that doesn't look right?\n-- \nKelvie Wong\n"},{"id":"63344","messageId":"7vwsrejta3.fsf@gitster.siamese.dyndns.org","threadId":"11312","inReplyTo":"94ccbe710712151946u22f02a8fkbc3c4cbc96ee22f5@mail.gmail.com","subject":"Re: [PATCH] Add an \"-i\" option to git-reset, to confirm a reset.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-12-16T20:09:40Z","receivedAt":"2007-12-16T20:09:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kelvie Wong\" writes:\n\n> On Dec 15, 2007 7:35 PM, Johannes Schindelin wrote:\n> ..\n>> I am slightly negative on this patch. Not only do I think that it is both\n>> easier and more natural to run diff/status/an-alias to see what a reset\n>> would do, but the patch only handles the index_file part (missing the --\n>> part AFAICT).\n\nI am in principle very negative on additional option that does the same\nthing as what the users on odd occasions can run a separate command\nthemselves to achieve, and I think \"reset -i\" falls into that category.\n\nAnd I am negative on this \"-i\" not just because I think that would be\nonly in \"odd occasions\" (i.e. rare), but because I think it would not\nhelp much. Either you are sure about resetting, in which case you would\nnot even use \"-i\" option (and not get this safety), or you are unsure,\nin which case you can do \"git status\" or whatever commands that are\nalready available.\n\n> w.r.t. the style, you were referring to just the array initializers\n> right? Or was there something else I did that doesn't look right?\n\nI spotted only two classes.\n\n+\n+\t/* Show the user what is about to be reset, and in more detail, if they\n+\t * like. */\n\n\t/*\n * Show the user what is about to be reset, and in more detail,\n * if they like.\n */\n\n+\tif(confirm_reset) {\n\n\tif (confirm_reset) {\n"},{"id":"63352","messageId":"94ccbe710712161326q7360d910ld9442e73630d46af@mail.gmail.com","threadId":"11312","inReplyTo":"7vwsrejta3.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Add an \"-i\" option to git-reset, to confirm a reset.","fromName":"Kelvie Wong","fromEmail":"kelvie@ieee.org","sentAt":"2007-12-16T21:26:53Z","receivedAt":"2007-12-16T21:26:53Z","isPatch":true,"sender":{"key":"kelvie@ieee.org","avatar":null},"body":"On Dec 16, 2007 12:09 PM, Junio C Hamano wrote:\n> \"Kelvie Wong\" writes:\n>\n> > On Dec 15, 2007 7:35 PM, Johannes Schindelin wrote:\n> > ..\n> >> I am slightly negative on this patch. Not only do I think that it is both\n> >> easier and more natural to run diff/status/an-alias to see what a reset\n> >> would do, but the patch only handles the index_file part (missing the --\n> >> part AFAICT).\n>\n> I am in principle very negative on additional option that does the same\n> thing as what the users on odd occasions can run a separate command\n> themselves to achieve, and I think \"reset -i\" falls into that category.\n>\n> And I am negative on this \"-i\" not just because I think that would be\n> only in \"odd occasions\" (i.e. rare), but because I think it would not\n> help much. Either you are sure about resetting, in which case you would\n> not even use \"-i\" option (and not get this safety), or you are unsure,\n> in which case you can do \"git status\" or whatever commands that are\n> already available.\n>\n> > w.r.t. the style, you were referring to just the array initializers\n> > right? Or was there something else I did that doesn't look right?\n>\n> I spotted only two classes.\n>\n> +\n> + /* Show the user what is about to be reset, and in more detail, if they\n> + * like. */\n>\n> /*\n> * Show the user what is about to be reset, and in more detail,\n> * if they like.\n> */\n>\n> + if(confirm_reset) {\n>\n> if (confirm_reset) {\n>\n\nI guess this is just for people like me who are used to the \"M-x\nvc-revert\" function in emacs, which shows you exactly what changes get\nreverted before doing so.\n\nBut I guess it is quite trivial to make an alias to do the same when\ninvoking git-reset (or checkout) directly.\n\n-- \nKelvie Wong\n"}]}