{"thread":{"id":"12411","subject":"[PATCH] Make builtin-reset.c use parse_options.","startedAt":"2008-03-01T16:29:38Z","lastAt":"2008-03-04T09:45:28Z","messageCount":15,"participants":["Carlos Rica","Junio C Hamano","Alex Riesen","Johannes Schindelin","Pierre Habouzit"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"70540","messageId":"47C98472.8000002@gmail.com","threadId":"12411","inReplyTo":null,"subject":"[PATCH] Make builtin-reset.c use parse_options.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2008-03-01T16:29:38Z","receivedAt":"2008-03-01T16:29:38Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"\nSigned-off-by: Carlos Rica \n---\n builtin-reset.c | 47 ++++++++++++++++++++---------------------------\n 1 files changed, 20 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin-reset.c b/builtin-reset.c\nindex af0037e..71892d0 100644\n--- a/builtin-reset.c\n+++ b/builtin-reset.c\n@@ -17,9 +17,13 @@\n #include \"diffcore.h\"\n #include \"tree.h\"\n #include \"branch.h\"\n+#include \"parse-options.h\"\n\n-static const char builtin_reset_usage[] =\n-\"git-reset [--mixed | --soft | --hard] [-q] [] [ [--] ...]\";\n+static const char * const git_reset_usage[] = {\n+\t\"git-reset [--mixed | --soft | --hard] [-q] []\",\n+\t\"git-reset [--mixed] [--] ...\",\n+\tNULL\n+};\n\n static char *args_to_str(const char **argv)\n {\n@@ -169,40 +173,31 @@ 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 = 0, reset_type = NONE, update_ref_status = 0, quiet = 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 \tstruct commit *commit;\n \tchar *reflog_action, msg[1024];\n+\tstruct option options[] = {\n+\t\tOPT_SET_INT(0, \"mixed\", &reset_type,\n+\t\t\t\t\t\t\"reset HEAD and index\", MIXED),\n+\t\tOPT_SET_INT(0, \"soft\", &reset_type, \"reset only HEAD\", SOFT),\n+\t\tOPT_SET_INT(0, \"hard\", &reset_type,\n+\t\t\t\t\"reset HEAD, index and working tree\", HARD),\n+\t\tOPT_BOOLEAN('q', NULL, &quiet,\n+\t\t\t\t\"disable showing new HEAD in hard reset\"),\n+\t\tOPT_END()\n+\t};\n\n \tgit_config(git_default_config);\n\n+\targc = parse_options(argc, argv, options, git_reset_usage,\n+\t\t\t\t\t\tPARSE_OPT_KEEP_DASHDASH);\n \treflog_action = args_to_str(argv);\n \tsetenv(\"GIT_REFLOG_ACTION\", reflog_action, 0);\n\n-\twhile (i < argc) {\n-\t\tif (!strcmp(argv[i], \"--mixed\")) {\n-\t\t\treset_type = MIXED;\n-\t\t\ti++;\n-\t\t}\n-\t\telse if (!strcmp(argv[i], \"--soft\")) {\n-\t\t\treset_type = SOFT;\n-\t\t\ti++;\n-\t\t}\n-\t\telse if (!strcmp(argv[i], \"--hard\")) {\n-\t\t\treset_type = HARD;\n-\t\t\ti++;\n-\t\t}\n-\t\telse if (!strcmp(argv[i], \"-q\")) {\n-\t\t\tquiet = 1;\n-\t\t\ti++;\n-\t\t}\n-\t\telse\n-\t\t\tbreak;\n-\t}\n"},{"id":"70573","messageId":"7vir05x2di.fsf@gitster.siamese.dyndns.org","threadId":"12411","inReplyTo":"47C98472.8000002@gmail.com","subject":"Re: [PATCH] Make builtin-reset.c use parse_options.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-02T02:53:45Z","receivedAt":"2008-03-02T02:53:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Rica writes:\n\n> Signed-off-by: Carlos Rica \n\nHmmm. \"git reset -h\" now defaults to --hard? \n\nIt somehow feels a bit risky for new people. I dunno.\n"},{"id":"70609","messageId":"20080302094031.GA2973@steel.home","threadId":"12411","inReplyTo":"47C98472.8000002@gmail.com","subject":"Re: [PATCH] Make builtin-reset.c use parse_options.","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-03-02T09:40:31Z","receivedAt":"2008-03-02T09:40:31Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Carlos Rica, Sat, Mar 01, 2008 17:29:38 +0100:\n> @@ -169,40 +173,31 @@ 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 = 0, reset_type = NONE, update_ref_status = 0, quiet = 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> \tstruct commit *commit;\n> \tchar *reflog_action, msg[1024];\n> +\tstruct option options[] = {\n\n\"static const\"?\n\n"},{"id":"70631","messageId":"1b46aba20803020437p2a5128a5wfee3e98c32c1a610@mail.gmail.com","threadId":"12411","inReplyTo":"7vir05x2di.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Make builtin-reset.c use parse_options.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2008-03-02T12:37:38Z","receivedAt":"2008-03-02T12:37:38Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"On Sun, Mar 2, 2008 at 3:53 AM, Junio C Hamano wrote:\n> Carlos Rica writes:\n>\n> > Signed-off-by: Carlos Rica \n>\n> Hmmm. \"git reset -h\" now defaults to --hard?\n>\n> It somehow feels a bit risky for new people. I dunno.\n>\n\nI don't understand what do you mean.\n\nOption -h just prints the options and exits.\nDo you mean that the help message is wrong?\n\nAlso, there is a test to check that \"git reset\" defaults to\n--mixed ('--mixed reset to HEAD should unadd the files'\nonly run a \"git reset\" without parameters),\nand changing it to do a --hard reset makes the tests to fail.\n\nIf there's a change in the behaviour of the command,\nplease, show me how to test it. I cannot see anything\nwrong now.\n\n-- Carlos\n"},{"id":"70633","messageId":"1b46aba20803020454v322f061bi4f34a737e9e2b7f3@mail.gmail.com","threadId":"12411","inReplyTo":"20080302094031.GA2973@steel.home","subject":"Re: [PATCH] Make builtin-reset.c use parse_options.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2008-03-02T12:54:10Z","receivedAt":"2008-03-02T12:54:10Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"On Sun, Mar 2, 2008 at 10:40 AM, Alex Riesen wrote:\n> Carlos Rica, Sat, Mar 01, 2008 17:29:38 +0100:\n>\n> > @@ -169,40 +173,31 @@ 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> > - int i = 1, reset_type = NONE, update_ref_status = 0, quiet = 0;\n> > + int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;\n> > const char *rev = \"HEAD\";\n> > unsigned char sha1[20], *orig = NULL, sha1_orig[20],\n> > *old_orig = NULL, sha1_old_orig[20];\n> > struct commit *commit;\n> > char *reflog_action, msg[1024];\n> > + struct option options[] = {\n>\n> \"static const\"?\n>\n>\n\n\"static const\" what?\noptions? cmd_reset? reset_type_names?\n"},{"id":"70653","messageId":"20080302155541.GH2973@steel.home","threadId":"12411","inReplyTo":"1b46aba20803020454v322f061bi4f34a737e9e2b7f3@mail.gmail.com","subject":"Re: [PATCH] Make builtin-reset.c use parse_options.","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-03-02T15:55:41Z","receivedAt":"2008-03-02T15:55:41Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Carlos Rica, Sun, Mar 02, 2008 13:54:10 +0100:\n> On Sun, Mar 2, 2008 at 10:40 AM, Alex Riesen wrote:\n> > Carlos Rica, Sat, Mar 01, 2008 17:29:38 +0100:\n> >\n> > > @@ -169,40 +173,31 @@ 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> > > - int i = 1, reset_type = NONE, update_ref_status = 0, quiet = 0;\n> > > + int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0;\n> > > const char *rev = \"HEAD\";\n> > > unsigned char sha1[20], *orig = NULL, sha1_orig[20],\n> > > *old_orig = NULL, sha1_old_orig[20];\n> > > struct commit *commit;\n> > > char *reflog_action, msg[1024];\n> > > + struct option options[] = {\n> >\n> > \"static const\"?\n> >\n> >\n> \n> \"static const\" what?\n> options? cmd_reset? reset_type_names?\n\n\"static const struct option options[] = {\"\n\nthe others are already either statics or const properly\n"},{"id":"70657","messageId":"7vbq5xqetq.fsf@gitster.siamese.dyndns.org","threadId":"12411","inReplyTo":"1b46aba20803020437p2a5128a5wfee3e98c32c1a610@mail.gmail.com","subject":"Re: [PATCH] Make builtin-reset.c use parse_options.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-02T16:18:57Z","receivedAt":"2008-03-02T16:18:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Carlos Rica\" writes:\n\n> On Sun, Mar 2, 2008 at 3:53 AM, Junio C Hamano wrote:\n>> Carlos Rica writes:\n>>\n>> > Signed-off-by: Carlos Rica \n>>\n>> Hmmm. \"git reset -h\" now defaults to --hard?\n>>\n>> It somehow feels a bit risky for new people. I dunno.\n>\n> I don't understand what do you mean.\n>\n> Option -h just prints the options and exits.\n> Do you mean that the help message is wrong?\n\nI guess I mistested when I first ran it. \"-h\" is Ok.\n\n\t$ ./git-reset -h\n usage: git-reset ...\n\nAlthough \"--h\" still favors \"--hard\" over \"--help\":\n\n\t$ ./git-reset --h\n HEAD is now at c149184...\n\n"},{"id":"70659","messageId":"alpine.LSU.1.00.0803021631210.22527@racer.site","threadId":"12411","inReplyTo":"1b46aba20803020437p2a5128a5wfee3e98c32c1a610@mail.gmail.com","subject":"Re: [PATCH] Make builtin-reset.c use parse_options.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2008-03-02T16:32:18Z","receivedAt":"2008-03-02T16:32:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 2 Mar 2008, Carlos Rica wrote:\n\n> On Sun, Mar 2, 2008 at 3:53 AM, Junio C Hamano wrote:\n> > Carlos Rica writes:\n> >\n> > > Signed-off-by: Carlos Rica \n> >\n> > Hmmm. \"git reset -h\" now defaults to --hard?\n> >\n> > It somehow feels a bit risky for new people. I dunno.\n> >\n> \n> Option -h just prints the options and exits.\n\n>From the original patch: \n\n+ OPT_SET_INT(0, \"hard\", &reset_type,\n+ \"reset HEAD, index and working tree\", HARD),\n\nIt might be that I am misreading something, but I gather that Junio missed \nthat there is no short option here.\n\nJunio, am I right?\n\nCiao,\nDscho\n\n"},{"id":"70684","messageId":"1b46aba20803021040ofec56b9q120a654a192eec09@mail.gmail.com","threadId":"12411","inReplyTo":"20080302155541.GH2973@steel.home","subject":"Re: [PATCH] Make builtin-reset.c use parse_options.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2008-03-02T18:40:09Z","receivedAt":"2008-03-02T18:40:09Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"On Sun, Mar 2, 2008 at 4:55 PM, Alex Riesen wrote:\n>\n> \"static const struct option options[] = {\"\n\nThe other files using parse_options have only \"static\", or nothing.\n\nTo make \"options\" static, then reset_type and quiet should be\nstatic too, otherwise it cannot compile (in my system).\n\nI don't know benefits of making all of them \"static\".\nHas this been discussed previously?\n"},{"id":"70702","messageId":"20080302213843.GA13954@steel.home","threadId":"12411","inReplyTo":"1b46aba20803021040ofec56b9q120a654a192eec09@mail.gmail.com","subject":"Re: [PATCH] Make builtin-reset.c use parse_options.","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2008-03-02T21:38:43Z","receivedAt":"2008-03-02T21:38:43Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Carlos Rica, Sun, Mar 02, 2008 19:40:09 +0100:\n> On Sun, Mar 2, 2008 at 4:55 PM, Alex Riesen wrote:\n> >\n> > \"static const struct option options[] = {\"\n> \n> The other files using parse_options have only \"static\", or nothing.\n\nWell, they all miss something. Besides all nice things about static\nsyntax checking, the compiler (GCC) can optimize string constants to\nuse the same data (not that it is interesting in this particular\ncase).\n\n> To make \"options\" static, then reset_type and quiet should be\n> static too, otherwise it cannot compile (in my system).\n\nOf course. Is it a problem for user-interface level code?\n\n> I don't know benefits of making all of them \"static\".\n\nIt is initialized statically.\n\n> Has this been discussed previously?\n\nYeah. Sometime around 1972.\n"},{"id":"70783","messageId":"1b46aba20803030607v726a6ba1y49ca8524e09c6eae@mail.gmail.com","threadId":"12411","inReplyTo":"7vbq5xqetq.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Make builtin-reset.c use parse_options.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2008-03-03T14:07:57Z","receivedAt":"2008-03-03T14:07:57Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"On Sun, Mar 2, 2008 at 5:18 PM, Junio C Hamano wrote:\n> Although \"--h\" still favors \"--hard\" over \"--help\":\n>\n> $ ./git-reset --h\n> HEAD is now at c149184...\n>\n\nPierre, is there a way to give preference to --help over --hard\nwhen someone uses --h in command line?\n"},{"id":"70786","messageId":"1b46aba20803030639i73557fb3v44de6387b8fd57a7@mail.gmail.com","threadId":"12411","inReplyTo":"20080302213843.GA13954@steel.home","subject":"Re: [PATCH] Make builtin-reset.c use parse_options.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2008-03-03T14:39:05Z","receivedAt":"2008-03-03T14:39:05Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"On Sun, Mar 2, 2008 at 10:38 PM, Alex Riesen wrote:\n> Carlos Rica, Sun, Mar 02, 2008 19:40:09 +0100:\n>\n> > On Sun, Mar 2, 2008 at 4:55 PM, Alex Riesen wrote:\n> > >\n> > > \"static const struct option options[] = {\"\n> >\n> > The other files using parse_options have only \"static\", or nothing.\n>\n> Well, they all miss something. Besides all nice things about static\n> syntax checking, the compiler (GCC) can optimize string constants to\n> use the same data (not that it is interesting in this particular\n> case).\n>\n>\n> > To make \"options\" static, then reset_type and quiet should be\n> > static too, otherwise it cannot compile (in my system).\n>\n> Of course. Is it a problem for user-interface level code?\n\nThe only problem could come from reusing this code to calling\nmany times to a cmd_reset function in the future. Then, I would\nprefer not to worry about previous values in the variables by\nusing only automatic variables in the function.\n\nAnyway, \"const\" is nice, since the options struct doesn't change.\n"},{"id":"70804","messageId":"20080303170706.GA23210@artemis.madism.org","threadId":"12411","inReplyTo":"1b46aba20803030607v726a6ba1y49ca8524e09c6eae@mail.gmail.com","subject":"Re: [PATCH] Make builtin-reset.c use parse_options.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-03-03T17:07:07Z","receivedAt":"2008-03-03T17:07:07Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Mar 03, 2008 at 02:07:57PM +0000, Carlos Rica wrote:\n> On Sun, Mar 2, 2008 at 5:18 PM, Junio C Hamano wrote:\n> > Although \"--h\" still favors \"--hard\" over \"--help\":\n> >\n> > $ ./git-reset --h\n> > HEAD is now at c149184...\n> >\n> \n> Pierre, is there a way to give preference to --help over --hard\n> when someone uses --h in command line?\n\n The problem is that --help (and --help-all for the matter) are \"magic\"\narguments that parse-options is not aware of when it deals with\nabbreviations.\n\n I assume the sole way is to always test against --help (--help-all\nwhom --help is a prefix anyways) for prefixes.\n\n So basically we should replace the block in parse-options.c:\n\n if (ambiguous_option)\n ...\n if (abbrev_option)\n return get_value(p, abbrev_option, abbrev_flags);\n\n with something that basically does:\n\n\n ambiguous:\n if (ambiguous_option)\n ...\n if (abbrev_option) {\n if (clashes with --help) {\n ambiguous_option = \"help\";\n ambiguous_flags = 0;\n goto ambiguous;\n }\n return get_value....\n }\n\n\nthe \"if (clashes with --help)\" obviously has to be expansed as real\ncode, and should use an array of hardcoded values so that it's\nextensible if the need arises.\n\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"70847","messageId":"7vwsojfn2v.fsf@gitster.siamese.dyndns.org","threadId":"12411","inReplyTo":"20080303170706.GA23210@artemis.madism.org","subject":"Re: [PATCH] Make builtin-reset.c use parse_options.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-03-03T22:40:40Z","receivedAt":"2008-03-03T22:40:40Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit writes:\n\n> On Mon, Mar 03, 2008 at 02:07:57PM +0000, Carlos Rica wrote:\n>> On Sun, Mar 2, 2008 at 5:18 PM, Junio C Hamano wrote:\n>> > Although \"--h\" still favors \"--hard\" over \"--help\":\n>> >\n>> > $ ./git-reset --h\n>> > HEAD is now at c149184...\n>> >\n>> \n>> Pierre, is there a way to give preference to --help over --hard\n>> when someone uses --h in command line?\n>\n> The problem is that --help (and --help-all for the matter) are \"magic\"\n> arguments that parse-options is not aware of when it deals with\n> abbreviations.\n\nYeah, I do not know if this really matters in real life, though.\n"},{"id":"70913","messageId":"20080304094527.GB32019@artemis.madism.org","threadId":"12411","inReplyTo":"7vwsojfn2v.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] Make builtin-reset.c use parse_options.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2008-03-04T09:45:28Z","receivedAt":"2008-03-04T09:45:28Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Mon, Mar 03, 2008 at 10:40:40PM +0000, Junio C Hamano wrote:\n> Pierre Habouzit writes:\n> \n> > On Mon, Mar 03, 2008 at 02:07:57PM +0000, Carlos Rica wrote:\n> >> On Sun, Mar 2, 2008 at 5:18 PM, Junio C Hamano wrote:\n> >> > Although \"--h\" still favors \"--hard\" over \"--help\":\n> >> >\n> >> > $ ./git-reset --h\n> >> > HEAD is now at c149184...\n> >> >\n> >> \n> >> Pierre, is there a way to give preference to --help over --hard\n> >> when someone uses --h in command line?\n> >\n> > The problem is that --help (and --help-all for the matter) are \"magic\"\n> > arguments that parse-options is not aware of when it deals with\n> > abbreviations.\n> \n> Yeah, I do not know if this really matters in real life, though.\n\n I'm not sure either, that's why I didn't really bothered to _write_\nthe patch, I just mentioned how to do that if someone cares enough.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"}]}