Pierre HabouzitThere's not a lot to tell, those are just three migration to parse-options.
threads / discuss / 15695
small warm-up: easy parse-opt migrations.
Subject: small warm-up: easy parse-opt migrations.
Pierre HabouzitPierre Habouzit[PATCH] parse-opt: migrate fmt-merge-msg.
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
Pierre Habouzit[PATCH] parse-opt: migrate git-merge-base.
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
Pierre Habouzit[PATCH] parse-opt: migrate builtin-merge-file.
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
Shawn O. PearceRe: [PATCH] parse-opt: migrate fmt-merge-msg.
Pierre Habouzit <madcoder@debian.org> wrote:
Show 6 quoted lines
> 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.
Pierre HabouzitRe: [PATCH] parse-opt: migrate fmt-merge-msg.
On Mon, Sep 29, 2008 at 04:35:23PM +0000, Shawn O. Pearce wrote:
Show 13 quoted lines
> 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
Shawn O. PearceRe: [PATCH] parse-opt: migrate fmt-merge-msg.
Pierre Habouzit <madcoder@debian.org> wrote:
Show 9 quoted lines
> 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.
Pierre HabouzitRe: [PATCH] parse-opt: migrate fmt-merge-msg.
On Tue, Sep 30, 2008 at 07:10:14PM +0000, Shawn O. Pearce wrote:
Show 29 quoted lines
> 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
Shawn O. PearceRe: [PATCH] parse-opt: migrate fmt-merge-msg.
Pierre Habouzit <madcoder@debian.org> wrote:
Show 12 quoted lines
> 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.
Pierre Habouzit[PATCH 1/3] parse-opt: migrate fmt-merge-msg.
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
Pierre Habouzit[PATCH 2/3] parse-opt: migrate git-merge-base.
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
Pierre Habouzit[PATCH 3/3] parse-opt: migrate builtin-merge-file.
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
Shawn O. PearceRe: [PATCH 3/3] parse-opt: migrate builtin-merge-file.
Pierre Habouzit <madcoder@debian.org> wrote:
Show 5 quoted lines
> 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 @@
...
Show 7 quoted lines
> +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.
Shawn O. PearceRe: [PATCH 1/3] parse-opt: migrate fmt-merge-msg.
Pierre Habouzit <madcoder@debian.org> wrote:
Show 10 quoted lines
> @@ -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()Show 11 quoted lines
> + 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.
Pierre HabouzitRe: [PATCH 1/3] parse-opt: migrate fmt-merge-msg.
On Wed, Oct 01, 2008 at 02:56:06PM +0000, Shawn O. Pearce wrote:
Show 26 quoted lines
> 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
Pierre Habouzit[RESEND PATCH] parse-opt: migrate fmt-merge-msg.
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
Pierre Habouzit[PATCH] parse-opt: migrate git-merge-base.
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
Pierre Habouzit[RESEND PATCH] parse-opt: migrate builtin-merge-file.
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.
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