{"thread":{"id":"19428","subject":"[PATCHv2 2/2] show-branch: migrate to parse-options API","startedAt":"2009-05-21T07:33:17Z","lastAt":"2009-05-21T21:27:25Z","messageCount":6,"participants":["Stephen Boyd","Junio C Hamano","René Scharfe"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"114396","messageId":"fbaa26d941f496efefc30e298f5534f8194ee716.1242891030.git.bebarino@gmail.com","threadId":"19428","inReplyTo":"1242557224-8411-1-git-send-email-bebarino@gmail.com","subject":"[PATCHv2 1/2] parse-options: add PARSE_OPT_LITERAL_ARGHELP for complicated argh's","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2009-05-21T07:33:17Z","receivedAt":"2009-05-21T07:33:17Z","isPatch":false,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"Usually, the argh element in struct option points at a placeholder value\n(e.g. \"val\"), and is shown in the usage message as\n\n    --option=<val>\n\nby enclosing the string inside of angle brackets.\n\nWhen the option is more complex (e.g. optional arguments separated by a\ncomma), you would want to produce a usage message that looks like\n\n    --option=<val1>[,<val2>]\n\nIn such a case, the caller can pass a string to argh with placeholders\nalready enclosed in necessary angle brackets (e.g.  \"<val1>[,<val2>]\")\nand set this flag.\n\n[nicer description from Junio Hamano]\n\nSigned-off-by: Stephen Boyd <bebarino@gmail.com>\n---\n\nI've decided to appease the pirate haters :-)\n\n parse-options.c |   26 +++++++++++++++++---------\n parse-options.h |    4 ++++\n 2 files changed, 21 insertions(+), 9 deletions(-)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex cf71bcf..2b880b1 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -361,6 +361,20 @@ int parse_options(int argc, const char **argv, const struct option *options,\n \treturn parse_options_end(&ctx);\n }\n \n+static int usage_argh(const struct option *opts, int pos)\n+{\n+\tconst char *s;\n+\tint literal = opts->flags & PARSE_OPT_LITERAL_ARGHELP;\n+\tif (opts->flags & PARSE_OPT_OPTARG)\n+\t\tif (opts->long_name)\n+\t\t\ts = literal ? \"[=%s]\" : \"[=<%s>]\";\n+\t\telse\n+\t\t\ts = literal ? \"[%s]\" : \"[<%s>]\";\n+\telse\n+\t\ts = literal ? \" %s\" : \" <%s>\";\n+\treturn pos + fprintf(stderr, s, opts->argh);\n+}\n+\n #define USAGE_OPTS_WIDTH 24\n #define USAGE_GAP         2\n \n@@ -421,15 +435,9 @@ int usage_with_options_internal(const char * const *usagestr,\n \t\t\t\tbreak;\n \t\t\t/* FALLTHROUGH */\n \t\tcase OPTION_STRING:\n-\t\t\tif (opts->argh) {\n-\t\t\t\tif (opts->flags & PARSE_OPT_OPTARG)\n-\t\t\t\t\tif (opts->long_name)\n-\t\t\t\t\t\tpos += fprintf(stderr, \"[=<%s>]\", opts->argh);\n-\t\t\t\t\telse\n-\t\t\t\t\t\tpos += fprintf(stderr, \"[<%s>]\", opts->argh);\n-\t\t\t\telse\n-\t\t\t\t\tpos += fprintf(stderr, \" <%s>\", opts->argh);\n-\t\t\t} else {\n+\t\t\tif (opts->argh)\n+\t\t\t\tpos += usage_argh(opts, pos);\n+\t\t\telse {\n \t\t\t\tif (opts->flags & PARSE_OPT_OPTARG)\n \t\t\t\t\tif (opts->long_name)\n \t\t\t\t\t\tpos += fprintf(stderr, \"[=...]\");\ndiff --git a/parse-options.h b/parse-options.h\nindex b54eec1..910aa1e 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -31,6 +31,7 @@ enum parse_opt_option_flags {\n \tPARSE_OPT_NONEG   = 4,\n \tPARSE_OPT_HIDDEN  = 8,\n \tPARSE_OPT_LASTARG_DEFAULT = 16,\n+\tPARSE_OPT_LITERAL_ARGHELP = 64,\n };\n \n struct option;\n@@ -66,6 +67,9 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);\n  *   PARSE_OPT_NONEG: says that this option cannot be negated\n  *   PARSE_OPT_HIDDEN this option is skipped in the default usage, showed in\n  *                    the long one.\n+ *   PARSE_OPT_LITERAL_ARGHELP: says that argh shouldn't be enclosed in brackets\n+ *\t\t\t\t(i.e. '<argh>') in the help message.\n+ *\t\t\t\tUseful for options with multiple parameters.\n  *\n  * `callback`::\n  *   pointer to the callback to use for OPTION_CALLBACK.\n-- \n1.6.3.1.61.g065b0\n"},{"id":"114395","messageId":"fc53e39984e7fabbd53fa23a7d7f24625e685d2c.1242891030.git.bebarino@gmail.com","threadId":"19428","inReplyTo":"fbaa26d941f496efefc30e298f5534f8194ee716.1242891030.git.bebarino@gmail.com","subject":"[PATCHv2 2/2] show-branch: migrate to parse-options API","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2009-05-21T07:33:18Z","receivedAt":"2009-05-21T07:33:18Z","isPatch":false,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"Note that \"-g\" no longer uses an equals '=' sign for its optional\narguments, but \"--reflog\" still does. This is normal behavior for parse\noptions, as arguments to \"-g\" are put immediately after the option with\nno space.\n\nFor example\n\n    git show-branch -g=4\n\nis now\n\n    git show-branch -g4\n\nSigned-off-by: Stephen Boyd <bebarino@gmail.com>\n---\n builtin-show-branch.c |  125 +++++++++++++++++++++++++------------------------\n 1 files changed, 63 insertions(+), 62 deletions(-)\n\ndiff --git a/builtin-show-branch.c b/builtin-show-branch.c\nindex c8e9b3c..b1affd2 100644\n--- a/builtin-show-branch.c\n+++ b/builtin-show-branch.c\n@@ -3,11 +3,13 @@\n #include \"refs.h\"\n #include \"builtin.h\"\n #include \"color.h\"\n+#include \"parse-options.h\"\n \n-static const char show_branch_usage[] =\n-\"git show-branch [--sparse] [--current] [--all] [--remotes] [--topo-order] [--more=count | --list | --independent | --merge-base ] [--topics] [<refs>...] | --reflog[=n[,b]] <branch>\";\n-static const char show_branch_usage_reflog[] =\n-\"--reflog is incompatible with --all, --remotes, --independent or --merge-base\";\n+static const char* show_branch_usage[] = {\n+    \"git show-branch [--sparse] [--current] [--all] [--remotes] [--topo-order] [--more=count | --list | --independent | --merge-base] [--topics] [--color] [<refs>...]\",\n+    \"--reflog[=n[,b]] [--list] [--color] <branch>\",\n+    NULL\n+};\n \n static int showbranch_use_color = -1;\n static char column_colors[][COLOR_MAXLEN] = {\n@@ -601,18 +603,25 @@ static int omit_in_dense(struct commit *commit, struct commit **rev, int n)\n \treturn 0;\n }\n \n-static void parse_reflog_param(const char *arg, int *cnt, const char **base)\n+static int reflog = 0;\n+\n+static int parse_reflog_param(const struct option *opt, const char *arg,\n+\t\t\t      int unset)\n {\n \tchar *ep;\n-\t*cnt = strtoul(arg, &ep, 10);\n+\tconst char **base = (const char **)opt->value;\n+\tif (!arg)\n+\t\targ = \"\";\n+\treflog = strtoul(arg, &ep, 10);\n \tif (*ep == ',')\n \t\t*base = ep + 1;\n \telse if (*ep)\n-\t\tdie(\"unrecognized reflog param '%s'\", arg);\n+\t\treturn error(\"unrecognized reflog param '%s'\", arg);\n \telse\n \t\t*base = NULL;\n-\tif (*cnt <= 0)\n-\t\t*cnt = DEFAULT_REFLOG;\n+\tif (reflog <= 0)\n+\t\treflog = DEFAULT_REFLOG;\n+\treturn 0;\n }\n \n int cmd_show_branch(int ac, const char **av, const char *prefix)\n@@ -638,8 +647,44 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \tint head_at = -1;\n \tint topics = 0;\n \tint dense = 1;\n-\tint reflog = 0;\n \tconst char *reflog_base = NULL;\n+\tstruct option builtin_show_branch_options[] = {\n+\t\tOPT_BOOLEAN('a', \"all\", &all_heads,\n+\t\t\t    \"show remote-tracking and local branches\"),\n+\t\tOPT_BOOLEAN('r', \"remotes\", &all_remotes,\n+\t\t\t    \"show remote-tracking branches\"),\n+\t\tOPT_BOOLEAN(0, \"color\", &showbranch_use_color,\n+\t\t\t    \"color '*!+-' corresponding to the branch\"),\n+\t\t{ OPTION_INTEGER, 0, \"more\", &extra, \"n\",\n+\t\t\t    \"show <n> more commits after the common ancestor\",\n+\t\t\t    PARSE_OPT_OPTARG | PARSE_OPT_LASTARG_DEFAULT,\n+\t\t\t    NULL, (intptr_t)1 },\n+\t\tOPT_SET_INT(0, \"list\", &extra, \"synonym to more=-1\", -1),\n+\t\tOPT_BOOLEAN(0, \"no-name\", &no_name, \"suppress naming strings\"),\n+\t\tOPT_BOOLEAN(0, \"current\", &with_current_branch,\n+\t\t\t    \"include the current branch\"),\n+\t\tOPT_BOOLEAN(0, \"sha1-name\", &sha1_name,\n+\t\t\t    \"name commits with their object names\"),\n+\t\tOPT_BOOLEAN(0, \"merge-base\", &merge_base,\n+\t\t\t    \"act like git merge-base -a\"),\n+\t\tOPT_BOOLEAN(0, \"independent\", &independent,\n+\t\t\t    \"show refs unreachable from any other ref\"),\n+\t\tOPT_BOOLEAN(0, \"topo-order\", &lifo,\n+\t\t\t    \"show commits in topological order\"),\n+\t\tOPT_BOOLEAN(0, \"topics\", &topics,\n+\t\t\t    \"show only commits not on the first branch\"),\n+\t\tOPT_SET_INT(0, \"sparse\", &dense,\n+\t\t\t    \"show merges reachable from only one tip\", 0),\n+\t\tOPT_SET_INT(0, \"date-order\", &lifo,\n+\t\t\t    \"show commits where no parent comes before its \"\n+\t\t\t    \"children\", 0),\n+\t\t{ OPTION_CALLBACK, 'g', \"reflog\", &reflog_base, \"<n>[,<base>]\",\n+\t\t\t    \"show <n> most recent ref-log entries starting at \"\n+\t\t\t    \"base\",\n+\t\t\t    PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP,\n+\t\t\t    parse_reflog_param },\n+\t\tOPT_END()\n+\t};\n \n \tgit_config(git_show_branch_config, NULL);\n \n@@ -652,63 +697,18 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\tav = default_arg - 1; /* ick; we would not address av[0] */\n \t}\n \n-\twhile (1 < ac && av[1][0] == '-') {\n-\t\tconst char *arg = av[1];\n-\t\tif (!strcmp(arg, \"--\")) {\n-\t\t\tac--; av++;\n-\t\t\tbreak;\n-\t\t}\n-\t\telse if (!strcmp(arg, \"--all\") || !strcmp(arg, \"-a\"))\n-\t\t\tall_heads = all_remotes = 1;\n-\t\telse if (!strcmp(arg, \"--remotes\") || !strcmp(arg, \"-r\"))\n-\t\t\tall_remotes = 1;\n-\t\telse if (!strcmp(arg, \"--more\"))\n-\t\t\textra = 1;\n-\t\telse if (!strcmp(arg, \"--list\"))\n-\t\t\textra = -1;\n-\t\telse if (!strcmp(arg, \"--no-name\"))\n-\t\t\tno_name = 1;\n-\t\telse if (!strcmp(arg, \"--current\"))\n-\t\t\twith_current_branch = 1;\n-\t\telse if (!strcmp(arg, \"--sha1-name\"))\n-\t\t\tsha1_name = 1;\n-\t\telse if (!prefixcmp(arg, \"--more=\"))\n-\t\t\textra = atoi(arg + 7);\n-\t\telse if (!strcmp(arg, \"--merge-base\"))\n-\t\t\tmerge_base = 1;\n-\t\telse if (!strcmp(arg, \"--independent\"))\n-\t\t\tindependent = 1;\n-\t\telse if (!strcmp(arg, \"--topo-order\"))\n-\t\t\tlifo = 1;\n-\t\telse if (!strcmp(arg, \"--topics\"))\n-\t\t\ttopics = 1;\n-\t\telse if (!strcmp(arg, \"--sparse\"))\n-\t\t\tdense = 0;\n-\t\telse if (!strcmp(arg, \"--date-order\"))\n-\t\t\tlifo = 0;\n-\t\telse if (!strcmp(arg, \"--reflog\") || !strcmp(arg, \"-g\")) {\n-\t\t\treflog = DEFAULT_REFLOG;\n-\t\t}\n-\t\telse if (!prefixcmp(arg, \"--reflog=\"))\n-\t\t\tparse_reflog_param(arg + 9, &reflog, &reflog_base);\n-\t\telse if (!prefixcmp(arg, \"-g=\"))\n-\t\t\tparse_reflog_param(arg + 3, &reflog, &reflog_base);\n-\t\telse if (!strcmp(arg, \"--color\"))\n-\t\t\tshowbranch_use_color = 1;\n-\t\telse if (!strcmp(arg, \"--no-color\"))\n-\t\t\tshowbranch_use_color = 0;\n-\t\telse\n-\t\t\tusage(show_branch_usage);\n-\t\tac--; av++;\n-\t}\n-\tac--; av++;\n+\tac = parse_options(ac, av, builtin_show_branch_options,\n+\t\t\t   show_branch_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n+\tif (all_heads)\n+\t\tall_remotes = 1;\n \n \tif (extra || reflog) {\n \t\t/* \"listing\" mode is incompatible with\n \t\t * independent nor merge-base modes.\n \t\t */\n \t\tif (independent || merge_base)\n-\t\t\tusage(show_branch_usage);\n+\t\t\tusage_with_options(show_branch_usage,\n+\t\t\t\t\t   builtin_show_branch_options);\n \t\tif (reflog && ((0 < extra) || all_heads || all_remotes))\n \t\t\t/*\n \t\t\t * Asking for --more in reflog mode does not\n@@ -716,7 +716,8 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)\n \t\t\t *\n \t\t\t * Also --all and --remotes do not make sense either.\n \t\t\t */\n-\t\t\tusage(show_branch_usage_reflog);\n+\t\t\tdie(\"--reflog is incompatible with --all, --remotes, \"\n+\t\t\t    \"--independent or --merge-base\");\n \t}\n \n \t/* If nothing is specified, show all branches by default */\n-- \n1.6.3.1.61.g065b0\n"},{"id":"114433","messageId":"7v3aaybewo.fsf@alter.siamese.dyndns.org","threadId":"19428","inReplyTo":"fbaa26d941f496efefc30e298f5534f8194ee716.1242891030.git.bebarino@gmail.com","subject":"Re: [PATCHv2 1/2] parse-options: add PARSE_OPT_LITERAL_ARGHELP for complicated argh's","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-21T16:26:47Z","receivedAt":"2009-05-21T16:26:47Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stephen Boyd <bebarino@gmail.com> writes:\n\n> I've decided to appease the pirate haters :-)\n\nHmmm, why does this break t0040 (I am queuing this on top of 5acb3e5)?\n"},{"id":"114435","messageId":"4A1586A7.9000909@lsrfire.ath.cx","threadId":"19428","inReplyTo":"7v3aaybewo.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2 1/2] parse-options: add PARSE_OPT_LITERAL_ARGHELP for complicated argh's","fromName":"René Scharfe","fromEmail":"rene.scharfe@lsrfire.ath.cx","sentAt":"2009-05-21T16:51:51Z","receivedAt":"2009-05-21T16:51:51Z","isPatch":false,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Junio C Hamano schrieb:\n> Stephen Boyd <bebarino@gmail.com> writes:\n> \n>> I've decided to appease the pirate haters :-)\n> \n> Hmmm, why does this break t0040 (I am queuing this on top of 5acb3e5)?\n\nProbably because it changes this:\n\n\tpos += fprintf(...);\n\ninto this (simplified, usage_argh() expanded):\n\n\tpos += pos + fprintf(...);\n\nusage_argh() doesn't need the parameter pos.\n\nRené\n"},{"id":"114437","messageId":"780e0a6b0905211203g3223fe3fp5eed27ebf244394a@mail.gmail.com","threadId":"19428","inReplyTo":"4A1586A7.9000909@lsrfire.ath.cx","subject":"Re: [PATCHv2 1/2] parse-options: add PARSE_OPT_LITERAL_ARGHELP for complicated argh's","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2009-05-21T19:03:23Z","receivedAt":"2009-05-21T19:03:23Z","isPatch":false,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"On Thu, May 21, 2009 at 9:51 AM, René Scharfe\n<rene.scharfe@lsrfire.ath.cx> wrote:\n> Junio C Hamano schrieb:\n>> Hmmm, why does this break t0040 (I am queuing this on top of 5acb3e5)?\n>\n> Probably because it changes this:\n>\n>        pos += fprintf(...);\n>\n> into this (simplified, usage_argh() expanded):\n>\n>        pos += pos + fprintf(...);\n>\n> usage_argh() doesn't need the parameter pos.\n>\n> René\n>\n\nWoops. I thought I ran the tests but I guess I didn't. This is the\ncorrect fix, thanks.\n"},{"id":"114443","messageId":"4A15C73D.3010301@gmail.com","threadId":"19428","inReplyTo":"780e0a6b0905211203g3223fe3fp5eed27ebf244394a@mail.gmail.com","subject":"Re: [PATCHv2 1/2] parse-options: add PARSE_OPT_LITERAL_ARGHELP for complicated argh's","fromName":"Stephen Boyd","fromEmail":"bebarino@gmail.com","sentAt":"2009-05-21T21:27:25Z","receivedAt":"2009-05-21T21:27:25Z","isPatch":false,"sender":{"key":"bebarino@gmail.com","avatar":"https://avatars.githubusercontent.com/u/38832?v=4"},"body":"Stephen Boyd wrote:\n> On Thu, May 21, 2009 at 9:51 AM, René Scharfe\n> <rene.scharfe@lsrfire.ath.cx> wrote:\n>> Junio C Hamano schrieb:\n>>> Hmmm, why does this break t0040 (I am queuing this on top of 5acb3e5)?\n>> Probably because it changes this:\n>>\n>>        pos += fprintf(...);\n>>\n>> into this (simplified, usage_argh() expanded):\n>>\n>>        pos += pos + fprintf(...);\n>>\n>> usage_argh() doesn't need the parameter pos.\n>>\n>> René\n>>\n>\n> Woops. I thought I ran the tests but I guess I didn't. This is the\n> correct fix, thanks.\n\nAnd here's the patch you can squash in.\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 2b880b1..e8955be 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -361,7 +361,7 @@ int parse_options(int argc, const char **argv, const struct option *options,\n        return parse_options_end(&ctx);\n }\n\n-static int usage_argh(const struct option *opts, int pos)\n+static int usage_argh(const struct option *opts)\n {\n        const char *s;\n        int literal = opts->flags & PARSE_OPT_LITERAL_ARGHELP;\n@@ -372,7 +372,7 @@ static int usage_argh(const struct option *opts, int pos)\n                        s = literal ? \"[%s]\" : \"[<%s>]\";\n        else\n                s = literal ? \" %s\" : \" <%s>\";\n-       return pos + fprintf(stderr, s, opts->argh);\n+       return fprintf(stderr, s, opts->argh);\n }\n\n #define USAGE_OPTS_WIDTH 24\n@@ -436,7 +436,7 @@ int usage_with_options_internal(const char * const *usagestr,\n                        /* FALLTHROUGH */\n                case OPTION_STRING:\n                        if (opts->argh)\n-                               pos += usage_argh(opts, pos);\n+                               pos += usage_argh(opts);\n                        else {\n                                if (opts->flags & PARSE_OPT_OPTARG)\n                                        if (opts->long_name)\n"}]}