{"thread":{"id":"10576","subject":"[PATCH] gc: use parse_options","startedAt":"2007-11-02T00:28:57Z","lastAt":"2007-11-06T00:37:08Z","messageCount":6,"participants":["James Bowes","Junio C Hamano","Pierre Habouzit","Brandon Casey"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"57906","messageId":"20071102002856.GB3282@crux.yyz.redhat.com","threadId":"10576","inReplyTo":null,"subject":"[PATCH] gc: use parse_options","fromName":"James Bowes","fromEmail":"jbowes@dangerouslyinc.com","sentAt":"2007-11-02T00:28:57Z","receivedAt":"2007-11-02T00:28:57Z","isPatch":true,"sender":{"key":"jbowes@dangerouslyinc.com","avatar":"https://gravatar.com/avatar/a2fe98c66b2b47a9fa9d2ba92ff949d54c3208b1f8acc2e745b4b84ae3c4483a?d=mp&s=160"},"body":"Signed-off-by: James Bowes <jbowes@dangerouslyinc.com>\n---\n builtin-gc.c |   42 ++++++++++++++++++++----------------------\n 1 files changed, 20 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin-gc.c b/builtin-gc.c\nindex 3a2ca4f..7bb873c 100644\n--- a/builtin-gc.c\n+++ b/builtin-gc.c\n@@ -12,11 +12,15 @@\n \n #include \"builtin.h\"\n #include \"cache.h\"\n+#include \"parse-options.h\"\n #include \"run-command.h\"\n \n #define FAILED_RUN \"failed to run %s\"\n \n-static const char builtin_gc_usage[] = \"git-gc [--prune] [--aggressive]\";\n+static const char * const builtin_gc_usage[] = {\n+\t\"git-gc [options]\",\n+\tNULL\n+};\n \n static int pack_refs = 1;\n static int aggressive_window = -1;\n@@ -165,38 +169,32 @@ static int need_to_gc(void)\n \n int cmd_gc(int argc, const char **argv, const char *prefix)\n {\n-\tint i;\n \tint prune = 0;\n+\tint aggressive = 0;\n \tint auto_gc = 0;\n \tchar buf[80];\n \n+\tstruct option builtin_gc_options[] = {\n+\t\tOPT_BOOLEAN(0, \"prune\", &prune, \"prune unused objects\"),\n+\t\tOPT_BOOLEAN(0, \"aggressive\", &aggressive, \"be more thorough (increased runtime)\"),\n+\t\tOPT_BOOLEAN(0, \"auto\", &auto_gc, \"enable auto-gc mode\"),\n+\t\tOPT_END()\n+\t};\n+\n \tgit_config(gc_config);\n \n \tif (pack_refs < 0)\n \t\tpack_refs = !is_bare_repository();\n \n-\tfor (i = 1; i < argc; i++) {\n-\t\tconst char *arg = argv[i];\n-\t\tif (!strcmp(arg, \"--prune\")) {\n-\t\t\tprune = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"--aggressive\")) {\n-\t\t\tappend_option(argv_repack, \"-f\", MAX_ADD);\n-\t\t\tif (aggressive_window > 0) {\n-\t\t\t\tsprintf(buf, \"--window=%d\", aggressive_window);\n-\t\t\t\tappend_option(argv_repack, buf, MAX_ADD);\n-\t\t\t}\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"--auto\")) {\n-\t\t\tauto_gc = 1;\n-\t\t\tcontinue;\n+\tparse_options(argc, argv, builtin_gc_options, builtin_gc_usage, 0);\n+\n+\tif (aggressive) {\n+\t\tappend_option(argv_repack, \"-f\", MAX_ADD);\n+\t\tif (aggressive_window > 0) {\n+\t\t\tsprintf(buf, \"--window=%d\", aggressive_window);\n+\t\t\tappend_option(argv_repack, buf, MAX_ADD);\n \t\t}\n-\t\tbreak;\n \t}\n-\tif (i != argc)\n-\t\tusage(builtin_gc_usage);\n \n \tif (auto_gc) {\n \t\t/*\n-- \n1.5.3.4.1481.g854da\n"},{"id":"57909","messageId":"7vhck579pm.fsf@gitster.siamese.dyndns.org","threadId":"10576","inReplyTo":"20071102002856.GB3282@crux.yyz.redhat.com","subject":"Re: [PATCH] gc: use parse_options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-02T00:49:25Z","receivedAt":"2007-11-02T00:49:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"James Bowes <jbowes@dangerouslyinc.com> writes:\n\n> +\tstruct option builtin_gc_options[] = {\n> +\t\tOPT_BOOLEAN(0, \"prune\", &prune, \"prune unused objects\"),\n\nI would write \"unreferenced loose\" instead of \"unused\" here...\n\n> +\t\tOPT_BOOLEAN(0, \"aggressive\", &aggressive, \"be more thorough (increased runtime)\"),\n> +\t\tOPT_BOOLEAN(0, \"auto\", &auto_gc, \"enable auto-gc mode\"),\n> +\t\tOPT_END()\n> +\t};\n> +\n>  \tgit_config(gc_config);\n>  \n>  \tif (pack_refs < 0)\n>  \t\tpack_refs = !is_bare_repository();\n>  \n> +\tparse_options(argc, argv, builtin_gc_options, builtin_gc_usage, 0);\n> +\n> +\tif (aggressive) {\n> +\t\tappend_option(argv_repack, \"-f\", MAX_ADD);\n> +\t\tif (aggressive_window > 0) {\n> +\t\t\tsprintf(buf, \"--window=%d\", aggressive_window);\n> +\t\t\tappend_option(argv_repack, buf, MAX_ADD);\n>  \t\t}\n>  \t}\n> -\tif (i != argc)\n> -\t\tusage(builtin_gc_usage);\n\nNow, what makes the command report error when the user says:\n\n\t$ git gc unwanted parameter\n\nOther than that, this is a good thing to have, I think.\n"},{"id":"57911","messageId":"20071102010226.GC3282@crux.yyz.redhat.com","threadId":"10576","inReplyTo":"7vhck579pm.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] gc: use parse_options","fromName":"James Bowes","fromEmail":"jbowes@dangerouslyinc.com","sentAt":"2007-11-02T01:02:27Z","receivedAt":"2007-11-02T01:02:27Z","isPatch":true,"sender":{"key":"jbowes@dangerouslyinc.com","avatar":"https://gravatar.com/avatar/a2fe98c66b2b47a9fa9d2ba92ff949d54c3208b1f8acc2e745b4b84ae3c4483a?d=mp&s=160"},"body":"Signed-off-by: James Bowes <jbowes@dangerouslyinc.com>\n---\n\nJunio C Hamano <gitster@pobox.com> writes:\n> Now, what makes the command report error when the user says:\n>\n>\t$ git gc unwanted parameter\n\nAh yes. I forgot about that :)\n\nThis version of the patch errors out with extra args, and calls them\n'unreferenced loose objects' rather than unused.\n\n-James\n\n builtin-gc.c |   44 ++++++++++++++++++++++----------------------\n 1 files changed, 22 insertions(+), 22 deletions(-)\n\ndiff --git a/builtin-gc.c b/builtin-gc.c\nindex 3a2ca4f..c5bce89 100644\n--- a/builtin-gc.c\n+++ b/builtin-gc.c\n@@ -12,11 +12,15 @@\n \n #include \"builtin.h\"\n #include \"cache.h\"\n+#include \"parse-options.h\"\n #include \"run-command.h\"\n \n #define FAILED_RUN \"failed to run %s\"\n \n-static const char builtin_gc_usage[] = \"git-gc [--prune] [--aggressive]\";\n+static const char * const builtin_gc_usage[] = {\n+\t\"git-gc [options]\",\n+\tNULL\n+};\n \n static int pack_refs = 1;\n static int aggressive_window = -1;\n@@ -165,38 +169,34 @@ static int need_to_gc(void)\n \n int cmd_gc(int argc, const char **argv, const char *prefix)\n {\n-\tint i;\n \tint prune = 0;\n+\tint aggressive = 0;\n \tint auto_gc = 0;\n \tchar buf[80];\n \n+\tstruct option builtin_gc_options[] = {\n+\t\tOPT_BOOLEAN(0, \"prune\", &prune, \"prune unreferenced loose objects\"),\n+\t\tOPT_BOOLEAN(0, \"aggressive\", &aggressive, \"be more thorough (increased runtime)\"),\n+\t\tOPT_BOOLEAN(0, \"auto\", &auto_gc, \"enable auto-gc mode\"),\n+\t\tOPT_END()\n+\t};\n+\n \tgit_config(gc_config);\n \n \tif (pack_refs < 0)\n \t\tpack_refs = !is_bare_repository();\n \n-\tfor (i = 1; i < argc; i++) {\n-\t\tconst char *arg = argv[i];\n-\t\tif (!strcmp(arg, \"--prune\")) {\n-\t\t\tprune = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"--aggressive\")) {\n-\t\t\tappend_option(argv_repack, \"-f\", MAX_ADD);\n-\t\t\tif (aggressive_window > 0) {\n-\t\t\t\tsprintf(buf, \"--window=%d\", aggressive_window);\n-\t\t\t\tappend_option(argv_repack, buf, MAX_ADD);\n-\t\t\t}\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"--auto\")) {\n-\t\t\tauto_gc = 1;\n-\t\t\tcontinue;\n+\targc = parse_options(argc, argv, builtin_gc_options, builtin_gc_usage, 0);\n+\tif (argc > 0)\n+\t\tusage_with_options(builtin_gc_usage, builtin_gc_options);\n+\n+\tif (aggressive) {\n+\t\tappend_option(argv_repack, \"-f\", MAX_ADD);\n+\t\tif (aggressive_window > 0) {\n+\t\t\tsprintf(buf, \"--window=%d\", aggressive_window);\n+\t\t\tappend_option(argv_repack, buf, MAX_ADD);\n \t\t}\n-\t\tbreak;\n \t}\n-\tif (i != argc)\n-\t\tusage(builtin_gc_usage);\n \n \tif (auto_gc) {\n \t\t/*\n-- \n1.5.3.4.1481.g854da\n"},{"id":"57937","messageId":"20071102083247.GB20200@artemis.corp","threadId":"10576","inReplyTo":"7vhck579pm.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] gc: use parse_options","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-11-02T08:32:47Z","receivedAt":"2007-11-02T08:32:47Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Fri, Nov 02, 2007 at 12:49:25AM +0000, Junio C Hamano wrote:\n> James Bowes <jbowes@dangerouslyinc.com> writes:\n> \n> > +\tstruct option builtin_gc_options[] = {\n> > +\t\tOPT_BOOLEAN(0, \"prune\", &prune, \"prune unused objects\"),\n> \n> I would write \"unreferenced loose\" instead of \"unused\" here...\n> \n> > +\t\tOPT_BOOLEAN(0, \"aggressive\", &aggressive, \"be more thorough (increased runtime)\"),\n> > +\t\tOPT_BOOLEAN(0, \"auto\", &auto_gc, \"enable auto-gc mode\"),\n> > +\t\tOPT_END()\n> > +\t};\n> > +\n> >  \tgit_config(gc_config);\n> >  \n> >  \tif (pack_refs < 0)\n> >  \t\tpack_refs = !is_bare_repository();\n> >  \n> > +\tparse_options(argc, argv, builtin_gc_options, builtin_gc_usage, 0);\n> > +\n> > +\tif (aggressive) {\n> > +\t\tappend_option(argv_repack, \"-f\", MAX_ADD);\n> > +\t\tif (aggressive_window > 0) {\n> > +\t\t\tsprintf(buf, \"--window=%d\", aggressive_window);\n> > +\t\t\tappend_option(argv_repack, buf, MAX_ADD);\n> >  \t\t}\n> >  \t}\n> > -\tif (i != argc)\n> > -\t\tusage(builtin_gc_usage);\n> \n> Now, what makes the command report error when the user says:\n> \n> \t$ git gc unwanted parameter\n\nthe commands works fine, because no additionnal checks were made. To\n\"fix\" this, that should be done:\n\nargc = parse_options(argc, argv, builtin_gc_options, builtin_gc_usage, 0);\nif (argc)\n    usage_with_options(......);\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"58447","messageId":"472FA26E.4060706@nrlssc.navy.mil","threadId":"10576","inReplyTo":"7vhck579pm.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] gc: use parse_options","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2007-11-05T23:08:30Z","receivedAt":"2007-11-05T23:08:30Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Junio C Hamano wrote:\n> James Bowes <jbowes@dangerouslyinc.com> writes:\n> \n>> +\tstruct option builtin_gc_options[] = {\n>> +\t\tOPT_BOOLEAN(0, \"prune\", &prune, \"prune unused objects\"),\n> \n> I would write \"unreferenced loose\" instead of \"unused\" here...\n\nIt is not just \"loose\" objects here, but also unreferenced objects in packs,\nsince the \"-a\" option to repack is now only used when --prune is specified.\nWithout --prune, \"-A\" is supplied to repack instead.\n\nSo maybe the message should just be \"prune unreferenced objects\"\n\n-brandon\n"},{"id":"58465","messageId":"7vr6j4ky4r.fsf@gitster.siamese.dyndns.org","threadId":"10576","inReplyTo":"472FA26E.4060706@nrlssc.navy.mil","subject":"Re: [PATCH] gc: use parse_options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-06T00:37:08Z","receivedAt":"2007-11-06T00:37:08Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Brandon Casey <casey@nrlssc.navy.mil> writes:\n\n> Junio C Hamano wrote:\n>> James Bowes <jbowes@dangerouslyinc.com> writes:\n>> \n>>> +\tstruct option builtin_gc_options[] = {\n>>> +\t\tOPT_BOOLEAN(0, \"prune\", &prune, \"prune unused objects\"),\n>> \n>> I would write \"unreferenced loose\" instead of \"unused\" here...\n>\n> It is not just \"loose\" objects here, but also unreferenced objects in packs,\n> since the \"-a\" option to repack is now only used when --prune is specified.\n> Without --prune, \"-A\" is supplied to repack instead.\n>\n> So maybe the message should just be \"prune unreferenced objects\"\n\nFair enough, will do.\n"}]}