[PATCH 1/3] show-branch: Fix die message in parse_reflog_param()

[PATCH 1/3] show-branch: Fix die message in parse_reflog_param()

[Stephen Boyd]

Commit 76a44c5 (show-branch --reflog: show the reflog message at the
top, 2007-01-19) introduced parse_reflog_param(). The die() call was
incorrectly passed arg + 9, when it should have been passed arg.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 builtin-show-branch.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 828e6f8..c3afabb 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -576,7 +576,7 @@ static void parse_reflog_param(const char *arg, int *cnt, const char **base)
 	if (*ep == ',')
 		*base = ep + 1;
 	else if (*ep)
-		die("unrecognized reflog param '%s'", arg + 9);
+		die("unrecognized reflog param '%s'", arg);
 	else
 		*base = NULL;
 	if (*cnt <= 0)
-- 
1.6.3.1.30.g55524

[PATCH 2/3] parse-options: add PARSE_OPT_CUSTOM_ARGH for complicated argh's

[Stephen Boyd]

If argh is complicated, i.e. the option takes more than one argument,
don't add the brackets around argh in the usage message.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---

    This is used in the next patch to for --reflog[=<n>[,<base>]]

    Maybe someone can think of a better name?

 parse-options.c |   26 +++++++++++++++++---------
 parse-options.h |    4 ++++
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index cf71bcf..cd9ab4f 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -361,6 +361,20 @@ int parse_options(int argc, const char **argv, const struct option *options,
 	return parse_options_end(&ctx);
 }
 
+static int usage_argh(const struct option *opts, int pos)
+{
+	const char *s;
+	int custom = opts->flags & PARSE_OPT_CUSTOM_ARGH;
+	if (opts->flags & PARSE_OPT_OPTARG)
+		if (opts->long_name)
+			s = custom ? "[=%s]" : "[=<%s>]";
+		else
+			s = custom ? "[%s]" : "[<%s>]";
+	else
+		s = custom ? " %s" : " <%s>";
+	return pos + fprintf(stderr, s, opts->argh);
+}
+
 #define USAGE_OPTS_WIDTH 24
 #define USAGE_GAP         2
 
@@ -421,15 +435,9 @@ int usage_with_options_internal(const char * const *usagestr,
 				break;
 			/* FALLTHROUGH */
 		case OPTION_STRING:
-			if (opts->argh) {
-				if (opts->flags & PARSE_OPT_OPTARG)
-					if (opts->long_name)
-						pos += fprintf(stderr, "[=<%s>]", opts->argh);
-					else
-						pos += fprintf(stderr, "[<%s>]", opts->argh);
-				else
-					pos += fprintf(stderr, " <%s>", opts->argh);
-			} else {
+			if (opts->argh)
+				pos += usage_argh(opts, pos);
+			else {
 				if (opts->flags & PARSE_OPT_OPTARG)
 					if (opts->long_name)
 						pos += fprintf(stderr, "[=...]");
diff --git a/parse-options.h b/parse-options.h
index b54eec1..671a635 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -31,6 +31,7 @@ enum parse_opt_option_flags {
 	PARSE_OPT_NONEG   = 4,
 	PARSE_OPT_HIDDEN  = 8,
 	PARSE_OPT_LASTARG_DEFAULT = 16,
+	PARSE_OPT_CUSTOM_ARGH = 64,
 };
 
 struct option;
@@ -66,6 +67,9 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
  *   PARSE_OPT_NONEG: says that this option cannot be negated
  *   PARSE_OPT_HIDDEN this option is skipped in the default usage, showed in
  *                    the long one.
+ *   PARSE_OPT_CUSTOM_ARGH: says that argh shouldn't be enclosed in brackets
+ *			    (i.e. '<argh>') in the help message.
+ *			    Useful for options with multiple parameters.
  *
  * `callback`::
  *   pointer to the callback to use for OPTION_CALLBACK.
-- 
1.6.3.1.30.g55524

[PATCH 3/3] show-branch: migrate to parse-options API

[Stephen Boyd]

Also die instead of calling usage with an error string.

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 builtin-show-branch.c |  117 +++++++++++++++++++++++++------------------------
 1 files changed, 60 insertions(+), 57 deletions(-)

diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index c3afabb..e9acd4c 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -2,11 +2,13 @@
 #include "commit.h"
 #include "refs.h"
 #include "builtin.h"
+#include "parse-options.h"
 
-static const char show_branch_usage[] =
-"git show-branch [--sparse] [--current] [--all] [--remotes] [--topo-order] [--more=count | --list | --independent | --merge-base ] [--topics] [<refs>...] | --reflog[=n[,b]] <branch>";
-static const char show_branch_usage_reflog[] =
-"--reflog is incompatible with --all, --remotes, --independent or --merge-base";
+static const char* show_branch_usage[] = {
+    "git show-branch [--sparse] [--current] [--all] [--remotes] [--topo-order] [--more=count | --list | --independent | --merge-base] [--topics] [<refs>...]",
+    "--reflog[=n[,b]] [--list] <branch>",
+    NULL
+};
 
 static int default_num;
 static int default_alloc;
@@ -569,18 +571,25 @@ static int omit_in_dense(struct commit *commit, struct commit **rev, int n)
 	return 0;
 }
 
-static void parse_reflog_param(const char *arg, int *cnt, const char **base)
+static int reflog = 0;
+
+static int parse_reflog_param(const struct option *opt, const char *arg,
+			      int unset)
 {
 	char *ep;
-	*cnt = strtoul(arg, &ep, 10);
+	const char **base = (const char **)opt->value;
+	if (!arg)
+		arg = "";
+	reflog = strtoul(arg, &ep, 10);
 	if (*ep == ',')
 		*base = ep + 1;
 	else if (*ep)
 		die("unrecognized reflog param '%s'", arg);
 	else
 		*base = NULL;
-	if (*cnt <= 0)
-		*cnt = DEFAULT_REFLOG;
+	if (reflog <= 0)
+		reflog = DEFAULT_REFLOG;
+	return 0;
 }
 
 int cmd_show_branch(int ac, const char **av, const char *prefix)
@@ -606,8 +615,42 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 	int head_at = -1;
 	int topics = 0;
 	int dense = 1;
-	int reflog = 0;
 	const char *reflog_base = NULL;
+	struct option builtin_show_branch_options[] = {
+		OPT_BOOLEAN('a', "all", &all_heads,
+			    "show remote-tracking and local branches"),
+		OPT_BOOLEAN('r', "remotes", &all_remotes,
+			    "show remote-tracking branches"),
+		{ OPTION_INTEGER, 0, "more", &extra, "n",
+			    "show <n> more commits after the common ancestor",
+			    PARSE_OPT_OPTARG | PARSE_OPT_LASTARG_DEFAULT,
+			    NULL, (intptr_t)1 },
+		OPT_SET_INT(0, "list", &extra, "synonym to more=-1", -1),
+		OPT_BOOLEAN(0, "no-name", &no_name, "suppress naming strings"),
+		OPT_BOOLEAN(0, "current", &with_current_branch,
+			    "include the current branch"),
+		OPT_BOOLEAN(0, "sha1-name", &sha1_name,
+			    "name commits with their object names"),
+		OPT_BOOLEAN(0, "merge-base", &merge_base,
+			    "act like git merge-base -a"),
+		OPT_BOOLEAN(0, "independent", &independent,
+			    "show refs unreachable from any other ref"),
+		OPT_BOOLEAN(0, "topo-order", &lifo,
+			    "show commits in topological order"),
+		OPT_BOOLEAN(0, "topics", &topics,
+			    "show only commits not on the first branch"),
+		OPT_SET_INT(0, "sparse", &dense,
+			    "show merges reachable from only one tip", 0),
+		OPT_SET_INT(0, "date-order", &lifo,
+			    "show commits where no parent comes before its "
+			    "children", 0),
+		{ OPTION_CALLBACK, 'g', "reflog", &reflog_base, "<n>[,<base>]",
+			    "show <n> most recent ref-log entries starting at "
+			    "base",
+			    PARSE_OPT_OPTARG | PARSE_OPT_CUSTOM_ARGH,
+			    parse_reflog_param },
+		OPT_END()
+	};
 
 	git_config(git_show_branch_config, NULL);
 
@@ -617,59 +660,18 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 		av = default_arg - 1; /* ick; we would not address av[0] */
 	}
 
-	while (1 < ac && av[1][0] == '-') {
-		const char *arg = av[1];
-		if (!strcmp(arg, "--")) {
-			ac--; av++;
-			break;
-		}
-		else if (!strcmp(arg, "--all") || !strcmp(arg, "-a"))
-			all_heads = all_remotes = 1;
-		else if (!strcmp(arg, "--remotes") || !strcmp(arg, "-r"))
-			all_remotes = 1;
-		else if (!strcmp(arg, "--more"))
-			extra = 1;
-		else if (!strcmp(arg, "--list"))
-			extra = -1;
-		else if (!strcmp(arg, "--no-name"))
-			no_name = 1;
-		else if (!strcmp(arg, "--current"))
-			with_current_branch = 1;
-		else if (!strcmp(arg, "--sha1-name"))
-			sha1_name = 1;
-		else if (!prefixcmp(arg, "--more="))
-			extra = atoi(arg + 7);
-		else if (!strcmp(arg, "--merge-base"))
-			merge_base = 1;
-		else if (!strcmp(arg, "--independent"))
-			independent = 1;
-		else if (!strcmp(arg, "--topo-order"))
-			lifo = 1;
-		else if (!strcmp(arg, "--topics"))
-			topics = 1;
-		else if (!strcmp(arg, "--sparse"))
-			dense = 0;
-		else if (!strcmp(arg, "--date-order"))
-			lifo = 0;
-		else if (!strcmp(arg, "--reflog") || !strcmp(arg, "-g")) {
-			reflog = DEFAULT_REFLOG;
-		}
-		else if (!prefixcmp(arg, "--reflog="))
-			parse_reflog_param(arg + 9, &reflog, &reflog_base);
-		else if (!prefixcmp(arg, "-g="))
-			parse_reflog_param(arg + 3, &reflog, &reflog_base);
-		else
-			usage(show_branch_usage);
-		ac--; av++;
-	}
-	ac--; av++;
+	ac = parse_options(ac, av, builtin_show_branch_options,
+			   show_branch_usage, PARSE_OPT_STOP_AT_NON_OPTION);
+	if (all_heads)
+		all_remotes = 1;
 
 	if (extra || reflog) {
 		/* "listing" mode is incompatible with
 		 * independent nor merge-base modes.
 		 */
 		if (independent || merge_base)
-			usage(show_branch_usage);
+			usage_with_options(show_branch_usage,
+					   builtin_show_branch_options);
 		if (reflog && ((0 < extra) || all_heads || all_remotes))
 			/*
 			 * Asking for --more in reflog mode does not
@@ -677,7 +679,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 			 *
 			 * Also --all and --remotes do not make sense either.
 			 */
-			usage(show_branch_usage_reflog);
+			die("--reflog is incompatible with --all, --remotes, "
+			    "--independent or --merge-base");
 	}
 
 	/* If nothing is specified, show all branches by default */
-- 
1.6.3.1.30.g55524

Re: [PATCH 2/3] parse-options: add PARSE_OPT_CUSTOM_ARGH for complicated argh's

[Junio C Hamano]

Stephen Boyd <bebarino@gmail.com> writes:

> If argh is complicated, i.e. the option takes more than one argument,
> don't add the brackets around argh in the usage message.

I think later user wants a bit more descriptive explanation, like...

	Usually, argh element in struct option points at a placeholder
	value (e.g. "val"), and this is used to show

	    --option=<val>

	by enclosing the string inside of angle brackets.

	When the option takes something more complex (e.g. optional part
	separated by comma), you would want to produce a help that looks
	like

	    --option=<val1>[,<val2>]

	In such a case, the caller can pass a string to argh with
	placeholders already enclosed in necessary angle brackets
	(e.g. "<val1>[,<val2>]") and set this option.

Please update Documentation/technical/api-parse-options.txt as well.

Re: [PATCH 2/3] parse-options: add PARSE_OPT_CUSTOM_ARGH for complicated argh's

[Stephen Boyd]

Junio C Hamano wrote:
> Stephen Boyd <bebarino@gmail.com> writes:
>> If argh is complicated, i.e. the option takes more than one argument,
>> don't add the brackets around argh in the usage message.
>
> I think later user wants a bit more descriptive explanation, like...
>
> 	Usually, argh element in struct option points at a placeholder
> 	value (e.g. "val"), and this is used to show
>
> 	    --option=<val>
>
> 	by enclosing the string inside of angle brackets.
>
> 	When the option takes something more complex (e.g. optional part
> 	separated by comma), you would want to produce a help that looks
> 	like
>
> 	    --option=<val1>[,<val2>]
>
> 	In such a case, the caller can pass a string to argh with
> 	placeholders already enclosed in necessary angle brackets
> 	(e.g. "<val1>[,<val2>]") and set this option.

This description sounds nice ;) Is PARSE_OPT_CUSTOM_ARGH a good name? I
was thinking maybe PARSE_OPT_MULTARGS is better?

> Please update Documentation/technical/api-parse-options.txt as well.

There's no documentation on the flags yet, but I suppose I could add that.

Re: [PATCH 2/3] parse-options: add PARSE_OPT_CUSTOM_ARGH for complicated argh's

[Junio C Hamano]

Stephen Boyd <bebarino@gmail.com> writes:

> This description sounds nice ;) Is PARSE_OPT_CUSTOM_ARGH a good name? I
> was thinking maybe PARSE_OPT_MULTARGS is better?

You are asking a wrong person.  I am terrible at naming things; think
"rerere" ;-).

But I think custom-arghelp is much better than multargs.  You are asking
"use this help for argument value _literally_", so another possibility
perhaps is PARSE_OPT_ARGHELP_LITERAL.

After all, there probably are many other valid reasons why you may not
want "s/.*/<&>/" blindly applied to your string.  One reason may be
because the string describes multiple arguments, but I suspect that it is
not the only one.  It is better to name the option after what it does,
than naming it after one sample reason why you might want to use that
option.

>> Please update Documentation/technical/api-parse-options.txt as well.
>
> There's no documentation on the flags yet, but I suppose I could add that.

Thanks.

Re: [PATCH 2/3] parse-options: add PARSE_OPT_CUSTOM_ARGH for complicated argh's

[Stephen Boyd]

Junio C Hamano wrote:
> You are asking a wrong person.  I am terrible at naming things; think
> "rerere" ;-).
>
> But I think custom-arghelp is much better than multargs.  You are asking
> "use this help for argument value _literally_", so another possibility
> perhaps is PARSE_OPT_ARGHELP_LITERAL.
>
> After all, there probably are many other valid reasons why you may not
> want "s/.*/<&>/" blindly applied to your string.  One reason may be
> because the string describes multiple arguments, but I suspect that it is
> not the only one.  It is better to name the option after what it does,
> than naming it after one sample reason why you might want to use that
> option.

Ok. I think PARSE_OPT_LITERAL_ARGH might be good; until someone comes up
with a better one of course.

Re: [PATCH 2/3] parse-options: add PARSE_OPT_CUSTOM_ARGH for complicated argh's

[Jakub Narebski]

Stephen Boyd <bebarino@gmail.com> writes:
> Junio C Hamano wrote:

> > You are asking a wrong person.  I am terrible at naming things; think
> > "rerere" ;-).
> >
> > But I think custom-arghelp is much better than multargs.  You are asking
> > "use this help for argument value _literally_", so another possibility
> > perhaps is PARSE_OPT_ARGHELP_LITERAL.
> >
> > After all, there probably are many other valid reasons why you may not
> > want "s/.*/<&>/" blindly applied to your string.  One reason may be
> > because the string describes multiple arguments, but I suspect that it is
> > not the only one.  It is better to name the option after what it does,
> > than naming it after one sample reason why you might want to use that
> > option.
> 
> Ok. I think PARSE_OPT_LITERAL_ARGH might be good; until someone comes up
> with a better one of course.

Well, 'ARGH' sounds a bit strange. I think I'd prefer ARGHELP. :-)
-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH 2/3] parse-options: add PARSE_OPT_CUSTOM_ARGH for complicated argh's

[Sverre Rabbelier]

Heya,

On Tue, May 19, 2009 at 00:52, Jakub Narebski <jnareb@gmail.com> wrote:
> Well, 'ARGH' sounds a bit strange. I think I'd prefer ARGHELP. :-)

Yarr, it be reminding me of a patch from ye ol' mentor summit!

-- 
Cheers,

Sverre Rabbelier

[PATCHv2 1/2] parse-options: add PARSE_OPT_LITERAL_ARGHELP for complicated argh's

[Stephen Boyd]

Usually, the argh element in struct option points at a placeholder value
(e.g. "val"), and is shown in the usage message as

    --option=<val>

by enclosing the string inside of angle brackets.

When the option is more complex (e.g. optional arguments separated by a
comma), you would want to produce a usage message that looks like

    --option=<val1>[,<val2>]

In such a case, the caller can pass a string to argh with placeholders
already enclosed in necessary angle brackets (e.g.  "<val1>[,<val2>]")
and set this flag.

[nicer description from Junio Hamano]

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---

I've decided to appease the pirate haters :-)

 parse-options.c |   26 +++++++++++++++++---------
 parse-options.h |    4 ++++
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index cf71bcf..2b880b1 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -361,6 +361,20 @@ int parse_options(int argc, const char **argv, const struct option *options,
 	return parse_options_end(&ctx);
 }
 
+static int usage_argh(const struct option *opts, int pos)
+{
+	const char *s;
+	int literal = opts->flags & PARSE_OPT_LITERAL_ARGHELP;
+	if (opts->flags & PARSE_OPT_OPTARG)
+		if (opts->long_name)
+			s = literal ? "[=%s]" : "[=<%s>]";
+		else
+			s = literal ? "[%s]" : "[<%s>]";
+	else
+		s = literal ? " %s" : " <%s>";
+	return pos + fprintf(stderr, s, opts->argh);
+}
+
 #define USAGE_OPTS_WIDTH 24
 #define USAGE_GAP         2
 
@@ -421,15 +435,9 @@ int usage_with_options_internal(const char * const *usagestr,
 				break;
 			/* FALLTHROUGH */
 		case OPTION_STRING:
-			if (opts->argh) {
-				if (opts->flags & PARSE_OPT_OPTARG)
-					if (opts->long_name)
-						pos += fprintf(stderr, "[=<%s>]", opts->argh);
-					else
-						pos += fprintf(stderr, "[<%s>]", opts->argh);
-				else
-					pos += fprintf(stderr, " <%s>", opts->argh);
-			} else {
+			if (opts->argh)
+				pos += usage_argh(opts, pos);
+			else {
 				if (opts->flags & PARSE_OPT_OPTARG)
 					if (opts->long_name)
 						pos += fprintf(stderr, "[=...]");
diff --git a/parse-options.h b/parse-options.h
index b54eec1..910aa1e 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -31,6 +31,7 @@ enum parse_opt_option_flags {
 	PARSE_OPT_NONEG   = 4,
 	PARSE_OPT_HIDDEN  = 8,
 	PARSE_OPT_LASTARG_DEFAULT = 16,
+	PARSE_OPT_LITERAL_ARGHELP = 64,
 };
 
 struct option;
@@ -66,6 +67,9 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
  *   PARSE_OPT_NONEG: says that this option cannot be negated
  *   PARSE_OPT_HIDDEN this option is skipped in the default usage, showed in
  *                    the long one.
+ *   PARSE_OPT_LITERAL_ARGHELP: says that argh shouldn't be enclosed in brackets
+ *				(i.e. '<argh>') in the help message.
+ *				Useful for options with multiple parameters.
  *
  * `callback`::
  *   pointer to the callback to use for OPTION_CALLBACK.
-- 
1.6.3.1.61.g065b0

Re: [PATCHv2 1/2] parse-options: add PARSE_OPT_LITERAL_ARGHELP for complicated argh's

[Junio C Hamano]

Stephen Boyd <bebarino@gmail.com> writes:

> I've decided to appease the pirate haters :-)

Hmmm, why does this break t0040 (I am queuing this on top of 5acb3e5)?

Re: [PATCHv2 1/2] parse-options: add PARSE_OPT_LITERAL_ARGHELP for complicated argh's

[René Scharfe]

Junio C Hamano schrieb:
> Stephen Boyd <bebarino@gmail.com> writes:
> 
>> I've decided to appease the pirate haters :-)
> 
> Hmmm, why does this break t0040 (I am queuing this on top of 5acb3e5)?

Probably because it changes this:

	pos += fprintf(...);

into this (simplified, usage_argh() expanded):

	pos += pos + fprintf(...);

usage_argh() doesn't need the parameter pos.

René

Re: [PATCHv2 1/2] parse-options: add PARSE_OPT_LITERAL_ARGHELP for complicated argh's

[Stephen Boyd]

On Thu, May 21, 2009 at 9:51 AM, René Scharfe
<rene.scharfe@lsrfire.ath.cx> wrote:
> Junio C Hamano schrieb:
>> Hmmm, why does this break t0040 (I am queuing this on top of 5acb3e5)?
>
> Probably because it changes this:
>
>        pos += fprintf(...);
>
> into this (simplified, usage_argh() expanded):
>
>        pos += pos + fprintf(...);
>
> usage_argh() doesn't need the parameter pos.
>
> René
>

Woops. I thought I ran the tests but I guess I didn't. This is the
correct fix, thanks.

Re: [PATCHv2 1/2] parse-options: add PARSE_OPT_LITERAL_ARGHELP for complicated argh's

[Stephen Boyd]

Stephen Boyd wrote:
> On Thu, May 21, 2009 at 9:51 AM, René Scharfe
> <rene.scharfe@lsrfire.ath.cx> wrote:
>> Junio C Hamano schrieb:
>>> Hmmm, why does this break t0040 (I am queuing this on top of 5acb3e5)?
>> Probably because it changes this:
>>
>> Â  Â  Â  Â pos += fprintf(...);
>>
>> into this (simplified, usage_argh() expanded):
>>
>> Â  Â  Â  Â pos += pos + fprintf(...);
>>
>> usage_argh() doesn't need the parameter pos.
>>
>> René
>>
>
> Woops. I thought I ran the tests but I guess I didn't. This is the
> correct fix, thanks.

And here's the patch you can squash in.

diff --git a/parse-options.c b/parse-options.c
index 2b880b1..e8955be 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -361,7 +361,7 @@ int parse_options(int argc, const char **argv, const struct option *options,
        return parse_options_end(&ctx);
 }

-static int usage_argh(const struct option *opts, int pos)
+static int usage_argh(const struct option *opts)
 {
        const char *s;
        int literal = opts->flags & PARSE_OPT_LITERAL_ARGHELP;
@@ -372,7 +372,7 @@ static int usage_argh(const struct option *opts, int pos)
                        s = literal ? "[%s]" : "[<%s>]";
        else
                s = literal ? " %s" : " <%s>";
-       return pos + fprintf(stderr, s, opts->argh);
+       return fprintf(stderr, s, opts->argh);
 }

 #define USAGE_OPTS_WIDTH 24
@@ -436,7 +436,7 @@ int usage_with_options_internal(const char * const *usagestr,
                        /* FALLTHROUGH */
                case OPTION_STRING:
                        if (opts->argh)
-                               pos += usage_argh(opts, pos);
+                               pos += usage_argh(opts);
                        else {
                                if (opts->flags & PARSE_OPT_OPTARG)
                                        if (opts->long_name)

[PATCHv2 2/2] show-branch: migrate to parse-options API

[Stephen Boyd]

Note that "-g" no longer uses an equals '=' sign for its optional
arguments, but "--reflog" still does. This is normal behavior for parse
options, as arguments to "-g" are put immediately after the option with
no space.

For example

    git show-branch -g=4

is now

    git show-branch -g4

Signed-off-by: Stephen Boyd <bebarino@gmail.com>
---
 builtin-show-branch.c |  125 +++++++++++++++++++++++++------------------------
 1 files changed, 63 insertions(+), 62 deletions(-)

diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index c8e9b3c..b1affd2 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -3,11 +3,13 @@
 #include "refs.h"
 #include "builtin.h"
 #include "color.h"
+#include "parse-options.h"
 
-static const char show_branch_usage[] =
-"git show-branch [--sparse] [--current] [--all] [--remotes] [--topo-order] [--more=count | --list | --independent | --merge-base ] [--topics] [<refs>...] | --reflog[=n[,b]] <branch>";
-static const char show_branch_usage_reflog[] =
-"--reflog is incompatible with --all, --remotes, --independent or --merge-base";
+static const char* show_branch_usage[] = {
+    "git show-branch [--sparse] [--current] [--all] [--remotes] [--topo-order] [--more=count | --list | --independent | --merge-base] [--topics] [--color] [<refs>...]",
+    "--reflog[=n[,b]] [--list] [--color] <branch>",
+    NULL
+};
 
 static int showbranch_use_color = -1;
 static char column_colors[][COLOR_MAXLEN] = {
@@ -601,18 +603,25 @@ static int omit_in_dense(struct commit *commit, struct commit **rev, int n)
 	return 0;
 }
 
-static void parse_reflog_param(const char *arg, int *cnt, const char **base)
+static int reflog = 0;
+
+static int parse_reflog_param(const struct option *opt, const char *arg,
+			      int unset)
 {
 	char *ep;
-	*cnt = strtoul(arg, &ep, 10);
+	const char **base = (const char **)opt->value;
+	if (!arg)
+		arg = "";
+	reflog = strtoul(arg, &ep, 10);
 	if (*ep == ',')
 		*base = ep + 1;
 	else if (*ep)
-		die("unrecognized reflog param '%s'", arg);
+		return error("unrecognized reflog param '%s'", arg);
 	else
 		*base = NULL;
-	if (*cnt <= 0)
-		*cnt = DEFAULT_REFLOG;
+	if (reflog <= 0)
+		reflog = DEFAULT_REFLOG;
+	return 0;
 }
 
 int cmd_show_branch(int ac, const char **av, const char *prefix)
@@ -638,8 +647,44 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 	int head_at = -1;
 	int topics = 0;
 	int dense = 1;
-	int reflog = 0;
 	const char *reflog_base = NULL;
+	struct option builtin_show_branch_options[] = {
+		OPT_BOOLEAN('a', "all", &all_heads,
+			    "show remote-tracking and local branches"),
+		OPT_BOOLEAN('r', "remotes", &all_remotes,
+			    "show remote-tracking branches"),
+		OPT_BOOLEAN(0, "color", &showbranch_use_color,
+			    "color '*!+-' corresponding to the branch"),
+		{ OPTION_INTEGER, 0, "more", &extra, "n",
+			    "show <n> more commits after the common ancestor",
+			    PARSE_OPT_OPTARG | PARSE_OPT_LASTARG_DEFAULT,
+			    NULL, (intptr_t)1 },
+		OPT_SET_INT(0, "list", &extra, "synonym to more=-1", -1),
+		OPT_BOOLEAN(0, "no-name", &no_name, "suppress naming strings"),
+		OPT_BOOLEAN(0, "current", &with_current_branch,
+			    "include the current branch"),
+		OPT_BOOLEAN(0, "sha1-name", &sha1_name,
+			    "name commits with their object names"),
+		OPT_BOOLEAN(0, "merge-base", &merge_base,
+			    "act like git merge-base -a"),
+		OPT_BOOLEAN(0, "independent", &independent,
+			    "show refs unreachable from any other ref"),
+		OPT_BOOLEAN(0, "topo-order", &lifo,
+			    "show commits in topological order"),
+		OPT_BOOLEAN(0, "topics", &topics,
+			    "show only commits not on the first branch"),
+		OPT_SET_INT(0, "sparse", &dense,
+			    "show merges reachable from only one tip", 0),
+		OPT_SET_INT(0, "date-order", &lifo,
+			    "show commits where no parent comes before its "
+			    "children", 0),
+		{ OPTION_CALLBACK, 'g', "reflog", &reflog_base, "<n>[,<base>]",
+			    "show <n> most recent ref-log entries starting at "
+			    "base",
+			    PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,
+			    parse_reflog_param },
+		OPT_END()
+	};
 
 	git_config(git_show_branch_config, NULL);
 
@@ -652,63 +697,18 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 		av = default_arg - 1; /* ick; we would not address av[0] */
 	}
 
-	while (1 < ac && av[1][0] == '-') {
-		const char *arg = av[1];
-		if (!strcmp(arg, "--")) {
-			ac--; av++;
-			break;
-		}
-		else if (!strcmp(arg, "--all") || !strcmp(arg, "-a"))
-			all_heads = all_remotes = 1;
-		else if (!strcmp(arg, "--remotes") || !strcmp(arg, "-r"))
-			all_remotes = 1;
-		else if (!strcmp(arg, "--more"))
-			extra = 1;
-		else if (!strcmp(arg, "--list"))
-			extra = -1;
-		else if (!strcmp(arg, "--no-name"))
-			no_name = 1;
-		else if (!strcmp(arg, "--current"))
-			with_current_branch = 1;
-		else if (!strcmp(arg, "--sha1-name"))
-			sha1_name = 1;
-		else if (!prefixcmp(arg, "--more="))
-			extra = atoi(arg + 7);
-		else if (!strcmp(arg, "--merge-base"))
-			merge_base = 1;
-		else if (!strcmp(arg, "--independent"))
-			independent = 1;
-		else if (!strcmp(arg, "--topo-order"))
-			lifo = 1;
-		else if (!strcmp(arg, "--topics"))
-			topics = 1;
-		else if (!strcmp(arg, "--sparse"))
-			dense = 0;
-		else if (!strcmp(arg, "--date-order"))
-			lifo = 0;
-		else if (!strcmp(arg, "--reflog") || !strcmp(arg, "-g")) {
-			reflog = DEFAULT_REFLOG;
-		}
-		else if (!prefixcmp(arg, "--reflog="))
-			parse_reflog_param(arg + 9, &reflog, &reflog_base);
-		else if (!prefixcmp(arg, "-g="))
-			parse_reflog_param(arg + 3, &reflog, &reflog_base);
-		else if (!strcmp(arg, "--color"))
-			showbranch_use_color = 1;
-		else if (!strcmp(arg, "--no-color"))
-			showbranch_use_color = 0;
-		else
-			usage(show_branch_usage);
-		ac--; av++;
-	}
-	ac--; av++;
+	ac = parse_options(ac, av, builtin_show_branch_options,
+			   show_branch_usage, PARSE_OPT_STOP_AT_NON_OPTION);
+	if (all_heads)
+		all_remotes = 1;
 
 	if (extra || reflog) {
 		/* "listing" mode is incompatible with
 		 * independent nor merge-base modes.
 		 */
 		if (independent || merge_base)
-			usage(show_branch_usage);
+			usage_with_options(show_branch_usage,
+					   builtin_show_branch_options);
 		if (reflog && ((0 < extra) || all_heads || all_remotes))
 			/*
 			 * Asking for --more in reflog mode does not
@@ -716,7 +716,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 			 *
 			 * Also --all and --remotes do not make sense either.
 			 */
-			usage(show_branch_usage_reflog);
+			die("--reflog is incompatible with --all, --remotes, "
+			    "--independent or --merge-base");
 	}
 
 	/* If nothing is specified, show all branches by default */
-- 
1.6.3.1.61.g065b0