Re: [PATCH] gc: use parse_options

James Bowes [off-list ref] writes:

+	struct option builtin_gc_options[] = {
+		OPT_BOOLEAN(0, "prune", &prune, "prune unused objects"),

I would write "unreferenced loose" instead of "unused" here...

+		OPT_BOOLEAN(0, "aggressive", &aggressive, "be more thorough (increased runtime)"),
+		OPT_BOOLEAN(0, "auto", &auto_gc, "enable auto-gc mode"),
+		OPT_END()
+	};
+
 	git_config(gc_config);
 
 	if (pack_refs < 0)
 		pack_refs = !is_bare_repository();
 
+	parse_options(argc, argv, builtin_gc_options, builtin_gc_usage, 0);
+
+	if (aggressive) {
+		append_option(argv_repack, "-f", MAX_ADD);
+		if (aggressive_window > 0) {
+			sprintf(buf, "--window=%d", aggressive_window);
+			append_option(argv_repack, buf, MAX_ADD);
 		}
 	}
-	if (i != argc)
-		usage(builtin_gc_usage);

Now, what makes the command report error when the user says:

	$ git gc unwanted parameter

Other than that, this is a good thing to have, I think.

Signed-off-by: James Bowes <redacted>
---

Junio C Hamano [off-list ref] writes:

Now, what makes the command report error when the user says:

$ git gc unwanted parameter

Ah yes. I forgot about that :)

This version of the patch errors out with extra args, and calls them
'unreferenced loose objects' rather than unused.

-James

 builtin-gc.c |   44 ++++++++++++++++++++++----------------------
 1 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/builtin-gc.c b/builtin-gc.c
index 3a2ca4f..c5bce89 100644
--- a/builtin-gc.c
+++ b/builtin-gc.c

@@ -12,11 +12,15 @@
 
 #include "builtin.h"
 #include "cache.h"
+#include "parse-options.h"
 #include "run-command.h"
 
 #define FAILED_RUN "failed to run %s"
 
-static const char builtin_gc_usage[] = "git-gc [--prune] [--aggressive]";
+static const char * const builtin_gc_usage[] = {
+	"git-gc [options]",
+	NULL
+};
 
 static int pack_refs = 1;
 static int aggressive_window = -1;

@@ -165,38 +169,34 @@ static int need_to_gc(void)
 
 int cmd_gc(int argc, const char **argv, const char *prefix)
 {
-	int i;
 	int prune = 0;
+	int aggressive = 0;
 	int auto_gc = 0;
 	char buf[80];
 
+	struct option builtin_gc_options[] = {
+		OPT_BOOLEAN(0, "prune", &prune, "prune unreferenced loose objects"),
+		OPT_BOOLEAN(0, "aggressive", &aggressive, "be more thorough (increased runtime)"),
+		OPT_BOOLEAN(0, "auto", &auto_gc, "enable auto-gc mode"),
+		OPT_END()
+	};
+
 	git_config(gc_config);
 
 	if (pack_refs < 0)
 		pack_refs = !is_bare_repository();
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-		if (!strcmp(arg, "--prune")) {
-			prune = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--aggressive")) {
-			append_option(argv_repack, "-f", MAX_ADD);
-			if (aggressive_window > 0) {
-				sprintf(buf, "--window=%d", aggressive_window);
-				append_option(argv_repack, buf, MAX_ADD);
-			}
-			continue;
-		}
-		if (!strcmp(arg, "--auto")) {
-			auto_gc = 1;
-			continue;
+	argc = parse_options(argc, argv, builtin_gc_options, builtin_gc_usage, 0);
+	if (argc > 0)
+		usage_with_options(builtin_gc_usage, builtin_gc_options);
+
+	if (aggressive) {
+		append_option(argv_repack, "-f", MAX_ADD);
+		if (aggressive_window > 0) {
+			sprintf(buf, "--window=%d", aggressive_window);
+			append_option(argv_repack, buf, MAX_ADD);
 		}
-		break;
 	}
-	if (i != argc)
-		usage(builtin_gc_usage);
 
 	if (auto_gc) {
 		/*

-- 
1.5.3.4.1481.g854da

On Fri, Nov 02, 2007 at 12:49:25AM +0000, Junio C Hamano wrote:

James Bowes [off-list ref] writes:

quoted

+	struct option builtin_gc_options[] = {
+		OPT_BOOLEAN(0, "prune", &prune, "prune unused objects"),

I would write "unreferenced loose" instead of "unused" here...

quoted

+		OPT_BOOLEAN(0, "aggressive", &aggressive, "be more thorough (increased runtime)"),
+		OPT_BOOLEAN(0, "auto", &auto_gc, "enable auto-gc mode"),
+		OPT_END()
+	};
+
 	git_config(gc_config);
 
 	if (pack_refs < 0)
 		pack_refs = !is_bare_repository();
 
+	parse_options(argc, argv, builtin_gc_options, builtin_gc_usage, 0);
+
+	if (aggressive) {
+		append_option(argv_repack, "-f", MAX_ADD);
+		if (aggressive_window > 0) {
+			sprintf(buf, "--window=%d", aggressive_window);
+			append_option(argv_repack, buf, MAX_ADD);
 		}
 	}
-	if (i != argc)
-		usage(builtin_gc_usage);

Now, what makes the command report error when the user says:

	$ git gc unwanted parameter

the commands works fine, because no additionnal checks were made. To
"fix" this, that should be done:

argc = parse_options(argc, argv, builtin_gc_options, builtin_gc_usage, 0);
if (argc)
    usage_with_options(......);

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

Junio C Hamano wrote:

James Bowes [off-list ref] writes:

quoted

+	struct option builtin_gc_options[] = {
+		OPT_BOOLEAN(0, "prune", &prune, "prune unused objects"),

I would write "unreferenced loose" instead of "unused" here...

It is not just "loose" objects here, but also unreferenced objects in packs,
since the "-a" option to repack is now only used when --prune is specified.
Without --prune, "-A" is supplied to repack instead.

So maybe the message should just be "prune unreferenced objects"

-brandon