small warm-up: easy parse-opt migrations.

small warm-up: easy parse-opt migrations.

[Pierre Habouzit]

There's not a lot to tell, those are just three migration to
parse-options.

[PATCH] parse-opt: migrate fmt-merge-msg.

[Pierre Habouzit]

Also fix an inefficient printf("%s", ...) where we can use write_in_full.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-fmt-merge-msg.c |   50 +++++++++++++++++++++-------------------------
 1 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index a1879f1..654d996 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -5,8 +5,10 @@
 #include "revision.h"
 #include "tag.h"
 
-static const char *fmt_merge_msg_usage =
-	"git fmt-merge-msg [--log] [--no-log] [--file <file>]";
+static const char * const fmt_merge_msg_usage[] = {
+	"git fmt-merge-msg [--log|--no-log] [--file <file>]",
+	NULL
+};
 
 static int merge_log;
 
@@ -342,37 +344,31 @@ int fmt_merge_msg(int merge_log, struct strbuf *in, struct strbuf *out) {
 
 int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 {
+	const char *inpath = NULL;
+	struct option options[] = {
+		OPT_BOOLEAN(0, "log", &merge_log, "populate log with the shortlog"),
+		OPT_STRING('F', "file", &inpath, "file", "file to read from"),
+		OPT_END()
+	};
+
 	FILE *in = stdin;
 	struct strbuf input, output;
 	int ret;
 
 	git_config(fmt_merge_msg_config, NULL);
-
-	while (argc > 1) {
-		if (!strcmp(argv[1], "--log"))
-			merge_log = 1;
-		else if (!strcmp(argv[1], "--no-log"))
-			merge_log = 0;
-		else if (!strcmp(argv[1], "-F") || !strcmp(argv[1], "--file")) {
-			if (argc < 3)
-				die ("Which file?");
-			if (!strcmp(argv[2], "-"))
-				in = stdin;
-			else {
-				fclose(in);
-				in = fopen(argv[2], "r");
-				if (!in)
-					die("cannot open %s", argv[2]);
-			}
-			argc--; argv++;
-		} else
-			break;
-		argc--; argv++;
+	argc = parse_options(argc, argv, options, fmt_merge_msg_usage, 0);
+	if (argc > 0)
+		usage_with_options(fmt_merge_msg_usage, options);
+
+	if (!inpath || strcmp(inpath, "-"))
+		in = stdin;
+	else {
+		fclose(in);
+		in = fopen(argv[2], "r");
+		if (!in)
+			die("cannot open %s", argv[2]);
 	}
 
-	if (argc > 1)
-		usage(fmt_merge_msg_usage);
-
 	strbuf_init(&input, 0);
 	if (strbuf_read(&input, fileno(in), 0) < 0)
 		die("could not read input file %s", strerror(errno));
@@ -381,6 +377,6 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 	ret = fmt_merge_msg(merge_log, &input, &output);
 	if (ret)
 		return ret;
-	printf("%s", output.buf);
+	write_in_full(STDOUT_FILENO, output.buf, output.len);
 	return 0;
 }
-- 
1.6.0.2.516.g12936

[PATCH] parse-opt: migrate git-merge-base.

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-merge-base.c |   37 ++++++++++++++++---------------------
 1 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/builtin-merge-base.c b/builtin-merge-base.c
index b08da51..03fc1c2 100644
--- a/builtin-merge-base.c
+++ b/builtin-merge-base.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "cache.h"
 #include "commit.h"
+#include "parse-options.h"
 
 static int show_merge_base(struct commit **rev, int rev_nr, int show_all)
 {
@@ -21,8 +22,10 @@ static int show_merge_base(struct commit **rev, int rev_nr, int show_all)
 	return 0;
 }
 
-static const char merge_base_usage[] =
-"git merge-base [--all] <commit-id> <commit-id>...";
+static const char * const merge_base_usage[] = {
+	"git merge-base [--all] <commit-id> <commit-id>...",
+	NULL
+};
 
 static struct commit *get_commit_reference(const char *arg)
 {
@@ -44,25 +47,17 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)
 	int rev_nr = 0;
 	int show_all = 0;
 
-	git_config(git_default_config, NULL);
-
-	while (1 < argc && argv[1][0] == '-') {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "-a") || !strcmp(arg, "--all"))
-			show_all = 1;
-		else
-			usage(merge_base_usage);
-		argc--; argv++;
-	}
-	if (argc < 3)
-		usage(merge_base_usage);
-
-	rev = xmalloc((argc - 1) * sizeof(*rev));
-
-	do {
-		rev[rev_nr++] = get_commit_reference(argv[1]);
-		argc--; argv++;
-	} while (argc > 1);
+	struct option options[] = {
+		OPT_BOOLEAN('a', "all", &show_all, "outputs all common ancestors"),
+		OPT_END()
+	};
 
+	git_config(git_default_config, NULL);
+	argc = parse_options(argc, argv, options, merge_base_usage, 0);
+	if (argc < 2)
+		usage_with_options(merge_base_usage, options);
+	rev = xmalloc(argc * sizeof(*rev));
+	while (argc-- > 0)
+		rev[rev_nr++] = get_commit_reference(*argv++);
 	return show_merge_base(rev, rev_nr, show_all);
 }
-- 
1.6.0.2.516.g12936

[PATCH] parse-opt: migrate builtin-merge-file.

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-merge-file.c |   68 +++++++++++++++++++++++++++----------------------
 1 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/builtin-merge-file.c b/builtin-merge-file.c
index 45c9853..7736fe8 100644
--- a/builtin-merge-file.c
+++ b/builtin-merge-file.c
@@ -2,21 +2,44 @@
 #include "cache.h"
 #include "xdiff/xdiff.h"
 #include "xdiff-interface.h"
+#include "parse-options.h"
 
-static const char merge_file_usage[] =
-"git merge-file [-p | --stdout] [--diff3] [-q | --quiet] [-L name1 [-L orig [-L name2]]] file1 orig_file file2";
+static const char *const merge_file_usage[] = {
+	"git merge-file [options] [-L name1 [-L orig [-L name2]]] file1 orig_file file2",
+	NULL
+};
+
+static int label_cb(const struct option *opt, const char *arg, int unset)
+{
+	static int label_count = 0;
+	const char **names = (const char **)opt->value;
+
+	if (label_count >= 3)
+		return error("too many labels on the command like");
+	names[label_count++] = arg;
+	return 0;
+}
 
 int cmd_merge_file(int argc, const char **argv, const char *prefix)
 {
-	const char *names[3];
+	const char *names[3] = { NULL, NULL, NULL };
 	mmfile_t mmfs[3];
 	mmbuffer_t result = {NULL, 0};
 	xpparam_t xpp = {XDF_NEED_MINIMAL};
 	int ret = 0, i = 0, to_stdout = 0;
 	int merge_level = XDL_MERGE_ZEALOUS_ALNUM;
-	int merge_style = 0;
+	int merge_style = 0, quiet = 0;
 	int nongit;
 
+	struct option options[] = {
+		OPT_BOOLEAN('p', "stdout", &to_stdout, "send results to standard output"),
+		OPT_SET_INT(0, "diff3", &merge_style, "use a diff3 based merge", XDL_MERGE_DIFF3),
+		OPT__QUIET(&quiet),
+		OPT_CALLBACK('L', NULL, names, "name",
+			     "set labels for file1/orig_file/file2", &label_cb),
+		OPT_END(),
+	};
+
 	prefix = setup_git_directory_gently(&nongit);
 	if (!nongit) {
 		/* Read the configuration file */
@@ -25,37 +48,20 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 			merge_style = git_xmerge_style;
 	}
 
-	while (argc > 4) {
-		if (!strcmp(argv[1], "-L") && i < 3) {
-			names[i++] = argv[2];
-			argc--;
-			argv++;
-		} else if (!strcmp(argv[1], "-p") ||
-				!strcmp(argv[1], "--stdout"))
-			to_stdout = 1;
-		else if (!strcmp(argv[1], "-q") ||
-				!strcmp(argv[1], "--quiet"))
-			freopen("/dev/null", "w", stderr);
-		else if (!strcmp(argv[1], "--diff3"))
-			merge_style = XDL_MERGE_DIFF3;
-		else
-			usage(merge_file_usage);
-		argc--;
-		argv++;
-	}
-
-	if (argc != 4)
-		usage(merge_file_usage);
-
-	for (; i < 3; i++)
-		names[i] = argv[i + 1];
+	argc = parse_options(argc, argv, options, merge_file_usage, 0);
+	if (argc != 3)
+		usage_with_options(merge_file_usage, options);
+	if (quiet)
+		freopen("/dev/null", "w", stderr);
 
 	for (i = 0; i < 3; i++) {
-		if (read_mmfile(mmfs + i, argv[i + 1]))
+		if (!names[i])
+			names[i] = argv[i];
+		if (read_mmfile(mmfs + i, argv[i]))
 			return -1;
 		if (buffer_is_binary(mmfs[i].ptr, mmfs[i].size))
 			return error("Cannot merge binary files: %s\n",
-					argv[i + 1]);
+					argv[i]);
 	}
 
 	ret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],
@@ -65,7 +71,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 		free(mmfs[i].ptr);
 
 	if (ret >= 0) {
-		const char *filename = argv[1];
+		const char *filename = argv[0];
 		FILE *f = to_stdout ? stdout : fopen(filename, "wb");
 
 		if (!f)
-- 
1.6.0.2.516.g12936

Re: [PATCH] parse-opt: migrate fmt-merge-msg.

[Shawn O. Pearce]

Pierre Habouzit <madcoder@debian.org> wrote:
> Also fix an inefficient printf("%s", ...) where we can use write_in_full.
> 
> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
>  builtin-fmt-merge-msg.c |   50 +++++++++++++++++++++-------------------------
>  1 files changed, 23 insertions(+), 27 deletions(-)

Near as I can tell, this is based upon a merge commit in next.

We can't do that.  Patches need to be based on master, or worst-case
on a topic head that is in next or pu (in which case the name of
the topic, or its tip commit, is helpful in the note).

-- 
Shawn.

Re: [PATCH] parse-opt: migrate fmt-merge-msg.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]

On Mon, Sep 29, 2008 at 04:35:23PM +0000, Shawn O. Pearce wrote:
> Pierre Habouzit <madcoder@debian.org> wrote:
> > Also fix an inefficient printf("%s", ...) where we can use write_in_full.
> > 
> > Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> > ---
> >  builtin-fmt-merge-msg.c |   50 +++++++++++++++++++++-------------------------
> >  1 files changed, 23 insertions(+), 27 deletions(-)
> 
> Near as I can tell, this is based upon a merge commit in next.
> 
> We can't do that.  Patches need to be based on master, or worst-case
> on a topic head that is in next or pu (in which case the name of
> the topic, or its tip commit, is helpful in the note).

Hmm I've always sent my patches this way, and I believe you can git am
-3 them on top of master easily. I can send you the updated series if
you want.
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] parse-opt: migrate fmt-merge-msg.

[Shawn O. Pearce]

Pierre Habouzit <madcoder@debian.org> wrote:
> On Mon, Sep 29, 2008 at 04:35:23PM +0000, Shawn O. Pearce wrote:
> > Pierre Habouzit <madcoder@debian.org> wrote:
> > > Also fix an inefficient printf("%s", ...) where we can use write_in_full.
> > 
> > Near as I can tell, this is based upon a merge commit in next.
> 
> Hmm I've always sent my patches this way, and I believe you can git am
> -3 them on top of master easily. I can send you the updated series if
> you want.

I'd appreciate an updated series if you can send it.  am -3 isn't
"easily" applying it.  Here I define "easy" as "the patch applies
without me needing to resolve conflicts":

 $ git co -b ph/parseopt master
 $ git am -3 -s X
 Applying: parse-opt: migrate fmt-merge-msg.
 error: patch failed: builtin-fmt-merge-msg.c:5
 error: builtin-fmt-merge-msg.c: patch does not apply
 Using index info to reconstruct a base tree...
 Falling back to patching base and 3-way merge...
 CONFLICT (content): Merge conflict in builtin-fmt-merge-msg.c
 Recorded preimage for 'builtin-fmt-merge-msg.c'
 Failed to merge in the changes.
 Patch failed at 0001.
 When you have resolved this problem run "git am -3 --resolved".
 If you would prefer to skip this patch, instead run "git am -3 --skip".
 To restore the original branch and stop patching run "git am -3 --abort".

:-\

-- 
Shawn.

Re: [PATCH] parse-opt: migrate fmt-merge-msg.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1829 bytes --]

On Tue, Sep 30, 2008 at 07:10:14PM +0000, Shawn O. Pearce wrote:
> Pierre Habouzit <madcoder@debian.org> wrote:
> > On Mon, Sep 29, 2008 at 04:35:23PM +0000, Shawn O. Pearce wrote:
> > > Pierre Habouzit <madcoder@debian.org> wrote:
> > > > Also fix an inefficient printf("%s", ...) where we can use write_in_full.
> > > 
> > > Near as I can tell, this is based upon a merge commit in next.
> > 
> > Hmm I've always sent my patches this way, and I believe you can git am
> > -3 them on top of master easily. I can send you the updated series if
> > you want.
> 
> I'd appreciate an updated series if you can send it.  am -3 isn't
> "easily" applying it.  Here I define "easy" as "the patch applies
> without me needing to resolve conflicts":
> 
>  $ git co -b ph/parseopt master
>  $ git am -3 -s X
>  Applying: parse-opt: migrate fmt-merge-msg.
>  error: patch failed: builtin-fmt-merge-msg.c:5
>  error: builtin-fmt-merge-msg.c: patch does not apply
>  Using index info to reconstruct a base tree...
>  Falling back to patching base and 3-way merge...
>  CONFLICT (content): Merge conflict in builtin-fmt-merge-msg.c
>  Recorded preimage for 'builtin-fmt-merge-msg.c'
>  Failed to merge in the changes.
>  Patch failed at 0001.
>  When you have resolved this problem run "git am -3 --resolved".
>  If you would prefer to skip this patch, instead run "git am -3 --skip".
>  To restore the original branch and stop patching run "git am -3 --abort".

Okay, I will then, but FWIW it means that when you'll try to merge this
in next it'll conflict at that time, so I'm not sure there's a huge win
for you at that point.

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

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] parse-opt: migrate fmt-merge-msg.

[Shawn O. Pearce]

Pierre Habouzit <madcoder@debian.org> wrote:
> On Tue, Sep 30, 2008 at 07:10:14PM +0000, Shawn O. Pearce wrote:
> > Pierre Habouzit <madcoder@debian.org> wrote:
> > > On Mon, Sep 29, 2008 at 04:35:23PM +0000, Shawn O. Pearce wrote:
> > > > Pierre Habouzit <madcoder@debian.org> wrote:
> > > > > Also fix an inefficient printf("%s", ...) where we can use write_in_full.
> > 
> > I'd appreciate an updated series if you can send it.  am -3 isn't
> > "easily" applying it.
> 
> Okay, I will then, but FWIW it means that when you'll try to merge this
> in next it'll conflict at that time, so I'm not sure there's a huge win
> for you at that point.

It may actually be a good idea to rebase this against master.

Reading Junio's notes for sg/merge-options (the branch this conflict
is coming out of) it sounds like we'd want to revert that anyway.
Its been around since April and Junio was talking about it needing
to be in a 1.7.0 release.  Its not going to graduate anytime soon.

IOW I'm quite tempted to revert sg/merge-options and cancel the
branch out of next.

-- 
Shawn.

[PATCH 1/3] parse-opt: migrate fmt-merge-msg.

[Pierre Habouzit]

Also fix an inefficient printf("%s", ...) where we can use write_in_full.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-fmt-merge-msg.c |   52 +++++++++++++++++++++-------------------------
 1 files changed, 24 insertions(+), 28 deletions(-)

   > It may actually be a good idea to rebase this against master.
   >
   > Reading Junio's notes for sg/merge-options (the branch this conflict
   > is coming out of) it sounds like we'd want to revert that anyway.
   > Its been around since April and Junio was talking about it needing
   > to be in a 1.7.0 release.  Its not going to graduate anytime soon.

   Fair enough, here it is.


diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index df02ba7..78c04bf 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -5,8 +5,10 @@
 #include "revision.h"
 #include "tag.h"
 
-static const char *fmt_merge_msg_usage =
-	"git fmt-merge-msg [--log] [--no-log] [--file <file>]";
+static const char * const fmt_merge_msg_usage[] = {
+	"git fmt-merge-msg [--log|--no-log] [--file <file>]",
+	NULL
+};
 
 static int merge_summary;
 
@@ -347,38 +349,32 @@ int fmt_merge_msg(int merge_summary, struct strbuf *in, struct strbuf *out) {
 
 int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 {
+	const char *inpath = NULL;
+	struct option options[] = {
+		OPT_BOOLEAN(0, "log",     &merge_summary, "populate log with the shortlog"),
+		OPT_BOOLEAN(0, "summary", &merge_summary, "alias for --log"),
+		OPT_STRING('F', "file",   &inpath, "file", "file to read from"),
+		OPT_END()
+	};
+
 	FILE *in = stdin;
 	struct strbuf input, output;
 	int ret;
 
 	git_config(fmt_merge_msg_config, NULL);
-
-	while (argc > 1) {
-		if (!strcmp(argv[1], "--log") || !strcmp(argv[1], "--summary"))
-			merge_summary = 1;
-		else if (!strcmp(argv[1], "--no-log")
-				|| !strcmp(argv[1], "--no-summary"))
-			merge_summary = 0;
-		else if (!strcmp(argv[1], "-F") || !strcmp(argv[1], "--file")) {
-			if (argc < 3)
-				die ("Which file?");
-			if (!strcmp(argv[2], "-"))
-				in = stdin;
-			else {
-				fclose(in);
-				in = fopen(argv[2], "r");
-				if (!in)
-					die("cannot open %s", argv[2]);
-			}
-			argc--; argv++;
-		} else
-			break;
-		argc--; argv++;
+	argc = parse_options(argc, argv, options, fmt_merge_msg_usage, 0);
+	if (argc > 0)
+		usage_with_options(fmt_merge_msg_usage, options);
+
+	if (!inpath || strcmp(inpath, "-"))
+		in = stdin;
+	else {
+		fclose(in);
+		in = fopen(argv[2], "r");
+		if (!in)
+			die("cannot open %s", argv[2]);
 	}
 
-	if (argc > 1)
-		usage(fmt_merge_msg_usage);
-
 	strbuf_init(&input, 0);
 	if (strbuf_read(&input, fileno(in), 0) < 0)
 		die("could not read input file %s", strerror(errno));
@@ -387,6 +383,6 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 	ret = fmt_merge_msg(merge_summary, &input, &output);
 	if (ret)
 		return ret;
-	printf("%s", output.buf);
+	write_in_full(STDOUT_FILENO, output.buf, output.len);
 	return 0;
 }
-- 
1.6.0.2.415.g9800c0.dirty

[PATCH 2/3] parse-opt: migrate git-merge-base.

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-merge-base.c |   37 ++++++++++++++++---------------------
 1 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/builtin-merge-base.c b/builtin-merge-base.c
index b08da51..03fc1c2 100644
--- a/builtin-merge-base.c
+++ b/builtin-merge-base.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "cache.h"
 #include "commit.h"
+#include "parse-options.h"
 
 static int show_merge_base(struct commit **rev, int rev_nr, int show_all)
 {
@@ -21,8 +22,10 @@ static int show_merge_base(struct commit **rev, int rev_nr, int show_all)
 	return 0;
 }
 
-static const char merge_base_usage[] =
-"git merge-base [--all] <commit-id> <commit-id>...";
+static const char * const merge_base_usage[] = {
+	"git merge-base [--all] <commit-id> <commit-id>...",
+	NULL
+};
 
 static struct commit *get_commit_reference(const char *arg)
 {
@@ -44,25 +47,17 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)
 	int rev_nr = 0;
 	int show_all = 0;
 
-	git_config(git_default_config, NULL);
-
-	while (1 < argc && argv[1][0] == '-') {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "-a") || !strcmp(arg, "--all"))
-			show_all = 1;
-		else
-			usage(merge_base_usage);
-		argc--; argv++;
-	}
-	if (argc < 3)
-		usage(merge_base_usage);
-
-	rev = xmalloc((argc - 1) * sizeof(*rev));
-
-	do {
-		rev[rev_nr++] = get_commit_reference(argv[1]);
-		argc--; argv++;
-	} while (argc > 1);
+	struct option options[] = {
+		OPT_BOOLEAN('a', "all", &show_all, "outputs all common ancestors"),
+		OPT_END()
+	};
 
+	git_config(git_default_config, NULL);
+	argc = parse_options(argc, argv, options, merge_base_usage, 0);
+	if (argc < 2)
+		usage_with_options(merge_base_usage, options);
+	rev = xmalloc(argc * sizeof(*rev));
+	while (argc-- > 0)
+		rev[rev_nr++] = get_commit_reference(*argv++);
 	return show_merge_base(rev, rev_nr, show_all);
 }
-- 
1.6.0.2.415.g9800c0.dirty

[PATCH 3/3] parse-opt: migrate builtin-merge-file.

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-merge-file.c |   68 +++++++++++++++++++++++++++----------------------
 1 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/builtin-merge-file.c b/builtin-merge-file.c
index 45c9853..7736fe8 100644
--- a/builtin-merge-file.c
+++ b/builtin-merge-file.c
@@ -2,21 +2,44 @@
 #include "cache.h"
 #include "xdiff/xdiff.h"
 #include "xdiff-interface.h"
+#include "parse-options.h"
 
-static const char merge_file_usage[] =
-"git merge-file [-p | --stdout] [--diff3] [-q | --quiet] [-L name1 [-L orig [-L name2]]] file1 orig_file file2";
+static const char *const merge_file_usage[] = {
+	"git merge-file [options] [-L name1 [-L orig [-L name2]]] file1 orig_file file2",
+	NULL
+};
+
+static int label_cb(const struct option *opt, const char *arg, int unset)
+{
+	static int label_count = 0;
+	const char **names = (const char **)opt->value;
+
+	if (label_count >= 3)
+		return error("too many labels on the command like");
+	names[label_count++] = arg;
+	return 0;
+}
 
 int cmd_merge_file(int argc, const char **argv, const char *prefix)
 {
-	const char *names[3];
+	const char *names[3] = { NULL, NULL, NULL };
 	mmfile_t mmfs[3];
 	mmbuffer_t result = {NULL, 0};
 	xpparam_t xpp = {XDF_NEED_MINIMAL};
 	int ret = 0, i = 0, to_stdout = 0;
 	int merge_level = XDL_MERGE_ZEALOUS_ALNUM;
-	int merge_style = 0;
+	int merge_style = 0, quiet = 0;
 	int nongit;
 
+	struct option options[] = {
+		OPT_BOOLEAN('p', "stdout", &to_stdout, "send results to standard output"),
+		OPT_SET_INT(0, "diff3", &merge_style, "use a diff3 based merge", XDL_MERGE_DIFF3),
+		OPT__QUIET(&quiet),
+		OPT_CALLBACK('L', NULL, names, "name",
+			     "set labels for file1/orig_file/file2", &label_cb),
+		OPT_END(),
+	};
+
 	prefix = setup_git_directory_gently(&nongit);
 	if (!nongit) {
 		/* Read the configuration file */
@@ -25,37 +48,20 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 			merge_style = git_xmerge_style;
 	}
 
-	while (argc > 4) {
-		if (!strcmp(argv[1], "-L") && i < 3) {
-			names[i++] = argv[2];
-			argc--;
-			argv++;
-		} else if (!strcmp(argv[1], "-p") ||
-				!strcmp(argv[1], "--stdout"))
-			to_stdout = 1;
-		else if (!strcmp(argv[1], "-q") ||
-				!strcmp(argv[1], "--quiet"))
-			freopen("/dev/null", "w", stderr);
-		else if (!strcmp(argv[1], "--diff3"))
-			merge_style = XDL_MERGE_DIFF3;
-		else
-			usage(merge_file_usage);
-		argc--;
-		argv++;
-	}
-
-	if (argc != 4)
-		usage(merge_file_usage);
-
-	for (; i < 3; i++)
-		names[i] = argv[i + 1];
+	argc = parse_options(argc, argv, options, merge_file_usage, 0);
+	if (argc != 3)
+		usage_with_options(merge_file_usage, options);
+	if (quiet)
+		freopen("/dev/null", "w", stderr);
 
 	for (i = 0; i < 3; i++) {
-		if (read_mmfile(mmfs + i, argv[i + 1]))
+		if (!names[i])
+			names[i] = argv[i];
+		if (read_mmfile(mmfs + i, argv[i]))
 			return -1;
 		if (buffer_is_binary(mmfs[i].ptr, mmfs[i].size))
 			return error("Cannot merge binary files: %s\n",
-					argv[i + 1]);
+					argv[i]);
 	}
 
 	ret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],
@@ -65,7 +71,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 		free(mmfs[i].ptr);
 
 	if (ret >= 0) {
-		const char *filename = argv[1];
+		const char *filename = argv[0];
 		FILE *f = to_stdout ? stdout : fopen(filename, "wb");
 
 		if (!f)
-- 
1.6.0.2.415.g9800c0.dirty

Re: [PATCH 3/3] parse-opt: migrate builtin-merge-file.

[Shawn O. Pearce]

Pierre Habouzit <madcoder@debian.org> wrote:
> diff --git a/builtin-merge-file.c b/builtin-merge-file.c
> index 45c9853..7736fe8 100644
> --- a/builtin-merge-file.c
> +++ b/builtin-merge-file.c
> @@ -2,21 +2,44 @@
...
> +static int label_cb(const struct option *opt, const char *arg, int unset)
> +{
> +	static int label_count = 0;
> +	const char **names = (const char **)opt->value;
> +
> +	if (label_count >= 3)
> +		return error("too many labels on the command like");

Typo on "command line".

-- 
Shawn.

Re: [PATCH 1/3] parse-opt: migrate fmt-merge-msg.

[Shawn O. Pearce]

Pierre Habouzit <madcoder@debian.org> wrote:
> @@ -347,38 +349,32 @@ int fmt_merge_msg(int merge_summary, struct strbuf *in, struct strbuf *out) {
>  
>  int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
>  {
> +	const char *inpath = NULL;
> +	struct option options[] = {
> +		OPT_BOOLEAN(0, "log",     &merge_summary, "populate log with the shortlog"),
> +		OPT_BOOLEAN(0, "summary", &merge_summary, "alias for --log"),
> +		OPT_STRING('F', "file",   &inpath, "file", "file to read from"),
> +		OPT_END()


> +	argc = parse_options(argc, argv, options, fmt_merge_msg_usage, 0);
> +	if (argc > 0)
> +		usage_with_options(fmt_merge_msg_usage, options);
> +
> +	if (!inpath || strcmp(inpath, "-"))
> +		in = stdin;
> +	else {
> +		fclose(in);
> +		in = fopen(argv[2], "r");
> +		if (!in)
> +			die("cannot open %s", argv[2]);

Really argv[2]?  Shouldn't that be inpath?

-- 
Shawn.

Re: [PATCH 1/3] parse-opt: migrate fmt-merge-msg.

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 1283 bytes --]

On Wed, Oct 01, 2008 at 02:56:06PM +0000, Shawn O. Pearce wrote:
> Pierre Habouzit <madcoder@debian.org> wrote:
> > @@ -347,38 +349,32 @@ int fmt_merge_msg(int merge_summary, struct strbuf *in, struct strbuf *out) {
> >  
> >  int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
> >  {
> > +	const char *inpath = NULL;
> > +	struct option options[] = {
> > +		OPT_BOOLEAN(0, "log",     &merge_summary, "populate log with the shortlog"),
> > +		OPT_BOOLEAN(0, "summary", &merge_summary, "alias for --log"),
> > +		OPT_STRING('F', "file",   &inpath, "file", "file to read from"),
> > +		OPT_END()
> 
> 
> > +	argc = parse_options(argc, argv, options, fmt_merge_msg_usage, 0);
> > +	if (argc > 0)
> > +		usage_with_options(fmt_merge_msg_usage, options);
> > +
> > +	if (!inpath || strcmp(inpath, "-"))
> > +		in = stdin;
> > +	else {
> > +		fclose(in);
> > +		in = fopen(argv[2], "r");
> > +		if (!in)
> > +			die("cannot open %s", argv[2]);
> 
> Really argv[2]?  Shouldn't that be inpath?

oh crap you're right. I'll submit and updated series soon... *sigh*

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

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]

[RESEND PATCH] parse-opt: migrate fmt-merge-msg.

[Pierre Habouzit]

Also fix an inefficient printf("%s", ...) where we can use write_in_full.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-fmt-merge-msg.c |   49 ++++++++++++++++++++--------------------------
 1 files changed, 21 insertions(+), 28 deletions(-)

 Also fixes a thinko wrt strcmp(inpath, "-"), a ! was missing ;)

diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index df02ba7..c6324dc 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -5,8 +5,10 @@
 #include "revision.h"
 #include "tag.h"
 
-static const char *fmt_merge_msg_usage =
-	"git fmt-merge-msg [--log] [--no-log] [--file <file>]";
+static const char * const fmt_merge_msg_usage[] = {
+	"git fmt-merge-msg [--log|--no-log] [--file <file>]",
+	NULL
+};
 
 static int merge_summary;
 
@@ -347,38 +349,29 @@ int fmt_merge_msg(int merge_summary, struct strbuf *in, struct strbuf *out) {
 
 int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 {
+	const char *inpath = NULL;
+	struct option options[] = {
+		OPT_BOOLEAN(0, "log",     &merge_summary, "populate log with the shortlog"),
+		OPT_BOOLEAN(0, "summary", &merge_summary, "alias for --log"),
+		OPT_STRING('F', "file",   &inpath, "file", "file to read from"),
+		OPT_END()
+	};
+
 	FILE *in = stdin;
 	struct strbuf input, output;
 	int ret;
 
 	git_config(fmt_merge_msg_config, NULL);
-
-	while (argc > 1) {
-		if (!strcmp(argv[1], "--log") || !strcmp(argv[1], "--summary"))
-			merge_summary = 1;
-		else if (!strcmp(argv[1], "--no-log")
-				|| !strcmp(argv[1], "--no-summary"))
-			merge_summary = 0;
-		else if (!strcmp(argv[1], "-F") || !strcmp(argv[1], "--file")) {
-			if (argc < 3)
-				die ("Which file?");
-			if (!strcmp(argv[2], "-"))
-				in = stdin;
-			else {
-				fclose(in);
-				in = fopen(argv[2], "r");
-				if (!in)
-					die("cannot open %s", argv[2]);
-			}
-			argc--; argv++;
-		} else
-			break;
-		argc--; argv++;
+	argc = parse_options(argc, argv, options, fmt_merge_msg_usage, 0);
+	if (argc > 0)
+		usage_with_options(fmt_merge_msg_usage, options);
+
+	if (inpath && strcmp(inpath, "-")) {
+		in = fopen(inpath, "r");
+		if (!in)
+			die("cannot open %s", inpath);
 	}
 
-	if (argc > 1)
-		usage(fmt_merge_msg_usage);
-
 	strbuf_init(&input, 0);
 	if (strbuf_read(&input, fileno(in), 0) < 0)
 		die("could not read input file %s", strerror(errno));
@@ -387,6 +380,6 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 	ret = fmt_merge_msg(merge_summary, &input, &output);
 	if (ret)
 		return ret;
-	printf("%s", output.buf);
+	write_in_full(STDOUT_FILENO, output.buf, output.len);
 	return 0;
 }
-- 
1.6.0.2.431.gb53ee

[PATCH] parse-opt: migrate git-merge-base.

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-merge-base.c |   37 ++++++++++++++++---------------------
 1 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/builtin-merge-base.c b/builtin-merge-base.c
index b08da51..03fc1c2 100644
--- a/builtin-merge-base.c
+++ b/builtin-merge-base.c
@@ -1,6 +1,7 @@
 #include "builtin.h"
 #include "cache.h"
 #include "commit.h"
+#include "parse-options.h"
 
 static int show_merge_base(struct commit **rev, int rev_nr, int show_all)
 {
@@ -21,8 +22,10 @@ static int show_merge_base(struct commit **rev, int rev_nr, int show_all)
 	return 0;
 }
 
-static const char merge_base_usage[] =
-"git merge-base [--all] <commit-id> <commit-id>...";
+static const char * const merge_base_usage[] = {
+	"git merge-base [--all] <commit-id> <commit-id>...",
+	NULL
+};
 
 static struct commit *get_commit_reference(const char *arg)
 {
@@ -44,25 +47,17 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)
 	int rev_nr = 0;
 	int show_all = 0;
 
-	git_config(git_default_config, NULL);
-
-	while (1 < argc && argv[1][0] == '-') {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "-a") || !strcmp(arg, "--all"))
-			show_all = 1;
-		else
-			usage(merge_base_usage);
-		argc--; argv++;
-	}
-	if (argc < 3)
-		usage(merge_base_usage);
-
-	rev = xmalloc((argc - 1) * sizeof(*rev));
-
-	do {
-		rev[rev_nr++] = get_commit_reference(argv[1]);
-		argc--; argv++;
-	} while (argc > 1);
+	struct option options[] = {
+		OPT_BOOLEAN('a', "all", &show_all, "outputs all common ancestors"),
+		OPT_END()
+	};
 
+	git_config(git_default_config, NULL);
+	argc = parse_options(argc, argv, options, merge_base_usage, 0);
+	if (argc < 2)
+		usage_with_options(merge_base_usage, options);
+	rev = xmalloc(argc * sizeof(*rev));
+	while (argc-- > 0)
+		rev[rev_nr++] = get_commit_reference(*argv++);
 	return show_merge_base(rev, rev_nr, show_all);
 }
-- 
1.6.0.2.431.gb53ee

[RESEND PATCH] parse-opt: migrate builtin-merge-file.

[Pierre Habouzit]

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 builtin-merge-file.c |   68 +++++++++++++++++++++++++++----------------------
 1 files changed, 37 insertions(+), 31 deletions(-)

diff --git a/builtin-merge-file.c b/builtin-merge-file.c
index 45c9853..9d4e874 100644
--- a/builtin-merge-file.c
+++ b/builtin-merge-file.c
@@ -2,21 +2,44 @@
 #include "cache.h"
 #include "xdiff/xdiff.h"
 #include "xdiff-interface.h"
+#include "parse-options.h"
 
-static const char merge_file_usage[] =
-"git merge-file [-p | --stdout] [--diff3] [-q | --quiet] [-L name1 [-L orig [-L name2]]] file1 orig_file file2";
+static const char *const merge_file_usage[] = {
+	"git merge-file [options] [-L name1 [-L orig [-L name2]]] file1 orig_file file2",
+	NULL
+};
+
+static int label_cb(const struct option *opt, const char *arg, int unset)
+{
+	static int label_count = 0;
+	const char **names = (const char **)opt->value;
+
+	if (label_count >= 3)
+		return error("too many labels on the command line");
+	names[label_count++] = arg;
+	return 0;
+}
 
 int cmd_merge_file(int argc, const char **argv, const char *prefix)
 {
-	const char *names[3];
+	const char *names[3] = { NULL, NULL, NULL };
 	mmfile_t mmfs[3];
 	mmbuffer_t result = {NULL, 0};
 	xpparam_t xpp = {XDF_NEED_MINIMAL};
 	int ret = 0, i = 0, to_stdout = 0;
 	int merge_level = XDL_MERGE_ZEALOUS_ALNUM;
-	int merge_style = 0;
+	int merge_style = 0, quiet = 0;
 	int nongit;
 
+	struct option options[] = {
+		OPT_BOOLEAN('p', "stdout", &to_stdout, "send results to standard output"),
+		OPT_SET_INT(0, "diff3", &merge_style, "use a diff3 based merge", XDL_MERGE_DIFF3),
+		OPT__QUIET(&quiet),
+		OPT_CALLBACK('L', NULL, names, "name",
+			     "set labels for file1/orig_file/file2", &label_cb),
+		OPT_END(),
+	};
+
 	prefix = setup_git_directory_gently(&nongit);
 	if (!nongit) {
 		/* Read the configuration file */
@@ -25,37 +48,20 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 			merge_style = git_xmerge_style;
 	}
 
-	while (argc > 4) {
-		if (!strcmp(argv[1], "-L") && i < 3) {
-			names[i++] = argv[2];
-			argc--;
-			argv++;
-		} else if (!strcmp(argv[1], "-p") ||
-				!strcmp(argv[1], "--stdout"))
-			to_stdout = 1;
-		else if (!strcmp(argv[1], "-q") ||
-				!strcmp(argv[1], "--quiet"))
-			freopen("/dev/null", "w", stderr);
-		else if (!strcmp(argv[1], "--diff3"))
-			merge_style = XDL_MERGE_DIFF3;
-		else
-			usage(merge_file_usage);
-		argc--;
-		argv++;
-	}
-
-	if (argc != 4)
-		usage(merge_file_usage);
-
-	for (; i < 3; i++)
-		names[i] = argv[i + 1];
+	argc = parse_options(argc, argv, options, merge_file_usage, 0);
+	if (argc != 3)
+		usage_with_options(merge_file_usage, options);
+	if (quiet)
+		freopen("/dev/null", "w", stderr);
 
 	for (i = 0; i < 3; i++) {
-		if (read_mmfile(mmfs + i, argv[i + 1]))
+		if (!names[i])
+			names[i] = argv[i];
+		if (read_mmfile(mmfs + i, argv[i]))
 			return -1;
 		if (buffer_is_binary(mmfs[i].ptr, mmfs[i].size))
 			return error("Cannot merge binary files: %s\n",
-					argv[i + 1]);
+					argv[i]);
 	}
 
 	ret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],
@@ -65,7 +71,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 		free(mmfs[i].ptr);
 
 	if (ret >= 0) {
-		const char *filename = argv[1];
+		const char *filename = argv[0];
 		FILE *f = to_stdout ? stdout : fopen(filename, "wb");
 
 		if (!f)
-- 
1.6.0.2.431.gb53ee

Re: small warm-up: easy parse-opt migrations.

[Sverre Rabbelier]

On Sun, Sep 28, 2008 at 11:45, Pierre Habouzit <madcoder@debian.org> wrote:
> There's not a lot to tell, those are just three migration to
> parse-options.

I think you forgot the -n switch to git format-patch, yes?

-- 
Cheers,

Sverre Rabbelier