threads / patch / 14641

patch, 9 partsbuiltin-checkout-index.c: use parse_options()

Subject: [PATCH 7/9] builtin-checkout-index.c: use parse_options()

## tl;dr

31 messages between Jul 23, 2008 and Jul 28, 2008. Diffs are folded; open one to read it.

replies: 30people: 5as markdown or json

Michele Ballabio· Jul 23, 2008, 21:42 UTC · lore

[PATCH 0/9] Extend use of parse_options()

These patches spread the use of parse_options() to some more commands. I tried to come up with a short description for every option, but I'm not sure I fully succeeded. In particular, the option "sparse" in builtin-rev-list.c has no description.

Michele Ballabio (9):
  builtin-verify-tag.c: use parse_options()
  builtin-write-tree.c: use parse_options()
  builtin-prune-packed.c: use parse_options()
  builtin-ls-tree.c: use parse_options()
  builtin-rev-list.c: use parse_options()
  builtin-init-db.c: use parse_options()
  builtin-checkout-index.c: use parse_options()
  builtin-fetch-pack.c: use parse_options()
  builtin-mailinfo.c: use parse_options()
 builtin-checkout-index.c |  146 +++++++++++++++++++++++++---------------------
 builtin-fetch-pack.c     |  144 ++++++++++++++++++++++++++++-----------------
 builtin-init-db.c        |   56 +++++++++++-------
 builtin-ls-tree.c        |   92 +++++++++++------------------
 builtin-mailinfo.c       |   39 +++++++------
 builtin-prune-packed.c   |   38 ++++++------
 builtin-rev-list.c       |  132 ++++++++++++++++++++---------------------
 builtin-verify-tag.c     |   25 +++++---
 builtin-write-tree.c     |   31 +++++-----
 9 files changed, 376 insertions(+), 327 deletions(-)
Michele Ballabio· Jul 23, 2008, 21:42 UTC · re: Michele Ballabio · lore

[PATCH 1/9] builtin-verify-tag.c: use parse_options()

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
 builtin-verify-tag.c |   25 +++++++++++++++----------
 1 files changed, 15 insertions(+), 10 deletions(-)
Show changes to builtin-verify-tag.c +15 −10
diff --git a/builtin-verify-tag.c b/builtin-verify-tag.c
index 7d837f0..590038b 100644
--- a/builtin-verify-tag.c
+++ b/builtin-verify-tag.c
@@ -9,10 +9,13 @@
 #include "builtin.h"
 #include "tag.h"
 #include "run-command.h"
+#include "parse-options.h"
 #include <signal.h>
 
-static const char builtin_verify_tag_usage[] =
-		"git verify-tag [-v|--verbose] <tag>...";
+static const char * const builtin_verify_tag_usage[] = {
+	"git verify-tag [-v|--verbose] <tag>...",
+	NULL
+};
 
 #define PGP_SIGNATURE "-----BEGIN PGP SIGNATURE-----"
 
@@ -88,23 +91,25 @@ static int verify_tag(const char *name, int verbose)
 
 int cmd_verify_tag(int argc, const char **argv, const char *prefix)
 {
-	int i = 1, verbose = 0, had_error = 0;
+	int verbose = 0, had_error = 0;
 
 	git_config(git_default_config, NULL);
 
+	const struct option options[] = {
+		OPT__VERBOSE(&verbose),
+		OPT_END()
+	};
+
 	if (argc == 1)
-		usage(builtin_verify_tag_usage);
+		usage_with_options(builtin_verify_tag_usage, options);
 
-	if (!strcmp(argv[i], "-v") || !strcmp(argv[i], "--verbose")) {
-		verbose = 1;
-		i++;
-	}
+	argc = parse_options(argc, argv, options, builtin_verify_tag_usage, 0);
 
 	/* sometimes the program was terminated because this signal
 	 * was received in the process of writing the gpg input: */
 	signal(SIGPIPE, SIG_IGN);
-	while (i < argc)
-		if (verify_tag(argv[i++], verbose))
+	while (argc-- > 0)
+		if (verify_tag(*argv++, verbose))
 			had_error = 1;
 	return had_error;
 }
-- 
1.5.6.3
Olivier Marin· Jul 24, 2008, 16:59 UTC · re: Michele Ballabio · lore

Re: [PATCH 1/9] builtin-verify-tag.c: use parse_options()

Michele Ballabio a écrit :
> 
>  	if (argc == 1)
> -		usage(builtin_verify_tag_usage);
> +		usage_with_options(builtin_verify_tag_usage, options);

It seems this is broken since the C rewrite: "git verify-tag -v" just do nothing instead of printing usage message.

Moving the if() after parse_options() call with s/argc == 1/argc == 0/ should do the trick.

Show 5 quoted lines
> -	if (!strcmp(argv[i], "-v") || !strcmp(argv[i], "--verbose")) {
> -		verbose = 1;
> -		i++;
> -	}
> +	argc = parse_options(argc, argv, options, builtin_verify_tag_usage, 0);
Olivier.
Johannes Schindelin· Jul 24, 2008, 17:08 UTC · re: Olivier Marin · lore

Re: [PATCH 1/9] builtin-verify-tag.c: use parse_options()

Hi,
On Thu, 24 Jul 2008, Olivier Marin wrote:
Show 11 quoted lines
> Michele Ballabio a écrit :
> > 
> >  	if (argc == 1)
> > -		usage(builtin_verify_tag_usage);
> > +		usage_with_options(builtin_verify_tag_usage, options);
> 
> It seems this is broken since the C rewrite: "git verify-tag -v" just do
> nothing instead of printing usage message.
> 
> Moving the if() after parse_options() call with s/argc == 1/argc == 0/
> should do the trick.

That would be a bugfix. As such, it belongs into a different commit. Care to provide a patch?

Show 5 quoted lines
> > -	if (!strcmp(argv[i], "-v") || !strcmp(argv[i], "--verbose")) {
> > -		verbose = 1;
> > -		i++;
> > -	}
> > +	argc = parse_options(argc, argv, options, builtin_verify_tag_usage, 0);
Why did you quote this?

Ciao, Dscho

Olivier Marin· Jul 25, 2008, 15:20 UTC · re: Johannes Schindelin · lore

Re: [PATCH 1/9] builtin-verify-tag.c: use parse_options()

Johannes Schindelin a écrit :
> 
> That would be a bugfix.  As such, it belongs into a different commit.  

I thought, for that kind of trivial bug that probably never hit anyone, a line in the commit message was enough.

> Care to provide a patch?
OK, will do.
Olivier.
Johannes Schindelin· Jul 26, 2008, 00:53 UTC · re: Olivier Marin · lore

Re: [PATCH 1/9] builtin-verify-tag.c: use parse_options()

Hi,
On Fri, 25 Jul 2008, Olivier Marin wrote:
Show 6 quoted lines
> Johannes Schindelin a écrit :
> > 
> > That would be a bugfix.  As such, it belongs into a different commit.  
> 
> I thought, for that kind of trivial bug that probably never hit anyone, 
> a line in the commit message was enough.
So bisectability goes down the gutter?
Olivier Marin· Jul 28, 2008, 10:48 UTC · re: Olivier Marin · lore

[PATCH] builtin-verify-tag: fix -v option parsing

From: Olivier Marin <dkr@freesurf.fr>

Since the C rewrite, "git verify-tag -v" just does nothing instead of printing the usage message with an error. This patch fix the regression.

Signed-off-by: Olivier Marin <dkr@freesurf.fr>
---
 builtin-verify-tag.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)
Show changes to builtin-verify-tag.c +5 −4
diff --git a/builtin-verify-tag.c b/builtin-verify-tag.c
index 7d837f0..729a159 100644
--- a/builtin-verify-tag.c
+++ b/builtin-verify-tag.c
@@ -92,14 +92,15 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
 
 	git_config(git_default_config, NULL);
 
-	if (argc == 1)
-		usage(builtin_verify_tag_usage);
-
-	if (!strcmp(argv[i], "-v") || !strcmp(argv[i], "--verbose")) {
+	if (argc > 1 &&
+	    (!strcmp(argv[i], "-v") || !strcmp(argv[i], "--verbose"))) {
 		verbose = 1;
 		i++;
 	}
 
+	if (argc <= i)
+		usage(builtin_verify_tag_usage);
+
 	/* sometimes the program was terminated because this signal
 	 * was received in the process of writing the gpg input: */
 	signal(SIGPIPE, SIG_IGN);
-- 
1.6.0.rc0.79.gb0320
Johannes Schindelin· Jul 28, 2008, 11:06 UTC · re: Olivier Marin · lore

Re: [PATCH] builtin-verify-tag: fix -v option parsing

Hi,
On Mon, 28 Jul 2008, Olivier Marin wrote:
> From: Olivier Marin <dkr@freesurf.fr>
> 
> Since the C rewrite, "git verify-tag -v" just does nothing instead of 
> printing the usage message with an error. This patch fix the regression.

Maybe a better solution would be to convert (trivially) to parse-options...

Ciao, Dscho

Olivier Marin· Jul 28, 2008, 11:42 UTC · re: Johannes Schindelin · lore

Re: [PATCH] builtin-verify-tag: fix -v option parsing

Johannes Schindelin a écrit :
Show 6 quoted lines
>>
>> Since the C rewrite, "git verify-tag -v" just does nothing instead of 
>> printing the usage message with an error. This patch fix the regression.
> 
> Maybe a better solution would be to convert (trivially) to 
> parse-options...
I am very puzzled.

You first asked me to do a separate commit with just the fix and now you seem to want the fix with the conversion...

What do you mean by "trivially"?
Olivier.
Johannes Schindelin· Jul 28, 2008, 12:10 UTC · re: Olivier Marin · lore

Re: [PATCH] builtin-verify-tag: fix -v option parsing

Hi,
On Mon, 28 Jul 2008, Olivier Marin wrote:
Show 13 quoted lines
> Johannes Schindelin a écrit :
> >>
> >> Since the C rewrite, "git verify-tag -v" just does nothing instead of 
> >> printing the usage message with an error. This patch fix the 
> >> regression.
> > 
> > Maybe a better solution would be to convert (trivially) to 
> > parse-options...
> 
> I am very puzzled.
> 
> You first asked me to do a separate commit with just the fix and now you 
> seem to want the fix with the conversion...

Sorry. It was not obvious to myself when I asked for a separate patch that the fix would fall out of the conversion to parse-options.

My fault, Dscho

Michele Ballabio· Jul 23, 2008, 21:42 UTC · re: Michele Ballabio · lore

[PATCH 2/9] builtin-write-tree.c: use parse_options()

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
 builtin-write-tree.c |   31 +++++++++++++++++--------------
 1 files changed, 17 insertions(+), 14 deletions(-)
Show changes to builtin-write-tree.c +17 −14
diff --git a/builtin-write-tree.c b/builtin-write-tree.c
index 52a3c01..25f3d8a 100644
--- a/builtin-write-tree.c
+++ b/builtin-write-tree.c
@@ -7,9 +7,12 @@
 #include "cache.h"
 #include "tree.h"
 #include "cache-tree.h"
+#include "parse-options.h"
 
-static const char write_tree_usage[] =
-"git write-tree [--missing-ok] [--prefix=<prefix>/]";
+static const char * const write_tree_usage[] = {
+	"git write-tree [--missing-ok] [--prefix=<directory>/]",
+	NULL
+};
 
 int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
 {
@@ -19,19 +22,19 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
 	const char *me = "git-write-tree";
 
 	git_config(git_default_config, NULL);
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--missing-ok"))
-			missing_ok = 1;
-		else if (!prefixcmp(arg, "--prefix="))
-			prefix = arg + 9;
-		else
-			usage(write_tree_usage);
-		argc--; argv++;
-	}
 
-	if (argc > 2)
-		die("too many options");
+	const struct option options[] = {
+		OPT_BOOLEAN(0, "missing-ok", &missing_ok,
+			    "disable existence check"),
+		OPT_STRING(0, "prefix", &prefix, "directory",
+			   "write a tree object for <directory>"),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, options, write_tree_usage, 0);
+
+	if (argc > 0)
+		usage_with_options(write_tree_usage, options);
 
 	ret = write_cache_as_tree(sha1, missing_ok, prefix);
 	switch (ret) {
-- 
1.5.6.3
Michele Ballabio· Jul 23, 2008, 21:42 UTC · re: Michele Ballabio · lore

[PATCH 3/9] builtin-prune-packed.c: use parse_options()

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
 builtin-prune-packed.c |   38 ++++++++++++++++++--------------------
 1 files changed, 18 insertions(+), 20 deletions(-)
Show changes to builtin-prune-packed.c +18 −20
diff --git a/builtin-prune-packed.c b/builtin-prune-packed.c
index 10cb8df..5866871 100644
--- a/builtin-prune-packed.c
+++ b/builtin-prune-packed.c
@@ -1,12 +1,15 @@
 #include "builtin.h"
 #include "cache.h"
 #include "progress.h"
+#include "parse-options.h"
 
-static const char prune_packed_usage[] =
-"git prune-packed [-n] [-q]";
+static const char * const prune_packed_usage[] = {
+	"git prune-packed [-n] [-q]",
+	NULL
+};
 
 #define DRY_RUN 01
-#define VERBOSE 02
+#define QUIET 02
 
 static struct progress *progress;
 
@@ -43,7 +46,7 @@ void prune_packed_objects(int opts)
 	const char *dir = get_object_directory();
 	int len = strlen(dir);
 
-	if (opts == VERBOSE)
+	if (!opts)
 		progress = start_progress_delay("Removing duplicate objects",
 			256, 95, 2);
 
@@ -67,24 +70,19 @@ void prune_packed_objects(int opts)
 
 int cmd_prune_packed(int argc, const char **argv, const char *prefix)
 {
-	int i;
-	int opts = VERBOSE;
+	int opts = 0;
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
+	const struct option options[] = {
+		OPT_BIT('n', "dry-run", &opts, "dry run", DRY_RUN),
+		OPT_BIT('q', "quiet", &opts, "be quiet", QUIET),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, options, prune_packed_usage, 0);
+
+	if (argc > 0)
+		usage_with_options(prune_packed_usage, options);
 
-		if (*arg == '-') {
-			if (!strcmp(arg, "-n"))
-				opts |= DRY_RUN;
-			else if (!strcmp(arg, "-q"))
-				opts &= ~VERBOSE;
-			else
-				usage(prune_packed_usage);
-			continue;
-		}
-		/* Handle arguments here .. */
-		usage(prune_packed_usage);
-	}
 	prune_packed_objects(opts);
 	return 0;
 }
-- 
1.5.6.3
Michele Ballabio· Jul 23, 2008, 21:42 UTC · re: Michele Ballabio · lore

[PATCH 4/9] builtin-ls-tree.c: use parse_options()

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
 builtin-ls-tree.c |   92 +++++++++++++++++++++--------------------------------
 1 files changed, 36 insertions(+), 56 deletions(-)
Show changes to builtin-ls-tree.c +36 −56
diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c
index d25767a..a0b17aa 100644
--- a/builtin-ls-tree.c
+++ b/builtin-ls-tree.c
@@ -9,6 +9,7 @@
 #include "commit.h"
 #include "quote.h"
 #include "builtin.h"
+#include "parse-options.h"
 
 static int line_termination = '\n';
 #define LS_RECURSIVE 1
@@ -22,8 +23,10 @@ static const char **pathspec;
 static int chomp_prefix;
 static const char *ls_tree_prefix;
 
-static const char ls_tree_usage[] =
-	"git ls-tree [-d] [-r] [-t] [-l] [-z] [--name-only] [--name-status] [--full-name] [--abbrev[=<n>]] <tree-ish> [path...]";
+static const char * const ls_tree_usage[] = {
+	"git ls-tree [options] <tree-ish> [path...]",
+	NULL
+};
 
 static int show_recursive(const char *base, int baselen, const char *pathname)
 {
@@ -122,70 +125,47 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 	unsigned char sha1[20];
 	struct tree *tree;
 
+	const struct option options[] = {
+		OPT_SET_INT('z', NULL, &line_termination,
+			    "\\0 line termination on output", 0),
+		OPT_BIT('r', NULL, &ls_options,
+			"recurse into sub-trees", LS_RECURSIVE),
+		OPT_BIT('d', NULL, &ls_options,
+			"show only the tree, not its children",
+			LS_TREE_ONLY),
+		OPT_BIT('t', NULL, &ls_options,
+			"show tree entries", LS_SHOW_TREES),
+		OPT_BIT('l', "long", &ls_options,
+			"show object size of blob (file) entries",
+			LS_SHOW_SIZE),
+		OPT_BIT(0, "name-only", &ls_options,
+			"list only filenames", LS_NAME_ONLY),
+		OPT_BIT(0, "name-status", &ls_options,
+			"same as --name-only", LS_NAME_ONLY),
+		OPT_SET_INT(0, "full-name", &chomp_prefix,
+			    "show the full path name", 0),
+		OPT__ABBREV(&abbrev),
+		OPT_END()
+	};
+
 	git_config(git_default_config, NULL);
 	ls_tree_prefix = prefix;
 	if (prefix && *prefix)
 		chomp_prefix = strlen(prefix);
-	while (1 < argc && argv[1][0] == '-') {
-		switch (argv[1][1]) {
-		case 'z':
-			line_termination = 0;
-			break;
-		case 'r':
-			ls_options |= LS_RECURSIVE;
-			break;
-		case 'd':
-			ls_options |= LS_TREE_ONLY;
-			break;
-		case 't':
-			ls_options |= LS_SHOW_TREES;
-			break;
-		case 'l':
-			ls_options |= LS_SHOW_SIZE;
-			break;
-		case '-':
-			if (!strcmp(argv[1]+2, "name-only") ||
-			    !strcmp(argv[1]+2, "name-status")) {
-				ls_options |= LS_NAME_ONLY;
-				break;
-			}
-			if (!strcmp(argv[1]+2, "long")) {
-				ls_options |= LS_SHOW_SIZE;
-				break;
-			}
-			if (!strcmp(argv[1]+2, "full-name")) {
-				chomp_prefix = 0;
-				break;
-			}
-			if (!prefixcmp(argv[1]+2, "abbrev=")) {
-				abbrev = strtoul(argv[1]+9, NULL, 10);
-				if (abbrev && abbrev < MINIMUM_ABBREV)
-					abbrev = MINIMUM_ABBREV;
-				else if (abbrev > 40)
-					abbrev = 40;
-				break;
-			}
-			if (!strcmp(argv[1]+2, "abbrev")) {
-				abbrev = DEFAULT_ABBREV;
-				break;
-			}
-			/* otherwise fallthru */
-		default:
-			usage(ls_tree_usage);
-		}
-		argc--; argv++;
-	}
+
+	argc = parse_options(argc, argv, options, ls_tree_usage, 0);
+
 	/* -d -r should imply -t, but -d by itself should not have to. */
 	if ( (LS_TREE_ONLY|LS_RECURSIVE) ==
 	    ((LS_TREE_ONLY|LS_RECURSIVE) & ls_options))
 		ls_options |= LS_SHOW_TREES;
 
-	if (argc < 2)
-		usage(ls_tree_usage);
-	if (get_sha1(argv[1], sha1))
-		die("Not a valid object name %s", argv[1]);
+	if (argc < 1)
+		usage_with_options(ls_tree_usage, options);
+	if (get_sha1(argv[0], sha1))
+		die("Not a valid object name %s", argv[0]);
 
-	pathspec = get_pathspec(prefix, argv + 2);
+	pathspec = get_pathspec(prefix, argv + 1);
 	tree = parse_tree_indirect(sha1);
 	if (!tree)
 		die("not a tree object");
-- 
1.5.6.3
Michele Ballabio· Jul 23, 2008, 21:42 UTC · re: Michele Ballabio · lore

[PATCH 5/9] builtin-rev-list.c: use parse_options()

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
 builtin-rev-list.c |  132 +++++++++++++++++++++++++--------------------------
 1 files changed, 65 insertions(+), 67 deletions(-)
Show changes to builtin-rev-list.c +65 −67
diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index 893762c..9200b20 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c
@@ -11,44 +11,16 @@
 #include "builtin.h"
 #include "log-tree.h"
 #include "graph.h"
+#include "parse-options.h"
 
 /* bits #0-15 in revision.h */
 
 #define COUNTED		(1u<<16)
 
-static const char rev_list_usage[] =
-"git rev-list [OPTION] <commit-id>... [ -- paths... ]\n"
-"  limiting output:\n"
-"    --max-count=nr\n"
-"    --max-age=epoch\n"
-"    --min-age=epoch\n"
-"    --sparse\n"
-"    --no-merges\n"
-"    --remove-empty\n"
-"    --all\n"
-"    --branches\n"
-"    --tags\n"
-"    --remotes\n"
-"    --stdin\n"
-"    --quiet\n"
-"  ordering output:\n"
-"    --topo-order\n"
-"    --date-order\n"
-"    --reverse\n"
-"  formatting output:\n"
-"    --parents\n"
-"    --children\n"
-"    --objects | --objects-edge\n"
-"    --unpacked\n"
-"    --header | --pretty\n"
-"    --abbrev=nr | --no-abbrev\n"
-"    --abbrev-commit\n"
-"    --left-right\n"
-"  special purpose:\n"
-"    --bisect\n"
-"    --bisect-vars\n"
-"    --bisect-all"
-;
+static const char * const rev_list_usage[] = {
+	"git rev-list [OPTION] <commit-id>... [ -- paths... ]",
+	NULL
+};
 
 static struct rev_info revs;
 
@@ -575,15 +547,65 @@ static struct commit_list *find_bisection(struct commit_list *list,
 	return best;
 }
 
+static int parse_header_cb(const struct option *opt, const char *arg, int unset)
+{
+	struct rev_info *t_revs = opt->value;
+	t_revs->verbose_header = unset ? 0 : 1;
+	return 0;
+}
+
 int cmd_rev_list(int argc, const char **argv, const char *prefix)
 {
 	struct commit_list *list;
-	int i;
 	int read_from_stdin = 0;
 	int bisect_show_vars = 0;
 	int bisect_find_all = 0;
 	int quiet = 0;
 
+	const struct option options[] = {
+		OPT_GROUP("limiting output:"),
+		OPT_ARGUMENT("max-count=nr", "limit number of commits output"),
+		OPT_ARGUMENT("max-age=epoch", "limit commits output by time"),
+		OPT_ARGUMENT("min-age=epoch", "limit commits output by time"),
+		OPT_ARGUMENT("sparse", ""),
+		OPT_ARGUMENT("no-merges", "do not print merges"),
+		OPT_ARGUMENT("remove-empty", "stop when a given path disappears from the tree"),
+		OPT_ARGUMENT("all", "all refs"),
+		OPT_ARGUMENT("branches", "show local branches"),
+		OPT_ARGUMENT("tags", "show tags"),
+		OPT_ARGUMENT("remotes", "show remote-tracking branches"),
+		OPT_BOOLEAN(0, "stdin", &read_from_stdin,
+			    "read commits also from command line"),
+		OPT__QUIET(&quiet),
+		OPT_GROUP("ordering output:"),
+		OPT_ARGUMENT("topo-order", "show commits in topological order"),
+		OPT_ARGUMENT("date-order", "use date order, preserving topology"),
+		OPT_ARGUMENT("reverse", "output commits in reverse order"),
+		OPT_GROUP("formatting output:"),
+		OPT_ARGUMENT("parents", "print the parents of the commit"),
+		OPT_ARGUMENT("children", "print the children of the commit"),
+		OPT_ARGUMENT("objects", "print all objects"),
+		OPT_ARGUMENT("objects-edge", "similar to --objects, used by git-pack-objects"),
+		OPT_ARGUMENT("unpacked", "print objects not in packs"),
+		{ OPTION_CALLBACK, 0, "header", &revs, NULL,
+		  "use raw-format", PARSE_OPT_NOARG, parse_header_cb, 0 },
+		OPT_ARGUMENT("pretty", "print contents in a given format"),
+		OPT_BOOLEAN(0, "timestamp", &show_timestamp,
+			    "print the raw commit timestamp"),
+		OPT_ARGUMENT("abbrev-commit", "show short sha1"),
+		OPT_ARGUMENT("abbrev=nr", "number of digits used for short sha1"),
+		OPT_ARGUMENT("no-abbrev", "do not use short sha1"),
+		OPT_ARGUMENT("left-right", "mark side of symmetric diff"),
+		OPT_ARGUMENT("graph", "show an ASCII graph"),
+		OPT_GROUP("special purpose:"),
+		OPT_BOOLEAN(0, "bisect", &bisect_list, "useful for binary searches"),
+		OPT_BOOLEAN(0, "bisect-all", &bisect_find_all,
+			    "order commits by their distance from given commits"),
+		OPT_BOOLEAN(0, "bisect-vars", &bisect_show_vars,
+			    "like --bisect, but ready to be eval'ed"),
+		OPT_END()
+	};
+
 	git_config(git_default_config, NULL);
 	init_revisions(&revs, prefix);
 	revs.abbrev = 0;
@@ -591,40 +613,16 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	argc = setup_revisions(argc, argv, &revs, NULL);
 
 	quiet = DIFF_OPT_TST(&revs.diffopt, QUIET);
-	for (i = 1 ; i < argc; i++) {
-		const char *arg = argv[i];
+	argc = parse_options(argc, argv, options, rev_list_usage, 0);
 
-		if (!strcmp(arg, "--header")) {
-			revs.verbose_header = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--timestamp")) {
-			show_timestamp = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--bisect")) {
-			bisect_list = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--bisect-all")) {
-			bisect_list = 1;
-			bisect_find_all = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--bisect-vars")) {
-			bisect_list = 1;
-			bisect_show_vars = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--stdin")) {
-			if (read_from_stdin++)
-				die("--stdin given twice?");
-			read_revisions_from_stdin(&revs);
-			continue;
-		}
-		usage(rev_list_usage);
+	if (argc > 0)
+		usage_with_options(rev_list_usage, options);
+
+	if (bisect_find_all || bisect_show_vars)
+		bisect_list = 1;
+	if (read_from_stdin)
+		read_revisions_from_stdin(&revs);
 
-	}
 	if (revs.commit_format != CMIT_FMT_UNSPECIFIED) {
 		/* The command line has a --pretty  */
 		hdr_termination = '\n';
@@ -643,7 +641,7 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 	     (!(revs.tag_objects||revs.tree_objects||revs.blob_objects) &&
 	      !revs.pending.nr)) ||
 	    revs.diff)
-		usage(rev_list_usage);
+		usage_with_options(rev_list_usage, options);
 
 	save_commit_buffer = revs.verbose_header || revs.grep_filter;
 	if (bisect_list)
-- 
1.5.6.3
Michele Ballabio· Jul 23, 2008, 21:42 UTC · re: Michele Ballabio · lore

[PATCH 6/9] builtin-init-db.c: use parse_options()

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
 builtin-init-db.c |   56 +++++++++++++++++++++++++++++++++-------------------
 1 files changed, 35 insertions(+), 21 deletions(-)
Show changes to builtin-init-db.c +35 −21
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 38b4fcb..ea1bed7 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -6,6 +6,7 @@
 #include "cache.h"
 #include "builtin.h"
 #include "exec_cmd.h"
+#include "parse-options.h"
 
 #ifndef DEFAULT_GIT_TEMPLATE_DIR
 #define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates"
@@ -353,8 +354,17 @@ static int guess_repository_type(const char *git_dir)
 	return 1;
 }
 
-static const char init_db_usage[] =
-"git init [-q | --quiet] [--bare] [--template=<template-directory>] [--shared[=<permissions>]]";
+static const char * const init_db_usage[] = {
+	"git init [-q | --quiet] [--bare] [--template=<dir>] [--shared[=<type>]]",
+	NULL
+};
+
+static int parse_opt_shared_cb(const struct option *opt, const char *arg,
+			       int unset)
+{
+	*(int *)(opt->value) = unset ? 0 : git_config_perm("arg", arg);
+	return 0;
+}
 
 /*
  * If you want to, you can share the DB area with any number of branches.
@@ -367,25 +377,29 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 	const char *git_dir;
 	const char *template_dir = NULL;
 	unsigned int flags = 0;
-	int i;
-
-	for (i = 1; i < argc; i++, argv++) {
-		const char *arg = argv[1];
-		if (!prefixcmp(arg, "--template="))
-			template_dir = arg+11;
-		else if (!strcmp(arg, "--bare")) {
-			static char git_dir[PATH_MAX+1];
-			is_bare_repository_cfg = 1;
-			setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir,
-						sizeof(git_dir)), 0);
-		} else if (!strcmp(arg, "--shared"))
-			shared_repository = PERM_GROUP;
-		else if (!prefixcmp(arg, "--shared="))
-			shared_repository = git_config_perm("arg", arg+9);
-		else if (!strcmp(arg, "-q") || !strcmp(arg, "--quiet"))
-			flags |= INIT_DB_QUIET;
-		else
-			usage(init_db_usage);
+	int bare = 0;
+
+	const struct option options[] = {
+		OPT_STRING(0, "template", &template_dir, "dir",
+			   "directory from which templates will be used"),
+		OPT_BOOLEAN(0, "bare", &bare, "set up a bare repo"),
+		{ OPTION_CALLBACK, 0, "shared", &shared_repository,
+		  "type", "type of shared repository",
+		  PARSE_OPT_OPTARG, parse_opt_shared_cb, PERM_GROUP },
+		OPT_BIT('q', "quiet", &flags, "be quiet", INIT_DB_QUIET),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, options, init_db_usage, 0);
+
+	if (argc > 0)
+		usage_with_options(init_db_usage, options);
+
+	if (bare) {
+		static char git_dir[PATH_MAX+1];
+		is_bare_repository_cfg = 1;
+		setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir,
+					sizeof(git_dir)), 0);
 	}
 
 	/*
-- 
1.5.6.3
Olivier Marin· Jul 24, 2008, 16:15 UTC · re: Michele Ballabio · lore

Re: [PATCH 6/9] builtin-init-db.c: use parse_options()

Michele Ballabio a écrit :
> 
> +	const struct option options[] = {
> +		OPT_STRING(0, "template", &template_dir, "dir",
> +			   "directory from which templates will be used"),
Perhaps "path", "path to the template repository" to stay consistent with clone.
> +		OPT_BOOLEAN(0, "bare", &bare, "set up a bare repo"),
s/set up/setup/ and s/repo/repository/?
> +		{ OPTION_CALLBACK, 0, "shared", &shared_repository,
> +		  "type", "type of shared repository",
What about "permissions", "setup a shared repository"?
> +		  PARSE_OPT_OPTARG, parse_opt_shared_cb, PERM_GROUP },
Are you sure the default value is really used here?

Also, perhaps we can play it safer by avoiding changing "share_repository" directly.

$ git init -> shared_repository == PERM_UMASK $ git init --shared --no-shared -> shared_repository == 0

It works because PERM_UMASK == 0, but it is a side effect. Don't you think?
> +		OPT_BIT('q', "quiet", &flags, "be quiet", INIT_DB_QUIET),
OPT__QUIET(&quiet),
if (quiet)
	flags |= INIT_DB_QUIET;
to use the same quiet option everywhere?

Just my opinion, Olivier.

Johannes Schindelin· Jul 24, 2008, 17:07 UTC · re: Olivier Marin · lore

Re: [PATCH 6/9] builtin-init-db.c: use parse_options()

Hi,
On Thu, 24 Jul 2008, Olivier Marin wrote:
Show 5 quoted lines
> Michele Ballabio a ??rit :
> 
> > +		OPT_BOOLEAN(0, "bare", &bare, "set up a bare repo"),
> 
> s/set up/setup/
No.  "setup" is a noun.
Show 11 quoted lines
> > +		{ OPTION_CALLBACK, 0, "shared", &shared_repository,
> > +		  "type", "type of shared repository",
> 
> What about "permissions", "setup a shared repository"?
> 
> > +		  PARSE_OPT_OPTARG, parse_opt_shared_cb, PERM_GROUP },
> 
> Are you sure the default value is really used here?
> 
> Also, perhaps we can play it safer by avoiding changing "share_repository"
> directly.
I do not see how that would be any safer.
> $ git init -> shared_repository == PERM_UMASK
> $ git init --shared --no-shared -> shared_repository == 0
> 
> It works because PERM_UMASK == 0, but it is a side effect. Don't you think?

Then the callback is wrong, too. I think, however, that it is by design, and correct.

We rely on shared_repository == 0 for non-shared repositories _almost everywhere_.

Show 8 quoted lines
> > +		OPT_BIT('q', "quiet", &flags, "be quiet", INIT_DB_QUIET),
> 
> OPT__QUIET(&quiet),
> 
> if (quiet)
> 	flags |= INIT_DB_QUIET;
> 
> to use the same quiet option everywhere?

Why? Doesn't make it more readable, I think. I'd rather have 3 lines less.

Ciao, Dscho

Olivier Marin· Jul 25, 2008, 15:20 UTC · re: Johannes Schindelin · lore

Re: [PATCH 6/9] builtin-init-db.c: use parse_options()

Johannes Schindelin a écrit :
Show 5 quoted lines
>>
>>> +		OPT_BOOLEAN(0, "bare", &bare, "set up a bare repo"),
>> s/set up/setup/
> 
> No.  "setup" is a noun.
Right, sorry.
> We rely on shared_repository == 0 for non-shared repositories _almost 
> everywhere_.

I think we rely on the fact that PERM_UMASK == 0 and not on the value of shared_repository. Not the same thing.

That said, perhaps you are right: it is harmless.
Show 10 quoted lines
>>> +		OPT_BIT('q', "quiet", &flags, "be quiet", INIT_DB_QUIET),
>> OPT__QUIET(&quiet),
>>
>> if (quiet)
>> 	flags |= INIT_DB_QUIET;
>>
>> to use the same quiet option everywhere?
> 
> Why?  Doesn't make it more readable, I think.  I'd rather have 3 lines 
> less.
Hum.
Olivier.
Johannes Schindelin· Jul 26, 2008, 00:55 UTC · re: Olivier Marin · lore

Re: [PATCH 6/9] builtin-init-db.c: use parse_options()

Hi,
On Fri, 25 Jul 2008, Olivier Marin wrote:
Show 7 quoted lines
> Johannes Schindelin a écrit :
> 
> > We rely on shared_repository == 0 for non-shared repositories _almost 
> > everywhere_.
> 
> I think we rely on the fact that PERM_UMASK == 0 and not on the value of 
> shared_repository. Not the same thing.

Just look at all the cases where we ask for "if (shared_repository)". And then look where PERM_UMASK is assigned to. It _is_ the same thing.

Hth, Dscho

Michele Ballabio· Jul 24, 2008, 20:07 UTC · re: Olivier Marin · lore

Re: [PATCH 6/9] builtin-init-db.c: use parse_options()

On Thursday 24 July 2008, Olivier Marin wrote:
Show 7 quoted lines
> Michele Ballabio a écrit :
> > 
> > +	const struct option options[] = {
> > +		OPT_STRING(0, "template", &template_dir, "dir",
> > +			   "directory from which templates will be used"),
> 
> Perhaps "path", "path to the template repository" to stay consistent with clone.
Ok.
> > +		OPT_BOOLEAN(0, "bare", &bare, "set up a bare repo"),
> 
> s/set up/setup/ and s/repo/repository/?
I think "set up a bare repository" will be fine.
> > +		{ OPTION_CALLBACK, 0, "shared", &shared_repository,
> > +		  "type", "type of shared repository",
> 
> What about "permissions", "setup a shared repository"?
Ok, but with s/setup/set up/.
> > +		  PARSE_OPT_OPTARG, parse_opt_shared_cb, PERM_GROUP },
> 
> Are you sure the default value is really used here?
Yes. Perhaps I don't understand your question. Can you explain what you mean?
Show 7 quoted lines
> Also, perhaps we can play it safer by avoiding changing "share_repository"
> directly.
> 
> $ git init -> shared_repository == PERM_UMASK
> $ git init --shared --no-shared -> shared_repository == 0
> 
> It works because PERM_UMASK == 0, but it is a side effect. Don't you think?
Would you like this better, with PARSE_OPT_NONEG?

+               { OPTION_CALLBACK, 0, "shared", &shared_repository, +                 "permissions", "set up a shared repository", +                 PARSE_OPT_OPTARG | PARSE_OPT_NONEG, parse_opt_shared_cb, PERM_GROUP },

Or do you prefer changing the callback like this:

+static int parse_opt_shared_cb(const struct option *opt, const char *arg, +                              int unset) +{ +       *(int *)(opt->value) = unset ? PERM_UMASK : git_config_perm("arg", arg); +       return 0; +}

Show 8 quoted lines
> > +		OPT_BIT('q', "quiet", &flags, "be quiet", INIT_DB_QUIET),
> 
> OPT__QUIET(&quiet),
> 
> if (quiet)
> 	flags |= INIT_DB_QUIET;
> 
> to use the same quiet option everywhere?

I thought about it and decided against ;) And it's one line vs four (counting "int quiet = 0;"). But I see your point.

Michele Ballabio· Jul 25, 2008, 08:15 UTC · re: Michele Ballabio · lore

[PATCH 6/9 - v2] builtin-init-db.c: use parse_options()

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
On Thursday 24 July 2008, Michele Ballabio wrote:
Show 7 quoted lines
> +static int parse_opt_shared_cb(const struct option *opt, const char *arg,
> +                              int unset)
> +{
> +       *(int *)(opt->value) = unset ? PERM_UMASK : git_config_perm("arg", arg);
> +       return 0;
> +}
> 
Did it this way (and changed help strings).
 builtin-init-db.c |   57 +++++++++++++++++++++++++++++++++-------------------
 1 files changed, 36 insertions(+), 21 deletions(-)
Show changes to builtin-init-db.c +36 −21
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 38b4fcb..42c2e20 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -6,6 +6,7 @@
 #include "cache.h"
 #include "builtin.h"
 #include "exec_cmd.h"
+#include "parse-options.h"
 
 #ifndef DEFAULT_GIT_TEMPLATE_DIR
 #define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates"
@@ -353,8 +354,18 @@ static int guess_repository_type(const char *git_dir)
 	return 1;
 }
 
-static const char init_db_usage[] =
-"git init [-q | --quiet] [--bare] [--template=<template-directory>] [--shared[=<permissions>]]";
+static const char * const init_db_usage[] = {
+	"git init [-q | --quiet] [--bare] [--template=<dir>] [--shared[=<type>]]",
+	NULL
+};
+
+static int parse_opt_shared_cb(const struct option *opt, const char *arg,
+			       int unset)
+{
+	*(int *)(opt->value) = unset ? PERM_UMASK :
+				       git_config_perm("arg", arg);
+	return 0;
+}
 
 /*
  * If you want to, you can share the DB area with any number of branches.
@@ -367,25 +378,29 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 	const char *git_dir;
 	const char *template_dir = NULL;
 	unsigned int flags = 0;
-	int i;
-
-	for (i = 1; i < argc; i++, argv++) {
-		const char *arg = argv[1];
-		if (!prefixcmp(arg, "--template="))
-			template_dir = arg+11;
-		else if (!strcmp(arg, "--bare")) {
-			static char git_dir[PATH_MAX+1];
-			is_bare_repository_cfg = 1;
-			setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir,
-						sizeof(git_dir)), 0);
-		} else if (!strcmp(arg, "--shared"))
-			shared_repository = PERM_GROUP;
-		else if (!prefixcmp(arg, "--shared="))
-			shared_repository = git_config_perm("arg", arg+9);
-		else if (!strcmp(arg, "-q") || !strcmp(arg, "--quiet"))
-			flags |= INIT_DB_QUIET;
-		else
-			usage(init_db_usage);
+	int bare = 0;
+
+	const struct option options[] = {
+		OPT_STRING(0, "template", &template_dir, "path",
+			   "path to the template directory"),
+		OPT_BOOLEAN(0, "bare", &bare, "set up a bare repository"),
+		{ OPTION_CALLBACK, 0, "shared", &shared_repository,
+		  "permissions", "set up a shared repository",
+		  PARSE_OPT_OPTARG, parse_opt_shared_cb, PERM_GROUP },
+		OPT_BIT('q', "quiet", &flags, "be quiet", INIT_DB_QUIET),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, options, init_db_usage, 0);
+
+	if (argc > 0)
+		usage_with_options(init_db_usage, options);
+
+	if (bare) {
+		static char git_dir[PATH_MAX+1];
+		is_bare_repository_cfg = 1;
+		setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir,
+					sizeof(git_dir)), 0);
 	}
 
 	/*
-- 
1.5.6.3
Olivier Marin· Jul 25, 2008, 15:20 UTC · re: Michele Ballabio · lore

Re: [PATCH 6/9] builtin-init-db.c: use parse_options()

Michele Ballabio a écrit :
Show 5 quoted lines
> 
>>> +		  PARSE_OPT_OPTARG, parse_opt_shared_cb, PERM_GROUP },
>> Are you sure the default value is really used here?
> 
> Yes. Perhaps I don't understand your question. Can you explain what you mean?

If I read the code correctly in parse-options.c, with OPTION_CALLBACK, the default value is not "automatically" used. You can use it in your callback if you want, but because you don't, I think it's never used.

> Would you like this better, with PARSE_OPT_NONEG?
No, I'm fine with the negated option.
Show 8 quoted lines
> Or do you prefer changing the callback like this:
> 
> +static int parse_opt_shared_cb(const struct option *opt, const char *arg,
> +                              int unset)
> +{
> +       *(int *)(opt->value) = unset ? PERM_UMASK : git_config_perm("arg", arg);
> +       return 0;
> +}
I think it's better but what I suggested is more something like:
static int parse_opt_shared_cb(const struct option *opt, const char *arg,
			       int unset)
{
	*(int *)(opt->value) = unset ? -1 : git_config_perm("arg", arg);
	return 0;
}
int shared = -1;
{ OPTION_CALLBACK, 0, "shared", &shared,
	"permissions", "setup as shared repository",
	PARSE_OPT_OPTARG, parse_perm_callback },
if (shared >= 0)
	shared_repository = shared;

This way we do not change shared_repository during parsing, so we do not loose the initial value.

But it seems nobody care about this kind of details, so perhaps, you can just ignore this suggestion.

Olivier.
Michele Ballabio· Jul 25, 2008, 19:53 UTC · re: Olivier Marin · lore

[PATCH 6/9 - v3] builtin-init-db.c: use parse_options()

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
On Friday 25 July 2008, Olivier Marin wrote:
Show 10 quoted lines
> Michele Ballabio a écrit :
> > 
> >>> +		  PARSE_OPT_OPTARG, parse_opt_shared_cb, PERM_GROUP },
> >> Are you sure the default value is really used here?
> > 
> > Yes. Perhaps I don't understand your question. Can you explain what you mean?
> 
> If I read the code correctly in parse-options.c, with OPTION_CALLBACK, the
> default value is not "automatically" used. You can use it in your callback
> if you want, but because you don't, I think it's never used.

Oh, you're right, but git_config_perm() handles NULL just fine, so I can remove it. Done in this patch, thanks.

Show 23 quoted lines
> what I suggested is more something like:
> 
> static int parse_opt_shared_cb(const struct option *opt, const char *arg,
> 			       int unset)
> {
> 	*(int *)(opt->value) = unset ? -1 : git_config_perm("arg", arg);
> 	return 0;
> }
> 
> int shared = -1;
> 
> { OPTION_CALLBACK, 0, "shared", &shared,
> 	"permissions", "setup as shared repository",
> 	PARSE_OPT_OPTARG, parse_perm_callback },
> 
> if (shared >= 0)
> 	shared_repository = shared;
> 
> This way we do not change shared_repository during parsing, so we do not
> loose the initial value.
> 
> But it seems nobody care about this kind of details, so perhaps, you can
> just ignore this suggestion.

I might be wrong, but shared_repository is initialized to PERM_UMASK and does not change before parse_options() is called, so this is not much useful.

 builtin-init-db.c |   57 +++++++++++++++++++++++++++++++++-------------------
 1 files changed, 36 insertions(+), 21 deletions(-)
Show changes to builtin-init-db.c +36 −21
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 38b4fcb..01b84a9 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -6,6 +6,7 @@
 #include "cache.h"
 #include "builtin.h"
 #include "exec_cmd.h"
+#include "parse-options.h"
 
 #ifndef DEFAULT_GIT_TEMPLATE_DIR
 #define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates"
@@ -353,8 +354,18 @@ static int guess_repository_type(const char *git_dir)
 	return 1;
 }
 
-static const char init_db_usage[] =
-"git init [-q | --quiet] [--bare] [--template=<template-directory>] [--shared[=<permissions>]]";
+static const char * const init_db_usage[] = {
+	"git init [-q | --quiet] [--bare] [--template=<dir>] [--shared[=<type>]]",
+	NULL
+};
+
+static int parse_opt_shared_cb(const struct option *opt, const char *arg,
+			       int unset)
+{
+	*(int *)(opt->value) = unset ? PERM_UMASK :
+				       git_config_perm("arg", arg);
+	return 0;
+}
 
 /*
  * If you want to, you can share the DB area with any number of branches.
@@ -367,25 +378,29 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 	const char *git_dir;
 	const char *template_dir = NULL;
 	unsigned int flags = 0;
-	int i;
-
-	for (i = 1; i < argc; i++, argv++) {
-		const char *arg = argv[1];
-		if (!prefixcmp(arg, "--template="))
-			template_dir = arg+11;
-		else if (!strcmp(arg, "--bare")) {
-			static char git_dir[PATH_MAX+1];
-			is_bare_repository_cfg = 1;
-			setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir,
-						sizeof(git_dir)), 0);
-		} else if (!strcmp(arg, "--shared"))
-			shared_repository = PERM_GROUP;
-		else if (!prefixcmp(arg, "--shared="))
-			shared_repository = git_config_perm("arg", arg+9);
-		else if (!strcmp(arg, "-q") || !strcmp(arg, "--quiet"))
-			flags |= INIT_DB_QUIET;
-		else
-			usage(init_db_usage);
+	int bare = 0;
+
+	const struct option options[] = {
+		OPT_STRING(0, "template", &template_dir, "path",
+			   "path to the template directory"),
+		OPT_BOOLEAN(0, "bare", &bare, "set up a bare repository"),
+		{ OPTION_CALLBACK, 0, "shared", &shared_repository,
+		  "permissions", "set up a shared repository",
+		  PARSE_OPT_OPTARG, parse_opt_shared_cb },
+		OPT_BIT('q', "quiet", &flags, "be quiet", INIT_DB_QUIET),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, options, init_db_usage, 0);
+
+	if (argc > 0)
+		usage_with_options(init_db_usage, options);
+
+	if (bare) {
+		static char git_dir[PATH_MAX+1];
+		is_bare_repository_cfg = 1;
+		setenv(GIT_DIR_ENVIRONMENT, getcwd(git_dir,
+					sizeof(git_dir)), 0);
 	}
 
 	/*
-- 
1.5.6.3
Michele Ballabio· Jul 23, 2008, 21:42 UTC · re: Michele Ballabio · lore
Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
 builtin-checkout-index.c |  146 +++++++++++++++++++++++++---------------------
 1 files changed, 79 insertions(+), 67 deletions(-)
Show changes to builtin-checkout-index.c +79 −67
diff --git a/builtin-checkout-index.c b/builtin-checkout-index.c
index 71ebabf..429c850 100644
--- a/builtin-checkout-index.c
+++ b/builtin-checkout-index.c
@@ -40,6 +40,7 @@
 #include "cache.h"
 #include "quote.h"
 #include "cache-tree.h"
+#include "parse-options.h"
 
 #define CHECKOUT_ALL 4
 static int line_termination = '\n';
@@ -153,18 +154,76 @@ static void checkout_all(const char *prefix, int prefix_length)
 		exit(128);
 }
 
-static const char checkout_cache_usage[] =
-"git checkout-index [-u] [-q] [-a] [-f] [-n] [--stage=[123]|all] [--prefix=<string>] [--temp] [--] <file>...";
+static const char * const checkout_cache_usage[] = {
+	"git checkout-index [options] [--] <file>...",
+	NULL
+};
+
+static int parse_state_force_cb(const struct option *opt, const char *arg, int unset)
+{
+	struct checkout *t_state = opt->value;
+	t_state->force = unset ? 0 : 1;
+	return 0;
+}
+
+static int parse_state_quiet_cb(const struct option *opt, const char *arg, int unset)
+{
+	struct checkout *t_state = opt->value;
+	t_state->quiet = unset ? 0 : 1;
+	return 0;
+}
+
+static int parse_state_no_create_cb(const struct option *opt, const char *arg, int unset)
+{
+	struct checkout *t_state = opt->value;
+	t_state->not_new = 1;
+	return 0;
+}
+
+static int parse_state_index_cb(const struct option *opt, const char *arg, int unset)
+{
+	struct checkout *t_state = opt->value;
+	t_state->refresh_cache = unset ? 0 : 1;
+	return 0;
+}
 
 static struct lock_file lock_file;
 
 int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 {
-	int i;
 	int newfd = -1;
 	int all = 0;
 	int read_from_stdin = 0;
 	int prefix_length;
+	char *stage = NULL;
+
+	const struct option options[] = {
+		OPT_BOOLEAN('a', "all", &all,
+			    "checks out all files in the index"),
+		{ OPTION_CALLBACK, 'f', "force", &state, NULL,
+		  "force overwrite of existing files",
+		  PARSE_OPT_NOARG, parse_state_force_cb, 0 },
+		{ OPTION_CALLBACK, 'q', "quiet", &state, NULL, "be quiet",
+		  PARSE_OPT_NOARG, parse_state_quiet_cb, 0 },
+		{ OPTION_CALLBACK, 'n', "no-create", &state, NULL,
+		  "do not checkout new files, refresh existing ones",
+		  PARSE_OPT_NOARG | PARSE_OPT_NONEG,
+		  parse_state_no_create_cb, 0 },
+		{ OPTION_CALLBACK, 'u', "index", &state, NULL,
+		  "update stat information in the index",
+		  PARSE_OPT_NOARG, parse_state_index_cb, 0 },
+		OPT_SET_INT('z', NULL, &line_termination,
+			    "separate paths with NUL", 0),
+		OPT_BOOLEAN(0, "stdin", &read_from_stdin,
+			    "read paths from stdin"),
+		OPT_BOOLEAN(0, "temp", &to_tempfile,
+			    "write content to temporary files"),
+		OPT_STRING(0, "prefix", &state.base_dir, "string",
+			   "prepend <string> when creating files"),
+		OPT_STRING(0, "stage", &stage, "1|2|3|all",
+			   "copy out files from the named stage"),
+		OPT_END()
+	};
 
 	git_config(git_default_config, NULL);
 	state.base_dir = "";
@@ -174,71 +233,24 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 		die("invalid cache");
 	}
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
+	argc = parse_options(argc, argv, options, checkout_cache_usage, 0);
 
-		if (!strcmp(arg, "--")) {
-			i++;
-			break;
-		}
-		if (!strcmp(arg, "-a") || !strcmp(arg, "--all")) {
-			all = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-f") || !strcmp(arg, "--force")) {
-			state.force = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-q") || !strcmp(arg, "--quiet")) {
-			state.quiet = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-n") || !strcmp(arg, "--no-create")) {
-			state.not_new = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-u") || !strcmp(arg, "--index")) {
-			state.refresh_cache = 1;
-			if (newfd < 0)
-				newfd = hold_locked_index(&lock_file, 1);
-			continue;
-		}
-		if (!strcmp(arg, "-z")) {
-			line_termination = 0;
-			continue;
-		}
-		if (!strcmp(arg, "--stdin")) {
-			if (i != argc - 1)
-				die("--stdin must be at the end");
-			read_from_stdin = 1;
-			i++; /* do not consider arg as a file name */
-			break;
-		}
-		if (!strcmp(arg, "--temp")) {
+	if ((state.refresh_cache) && (newfd < 0))
+		newfd = hold_locked_index(&lock_file, 1);
+	if (state.base_dir)
+		state.base_dir_len = strlen(state.base_dir);
+
+	if (stage) {
+		if (!strcmp(stage, "all")) {
 			to_tempfile = 1;
-			continue;
-		}
-		if (!prefixcmp(arg, "--prefix=")) {
-			state.base_dir = arg+9;
-			state.base_dir_len = strlen(state.base_dir);
-			continue;
-		}
-		if (!prefixcmp(arg, "--stage=")) {
-			if (!strcmp(arg + 8, "all")) {
-				to_tempfile = 1;
-				checkout_stage = CHECKOUT_ALL;
-			} else {
-				int ch = arg[8];
-				if ('1' <= ch && ch <= '3')
-					checkout_stage = arg[8] - '0';
-				else
-					die("stage should be between 1 and 3 or all");
-			}
-			continue;
+			checkout_stage = CHECKOUT_ALL;
+		} else {
+			int ch = stage[0];
+			if ('1' <= ch && ch <= '3')
+				checkout_stage = stage[0] - '0';
+			else
+				die("stage should be between 1 and 3 or all");
 		}
-		if (arg[0] == '-')
-			usage(checkout_cache_usage);
-		break;
 	}
 
 	if (state.base_dir_len || to_tempfile) {
@@ -253,8 +265,8 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 	}
 
 	/* Check out named files first */
-	for ( ; i < argc; i++) {
-		const char *arg = argv[i];
+	while (argc-- > 0) {
+		const char *arg = *argv++;
 		const char *p;
 
 		if (all)
-- 
1.5.6.3
Johannes Schindelin· Jul 24, 2008, 14:44 UTC · re: Michele Ballabio · lore

Re: [PATCH 7/9] builtin-checkout-index.c: use parse_options()

Hi,
On Wed, 23 Jul 2008, Michele Ballabio wrote:
> +		{ OPTION_CALLBACK, 'f', "force", &state, NULL,
> +		  "force overwrite of existing files",
> +		  PARSE_OPT_NOARG, parse_state_force_cb, 0 },
I wonder if this could not be written as
		OPT_BOOLEAN('f', "force", &state.force,
			"force overwrite of existing files"),

Ciao, Dscho

Michele Ballabio· Jul 24, 2008, 20:08 UTC · re: Johannes Schindelin · lore

Re: [PATCH 7/9] builtin-checkout-index.c: use parse_options()

On Thursday 24 July 2008, Johannes Schindelin wrote:
Show 10 quoted lines
> On Wed, 23 Jul 2008, Michele Ballabio wrote:
> 
> > +		{ OPTION_CALLBACK, 'f', "force", &state, NULL,
> > +		  "force overwrite of existing files",
> > +		  PARSE_OPT_NOARG, parse_state_force_cb, 0 },
> 
> I wonder if this could not be written as
> 
> 		OPT_BOOLEAN('f', "force", &state.force,
> 			"force overwrite of existing files"), 
I did it that way because 'force' is a bitfield.
Sverre Rabbelier· Jul 24, 2008, 20:35 UTC · re: Michele Ballabio · lore

Re: [PATCH 7/9] builtin-checkout-index.c: use parse_options()

On Thu, Jul 24, 2008 at 10:08 PM, Michele Ballabio <barra_cuda@katamail.com> wrote:

Show 13 quoted lines
> On Thursday 24 July 2008, Johannes Schindelin wrote:
>> On Wed, 23 Jul 2008, Michele Ballabio wrote:
>>
>> > +           { OPTION_CALLBACK, 'f', "force", &state, NULL,
>> > +             "force overwrite of existing files",
>> > +             PARSE_OPT_NOARG, parse_state_force_cb, 0 },
>>
>> I wonder if this could not be written as
>>
>>               OPT_BOOLEAN('f', "force", &state.force,
>>                       "force overwrite of existing files"),
>
> I did it that way because 'force' is a bitfield.
I thought there is an OPT_BIT?
-- 
Cheers,

Sverre Rabbelier
René Scharfe· Jul 25, 2008, 09:01 UTC · re: Sverre Rabbelier · lore

Re: [PATCH 7/9] builtin-checkout-index.c: use parse_options()

Sverre Rabbelier schrieb:
Show 15 quoted lines
> On Thu, Jul 24, 2008 at 10:08 PM, Michele Ballabio
> <barra_cuda@katamail.com> wrote:
>> On Thursday 24 July 2008, Johannes Schindelin wrote:
>>> On Wed, 23 Jul 2008, Michele Ballabio wrote:
>>>
>>>> +           { OPTION_CALLBACK, 'f', "force", &state, NULL,
>>>> +             "force overwrite of existing files",
>>>> +             PARSE_OPT_NOARG, parse_state_force_cb, 0 },
>>> I wonder if this could not be written as
>>>
>>>               OPT_BOOLEAN('f', "force", &state.force,
>>>                       "force overwrite of existing files"),
>> I did it that way because 'force' is a bitfield.
> 
> I thought there is an OPT_BIT?
OPT_BIT is for flags and bitmasks, not for bitfields.

Since you can't get the address of a bitfield member, a function that wants to change its value needs to know its name. Switching to bitmasks would make the option parsing code look cleaner, but you'd have to change all those bitfield accesses to explicit bitmask operations, e.g.:

	if (state.force)
		state.force = 0;
vs.
	if (state.flags & CHECKOUT_FORCE)
		state.flags &= ~CHECKOUT_FORCE;

In the case of struct checkout, though, we could simply make the bitfield members full ints, because there are only a few instances of this structure in memory at any given time. Wasting a few bytes of RAM in order to gain much simpler code is OK in this case, I think. OPT_BOOLEAN looks a lot nicer than a callback.

René
Michele Ballabio· Jul 25, 2008, 16:25 UTC · re: René Scharfe · lore

[PATCH 7/9 - v2] builtin-checkout-index.c: use parse_options()

This changes "struct checkout" (now it uses ints and not bitfields) to simplify the parsing code.

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
On Friday 25 July 2008, René Scharfe wrote:
Show 5 quoted lines
> In the case of struct checkout, though, we could simply make the
> bitfield members full ints, because there are only a few instances of
> this structure in memory at any given time. Wasting a few bytes of RAM
> in order to gain much simpler code is OK in this case, I think.
> OPT_BOOLEAN looks a lot nicer than a callback.

Yes. I only wanted the changes to be minimal, and only affect the option parsing. In this sense, I still think the old patch is better. Here it is the one you suggested (maybe Johannes suggested the same, but I didn't understand :).

 builtin-checkout-index.c |  113 +++++++++++++++++++---------------------------
 cache.h                  |    8 ++--
 2 files changed, 50 insertions(+), 71 deletions(-)
Show changes to 2 files +50 −71

builtin-checkout-index.c, cache.h

diff --git a/builtin-checkout-index.c b/builtin-checkout-index.c
index 71ebabf..135348e 100644
--- a/builtin-checkout-index.c
+++ b/builtin-checkout-index.c
@@ -40,6 +40,7 @@
 #include "cache.h"
 #include "quote.h"
 #include "cache-tree.h"
+#include "parse-options.h"
 
 #define CHECKOUT_ALL 4
 static int line_termination = '\n';
@@ -153,18 +154,43 @@ static void checkout_all(const char *prefix, int prefix_length)
 		exit(128);
 }
 
-static const char checkout_cache_usage[] =
-"git checkout-index [-u] [-q] [-a] [-f] [-n] [--stage=[123]|all] [--prefix=<string>] [--temp] [--] <file>...";
+static const char * const checkout_cache_usage[] = {
+	"git checkout-index [options] [--] <file>...",
+	NULL
+};
 
 static struct lock_file lock_file;
 
 int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 {
-	int i;
 	int newfd = -1;
 	int all = 0;
 	int read_from_stdin = 0;
 	int prefix_length;
+	char *stage = NULL;
+
+	const struct option options[] = {
+		OPT_BOOLEAN('a', "all", &all,
+			    "checks out all files in the index"),
+		OPT_BOOLEAN('f', "force", &state.force,
+			    "force overwrite of existing files"),
+		OPT__QUIET(&state.quiet),
+		OPT_SET_INT('n', "no-create", &state.not_new,
+			"do not checkout new files, refresh existing ones", 1),
+		OPT_BOOLEAN('u', "index", &state.refresh_cache,
+			    "update stat information in the index"),
+		OPT_SET_INT('z', NULL, &line_termination,
+			    "separate paths with NUL", 0),
+		OPT_BOOLEAN(0, "stdin", &read_from_stdin,
+			    "read paths from stdin"),
+		OPT_BOOLEAN(0, "temp", &to_tempfile,
+			    "write content to temporary files"),
+		OPT_STRING(0, "prefix", &state.base_dir, "string",
+			   "prepend <string> when creating files"),
+		OPT_STRING(0, "stage", &stage, "1|2|3|all",
+			   "copy out files from the named stage"),
+		OPT_END()
+	};
 
 	git_config(git_default_config, NULL);
 	state.base_dir = "";
@@ -174,71 +200,24 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 		die("invalid cache");
 	}
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
+	argc = parse_options(argc, argv, options, checkout_cache_usage, 0);
 
-		if (!strcmp(arg, "--")) {
-			i++;
-			break;
-		}
-		if (!strcmp(arg, "-a") || !strcmp(arg, "--all")) {
-			all = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-f") || !strcmp(arg, "--force")) {
-			state.force = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-q") || !strcmp(arg, "--quiet")) {
-			state.quiet = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-n") || !strcmp(arg, "--no-create")) {
-			state.not_new = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-u") || !strcmp(arg, "--index")) {
-			state.refresh_cache = 1;
-			if (newfd < 0)
-				newfd = hold_locked_index(&lock_file, 1);
-			continue;
-		}
-		if (!strcmp(arg, "-z")) {
-			line_termination = 0;
-			continue;
-		}
-		if (!strcmp(arg, "--stdin")) {
-			if (i != argc - 1)
-				die("--stdin must be at the end");
-			read_from_stdin = 1;
-			i++; /* do not consider arg as a file name */
-			break;
-		}
-		if (!strcmp(arg, "--temp")) {
+	if ((state.refresh_cache) && (newfd < 0))
+		newfd = hold_locked_index(&lock_file, 1);
+	if (state.base_dir)
+		state.base_dir_len = strlen(state.base_dir);
+
+	if (stage) {
+		if (!strcmp(stage, "all")) {
 			to_tempfile = 1;
-			continue;
-		}
-		if (!prefixcmp(arg, "--prefix=")) {
-			state.base_dir = arg+9;
-			state.base_dir_len = strlen(state.base_dir);
-			continue;
-		}
-		if (!prefixcmp(arg, "--stage=")) {
-			if (!strcmp(arg + 8, "all")) {
-				to_tempfile = 1;
-				checkout_stage = CHECKOUT_ALL;
-			} else {
-				int ch = arg[8];
-				if ('1' <= ch && ch <= '3')
-					checkout_stage = arg[8] - '0';
-				else
-					die("stage should be between 1 and 3 or all");
-			}
-			continue;
+			checkout_stage = CHECKOUT_ALL;
+		} else {
+			int ch = stage[0];
+			if ('1' <= ch && ch <= '3')
+				checkout_stage = stage[0] - '0';
+			else
+				die("stage should be between 1 and 3 or all");
 		}
-		if (arg[0] == '-')
-			usage(checkout_cache_usage);
-		break;
 	}
 
 	if (state.base_dir_len || to_tempfile) {
@@ -253,8 +232,8 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 	}
 
 	/* Check out named files first */
-	for ( ; i < argc; i++) {
-		const char *arg = argv[i];
+	while (argc-- > 0) {
+		const char *arg = *argv++;
 		const char *p;
 
 		if (all)
diff --git a/cache.h b/cache.h
index 38985aa..0bbe33b 100644
--- a/cache.h
+++ b/cache.h
@@ -618,10 +618,10 @@ extern const char *fmt_name(const char *name, const char *email);
 struct checkout {
 	const char *base_dir;
 	int base_dir_len;
-	unsigned force:1,
-		 quiet:1,
-		 not_new:1,
-		 refresh_cache:1;
+	int force;
+	int quiet;
+	int not_new;
+	int refresh_cache;
 };
 
 extern int checkout_entry(struct cache_entry *ce, const struct checkout *state, char *topath);
-- 
1.5.6.3
Michele Ballabio· Jul 23, 2008, 21:42 UTC · re: Michele Ballabio · lore

[PATCH 8/9] builtin-fetch-pack.c: use parse_options()

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
 builtin-fetch-pack.c |  144 +++++++++++++++++++++++++++++++-------------------
 1 files changed, 90 insertions(+), 54 deletions(-)
Show changes to builtin-fetch-pack.c +90 −54
diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 273239a..701be41 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -9,6 +9,7 @@
 #include "fetch-pack.h"
 #include "remote.h"
 #include "run-command.h"
+#include "parse-options.h"
 
 static int transfer_unpack_limit = -1;
 static int fetch_unpack_limit = -1;
@@ -17,8 +18,10 @@ static struct fetch_pack_args args = {
 	/* .uploadpack = */ "git-upload-pack",
 };
 
-static const char fetch_pack_usage[] =
-"git fetch-pack [--all] [--quiet|-q] [--keep|-k] [--thin] [--include-tag] [--upload-pack=<git-upload-pack>] [--depth=<n>] [--no-progress] [-v] [<host>:]<directory> [<refs>...]";
+static const char * const fetch_pack_usage[] = {
+	"git fetch-pack [options] [<host>:]<directory> [<refs>...]",
+	NULL
+};
 
 #define COMPLETE	(1U << 0)
 #define COMMON		(1U << 1)
@@ -667,6 +670,56 @@ static void fetch_pack_setup(void)
 	did_setup = 1;
 }
 
+static int parse_opt_keep_pack_cb(const struct option *opt, const char *arg, int unset)
+{
+	struct fetch_pack_args *t_args = opt->value;
+	t_args->lock_pack = t_args->keep_pack;
+	t_args->keep_pack = 1;
+	return 0;
+}
+
+static int parse_opt_thin_cb(const struct option *opt, const char *arg, int unset)
+{
+	struct fetch_pack_args *t_args = opt->value;
+	t_args->use_thin_pack = unset ? 0 : 1;
+	return 0;
+}
+
+static int parse_opt_include_tag_cb(const struct option *opt, const char *arg, int unset)
+{
+	struct fetch_pack_args *t_args = opt->value;
+	t_args->include_tag = unset ? 0 : 1;
+	return 0;
+}
+
+static int parse_opt_no_progress_cb(const struct option *opt, const char *arg, int unset)
+{
+	struct fetch_pack_args *t_args = opt->value;
+	t_args->no_progress = 1;
+	return 0;
+}
+
+static int parse_opt_fetch_all_cb(const struct option *opt, const char *arg, int unset)
+{
+	struct fetch_pack_args *t_args = opt->value;
+	t_args->fetch_all = unset ? 0 : 1;
+	return 0;
+}
+
+static int parse_opt_quiet_cb(const struct option *opt, const char *arg, int unset)
+{
+	struct fetch_pack_args *t_args = opt->value;
+	t_args->quiet = unset ? 0 : 1;
+	return 0;
+}
+
+static int parse_opt_verbose_cb(const struct option *opt, const char *arg, int unset)
+{
+	struct fetch_pack_args *t_args = opt->value;
+	t_args->verbose = unset ? 0 : 1;
+	return 0;
+}
+
 int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 {
 	int i, ret, nr_heads;
@@ -677,60 +730,43 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 
 	nr_heads = 0;
 	heads = NULL;
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
 
-		if (*arg == '-') {
-			if (!prefixcmp(arg, "--upload-pack=")) {
-				args.uploadpack = arg + 14;
-				continue;
-			}
-			if (!prefixcmp(arg, "--exec=")) {
-				args.uploadpack = arg + 7;
-				continue;
-			}
-			if (!strcmp("--quiet", arg) || !strcmp("-q", arg)) {
-				args.quiet = 1;
-				continue;
-			}
-			if (!strcmp("--keep", arg) || !strcmp("-k", arg)) {
-				args.lock_pack = args.keep_pack;
-				args.keep_pack = 1;
-				continue;
-			}
-			if (!strcmp("--thin", arg)) {
-				args.use_thin_pack = 1;
-				continue;
-			}
-			if (!strcmp("--include-tag", arg)) {
-				args.include_tag = 1;
-				continue;
-			}
-			if (!strcmp("--all", arg)) {
-				args.fetch_all = 1;
-				continue;
-			}
-			if (!strcmp("-v", arg)) {
-				args.verbose = 1;
-				continue;
-			}
-			if (!prefixcmp(arg, "--depth=")) {
-				args.depth = strtol(arg + 8, NULL, 0);
-				continue;
-			}
-			if (!strcmp("--no-progress", arg)) {
-				args.no_progress = 1;
-				continue;
-			}
-			usage(fetch_pack_usage);
-		}
-		dest = (char *)arg;
-		heads = (char **)(argv + i + 1);
-		nr_heads = argc - i - 1;
-		break;
-	}
+	const struct option options[] = {
+		{ OPTION_CALLBACK, 0, "all", &args, NULL,
+		 "fetch all remote refs", PARSE_OPT_NOARG,
+		 parse_opt_fetch_all_cb },
+		OPT_STRING(0, "upload-pack", &args.uploadpack, "git-upload-pack",
+			   "specify path to git-upload-pack on remote"),
+		OPT_STRING(0, "exec", &args.uploadpack, "git-upload-pack",
+			   "same as --upload-pack <git-upload-pack>."),
+		{ OPTION_CALLBACK, 0, "no-progress", &args, NULL,
+		 "do not show the progress", PARSE_OPT_NOARG | PARSE_OPT_NONEG,
+		 parse_opt_no_progress_cb },
+		{ OPTION_CALLBACK, 'q', "quiet", &args, NULL,
+		 "be quiet", PARSE_OPT_NOARG, parse_opt_quiet_cb },
+		{ OPTION_CALLBACK, 'v', "verbose", &args, NULL,
+		 "be verbose", PARSE_OPT_NOARG, parse_opt_verbose_cb },
+		OPT_INTEGER(0, "depth", &args.depth, "fetch chains not longer than <n>"),
+		{ OPTION_CALLBACK, 'k', "keep", &args, NULL,
+		 "create a single packfile of received data",
+		 PARSE_OPT_NOARG | PARSE_OPT_NONEG, parse_opt_keep_pack_cb },
+		{ OPTION_CALLBACK, 0, "include-tag", &args, NULL,
+		 "download annotated tags too", PARSE_OPT_NOARG,
+		 parse_opt_include_tag_cb },
+		{ OPTION_CALLBACK, 0, "thin", &args, NULL,
+		 "minimize number of objects to be sent",
+		 PARSE_OPT_NOARG, parse_opt_thin_cb },
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, options, fetch_pack_usage, 0);
+
+	dest = (char *)argv[0];
+	heads = (char **)(argv + 1);
+	nr_heads = argc - 1;
+
 	if (!dest)
-		usage(fetch_pack_usage);
+		usage_with_options(fetch_pack_usage, options);
 
 	conn = git_connect(fd, (char *)dest, args.uploadpack,
 			   args.verbose ? CONNECT_VERBOSE : 0);
-- 
1.5.6.3
Michele Ballabio· Jul 23, 2008, 21:42 UTC · re: Michele Ballabio · lore

[PATCH 9/9] builtin-mailinfo.c: use parse_options()

Signed-off-by: Michele Ballabio <barra_cuda@katamail.com>
---
 builtin-mailinfo.c |   39 +++++++++++++++++++++------------------
 1 files changed, 21 insertions(+), 18 deletions(-)
Show changes to builtin-mailinfo.c +21 −18
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index f974b9d..f1ed269 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -6,6 +6,7 @@
 #include "builtin.h"
 #include "utf8.h"
 #include "strbuf.h"
+#include "parse-options.h"
 
 static FILE *cmitmsg, *patchfile, *fin, *fout;
 
@@ -905,8 +906,10 @@ static int mailinfo(FILE *in, FILE *out, int ks, const char *encoding,
 	return 0;
 }
 
-static const char mailinfo_usage[] =
-	"git mailinfo [-k] [-u | --encoding=<encoding> | -n] msg patch <mail >info";
+static const char * const mailinfo_usage[] = {
+	"git mailinfo [-k] [-u | --encoding=<encoding> | -n] msg patch <mail >info",
+	NULL
+};
 
 int cmd_mailinfo(int argc, const char **argv, const char *prefix)
 {
@@ -920,22 +923,22 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)
 	def_charset = (git_commit_encoding ? git_commit_encoding : "utf-8");
 	metainfo_charset = def_charset;
 
-	while (1 < argc && argv[1][0] == '-') {
-		if (!strcmp(argv[1], "-k"))
-			keep_subject = 1;
-		else if (!strcmp(argv[1], "-u"))
-			metainfo_charset = def_charset;
-		else if (!strcmp(argv[1], "-n"))
-			metainfo_charset = NULL;
-		else if (!prefixcmp(argv[1], "--encoding="))
-			metainfo_charset = argv[1] + 11;
-		else
-			usage(mailinfo_usage);
-		argc--; argv++;
-	}
+	const struct option options[] = {
+		OPT_BOOLEAN('k', NULL, &keep_subject,
+			    "keep subject, don't clean it up"),
+		OPT_SET_PTR('u', NULL, &metainfo_charset,
+			    "re-code in UTF-8", (intptr_t)def_charset),
+		OPT_SET_PTR('n', NULL, &metainfo_charset,
+			    "disable re-coding", (intptr_t)NULL),
+		OPT_STRING(0, "encoding", &metainfo_charset,
+			   "encoding", "override default encoding"),
+		OPT_END()
+	};
+
+	argc = parse_options(argc, argv, options, mailinfo_usage, 0);
 
-	if (argc != 3)
-		usage(mailinfo_usage);
+	if (argc != 2)
+		usage_with_options(mailinfo_usage, options);
 
-	return !!mailinfo(stdin, stdout, keep_subject, metainfo_charset, argv[1], argv[2]);
+	return !!mailinfo(stdin, stdout, keep_subject, metainfo_charset, argv[0], argv[1]);
 }
-- 
1.5.6.3

← back to recent threads