{"thread":{"id":"10133","subject":"[PATCH] Port builtin-add.c to use the new option parser.","startedAt":"2007-10-03T21:45:01Z","lastAt":"2007-10-07T17:01:54Z","messageCount":26,"participants":["Kristian Høgsberg","Pierre Habouzit","Johannes Schindelin","Mike Hommey","David Kastrup","Medve Emilian-EMMEDVE1","Linus Torvalds","Sven Verdoolaege"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"54766","messageId":"1191447902-27326-1-git-send-email-krh@redhat.com","threadId":"10133","inReplyTo":null,"subject":"[PATCH] Add a simple option parser.","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-10-03T21:45:01Z","receivedAt":"2007-10-03T21:45:01Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"The option parser takes argc, argv, an array of struct option\nand a usage string. Each of the struct option elements in the array\ndescribes a valid option, its type and a pointer to the location where the\nvalue is written. The entry point is parse_options(), which scans through\nthe given argv, and matches each option there against the list of valid\noptions. During the scan, argv is rewritten to only contain the\nnon-option command line arguments and the number of these is returned.\n\nSigned-off-by: Kristian Høgsberg \n---\n Makefile | 2 +-\n parse-options.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n parse-options.h | 33 +++++++++++++++++\n 3 files changed, 140 insertions(+), 1 deletions(-)\n create mode 100644 parse-options.c\n create mode 100644 parse-options.h\n\ndiff --git a/Makefile b/Makefile\nindex 62bdac6..d90e959 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -310,7 +310,7 @@ LIB_OBJS = \\\n \talloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \\\n \tcolor.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \\\n \tconvert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \\\n-\ttransport.o bundle.o\n+\ttransport.o bundle.o parse-options.o\n \n BUILTIN_OBJS = \\\n \tbuiltin-add.o \\\ndiff --git a/parse-options.c b/parse-options.c\nnew file mode 100644\nindex 0000000..130b609\n--- /dev/null\n+++ b/parse-options.c\n@@ -0,0 +1,106 @@\n+#include \"git-compat-util.h\"\n+#include \"parse-options.h\"\n+\n+static int parse_one(const char **argv,\n+\t\t struct option *options, int count,\n+\t\t const char *usage_string)\n+{\n+\tconst char *eq, *arg, *value;\n+\tint i, processed;\n+\n+\targ = argv[0];\n+\tvalue = NULL;\n+\n+\tif (arg[0] != '-')\n+\t\treturn 0;\n+\n+\tfor (i = 0; i < count; i++) {\n+\t\tif (arg[1] == '-') {\n+\t\t\tif (!prefixcmp(options[i].long_name, arg + 2)) {\n+\t\t\t\tif (options[i].type != OPTION_BOOLEAN) {\n+\t\t\t\t\tvalue = argv[1];\n+\t\t\t\t\tprocessed = 2;\n+\t\t\t\t} else {\n+\t\t\t\t\tprocessed = 1;\n+\t\t\t\t}\n+\t\t\t\tbreak;\n+\t\t\t}\n+\n+\t\t\teq = strchr(arg + 2, '=');\n+\t\t\tif (eq && options[i].type != OPTION_BOOLEAN &&\n+\t\t\t !strncmp(arg + 2,\n+\t\t\t\t options[i].long_name, eq - arg - 2)) {\n+\t\t\t\tvalue = eq + 1;\n+\t\t\t\tprocessed = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (arg[1] == options[i].short_name) {\n+\t\t\tif (arg[2] == '\\0') {\n+\t\t\t\tif (options[i].type != OPTION_BOOLEAN) {\n+\t\t\t\t\tvalue = argv[1];\n+\t\t\t\t\tprocessed = 2;\n+\t\t\t\t} else {\n+\t\t\t\t\tprocessed = 1;\n+\t\t\t\t}\n+\t\t\t\tbreak;\n+\t\t\t}\n+\n+\t\t\tif (options[i].type != OPTION_BOOLEAN) {\n+\t\t\t\tvalue = arg + 2;\n+\t\t\t\tprocessed = 1;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t}\n+\n+\tif (i == count)\n+\t\tusage(usage_string);\n+\telse switch (options[i].type) {\n+\tcase OPTION_BOOLEAN:\n+\t\t(*(int *)options[i].value)++;\n+\t\tbreak;\n+\tcase OPTION_STRING:\n+\t\tif (value == NULL) {\n+\t\t\terror(\"option %s requires a value.\", arg);\n+\t\t\tusage(usage_string);\n+\t\t}\n+\t\t*(const char **)options[i].value = value;\n+\t\tbreak;\n+\tcase OPTION_INTEGER:\n+\t\tif (value == NULL) {\n+\t\t\terror(\"option %s requires a value.\", argv);\n+\t\t\tusage(usage_string);\n+\t\t}\n+\t\t*(int *)options[i].value = atoi(value);\n+\t\tbreak;\n+\tdefault:\n+\t\tassert(0);\n+\t}\n+\n+\treturn processed;\n+}\n+\n+int parse_options(int argc, const char **argv,\n+\t\t struct option *options, int count,\n+\t\t const char *usage_string)\n+{\n+\tint i, j, processed;\n+\n+\tfor (i = 1, j = 0; i < argc; ) {\n+\t\tif (!strcmp(argv[i], \"--\"))\n+\t\t\tbreak;\n+\t\tprocessed = parse_one(argv + i, options, count, usage_string);\n+\t\tif (processed == 0)\n+\t\t\targv[j++] = argv[i++];\n+\t\telse\n+\t\t\ti += processed;\n+\t}\n+\n+\twhile (i < argc)\n+\t\targv[j++] = argv[i++];\n+\targv[j] = NULL;\n+\n+\treturn j;\n+}\ndiff --git a/parse-options.h b/parse-options.h\nnew file mode 100644\nindex 0000000..5be9c20\n--- /dev/null\n+++ b/parse-options.h\n@@ -0,0 +1,33 @@\n+#ifndef PARSE_OPTIONS_H\n+#define PARSE_OPTIONS_H\n+\n+enum option_type {\n+\tOPTION_BOOLEAN,\n+\tOPTION_STRING,\n+\tOPTION_INTEGER,\n+\tOPTION_LAST,\n+};\n+\n+struct option {\n+\tenum option_type type;\n+\tconst char *long_name;\n+\tchar short_name;\n+\tvoid *value;\n+};\n+\n+/* Parse the given options against the list of known options. The\n+ * order of the option structs matters, in that ambiguous\n+ * abbreviations (eg, --in could be short for --include or\n+ * --interactive) are matched by the first option that share the\n+ * prefix.\n+ *\n+ * parse_options() will filter out the processed options and leave the\n+ * non-option argments in argv[]. The return value is the number of\n+ * arguments left in argv[].\n+ */\n+\n+extern int parse_options(int argc, const char **argv,\n+\t\t\t struct option *options, int count,\n+\t\t\t const char *usage_string);\n+\n+#endif\n-- \n1.5.2.5\n"},{"id":"54764","messageId":"1191447902-27326-2-git-send-email-krh@redhat.com","threadId":"10133","inReplyTo":"1191447902-27326-1-git-send-email-krh@redhat.com","subject":"[PATCH] Port builtin-add.c to use the new option parser.","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-10-03T21:45:02Z","receivedAt":"2007-10-03T21:45:02Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"Signed-off-by: Kristian Høgsberg \n---\n builtin-add.c | 64 ++++++++++++++++++--------------------------------------\n 1 files changed, 21 insertions(+), 43 deletions(-)\n\ndiff --git a/builtin-add.c b/builtin-add.c\nindex 966e145..66fd99d 100644\n--- a/builtin-add.c\n+++ b/builtin-add.c\n@@ -13,6 +13,7 @@\n #include \"commit.h\"\n #include \"revision.h\"\n #include \"run-command.h\"\n+#include \"parse-options.h\"\n \n static const char builtin_add_usage[] =\n \"git-add [-n] [-v] [-f] [--interactive | -i] [-u] [--refresh] [--] ...\";\n@@ -160,21 +161,30 @@ static struct lock_file lock_file;\n static const char ignore_error[] =\n \"The following paths are ignored by one of your .gitignore files:\\n\";\n \n+static int verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;\n+static int add_interactive = 0;\n+\n+static struct option builtin_add_options[] = {\n+\t{ OPTION_BOOLEAN, \"interactive\", 'i', &add_interactive },\n+\t{ OPTION_BOOLEAN, NULL, 'n', &show_only },\n+\t{ OPTION_BOOLEAN, NULL, 'f', &ignored_too },\n+\t{ OPTION_BOOLEAN, NULL, 'v',&verbose },\n+\t{ OPTION_BOOLEAN, NULL, 'u',&take_worktree_changes },\n+\t{ OPTION_BOOLEAN, \"refresh\", 0, &refresh_only }\n+};\n+\n int cmd_add(int argc, const char **argv, const char *prefix)\n {\n \tint i, newfd;\n-\tint verbose = 0, show_only = 0, ignored_too = 0, refresh_only = 0;\n \tconst char **pathspec;\n \tstruct dir_struct dir;\n-\tint add_interactive = 0;\n \n-\tfor (i = 1; i < argc; i++) {\n-\t\tif (!strcmp(\"--interactive\", argv[i]) ||\n-\t\t !strcmp(\"-i\", argv[i]))\n-\t\t\tadd_interactive++;\n-\t}\n+\ti = parse_options(argc, argv, builtin_add_options,\n+\t\t\t ARRAY_SIZE(builtin_add_options),\n+\t\t\t builtin_add_usage);\n+\n \tif (add_interactive) {\n-\t\tif (argc != 2)\n+\t\tif (i > 0)\n \t\t\tdie(\"add --interactive does not take any parameters\");\n \t\texit(interactive_add());\n \t}\n@@ -183,51 +193,19 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \n \tnewfd = hold_locked_index(&lock_file, 1);\n \n-\tfor (i = 1; i < argc; i++) {\n-\t\tconst char *arg = argv[i];\n-\n-\t\tif (arg[0] != '-')\n-\t\t\tbreak;\n-\t\tif (!strcmp(arg, \"--\")) {\n-\t\t\ti++;\n-\t\t\tbreak;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-n\")) {\n-\t\t\tshow_only = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-f\")) {\n-\t\t\tignored_too = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-v\")) {\n-\t\t\tverbose = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"-u\")) {\n-\t\t\ttake_worktree_changes = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tif (!strcmp(arg, \"--refresh\")) {\n-\t\t\trefresh_only = 1;\n-\t\t\tcontinue;\n-\t\t}\n-\t\tusage(builtin_add_usage);\n-\t}\n-\n \tif (take_worktree_changes) {\n \t\tif (read_cache() < 0)\n \t\t\tdie(\"index file corrupt\");\n-\t\tadd_files_to_cache(verbose, prefix, argv + i);\n+\t\tadd_files_to_cache(verbose, prefix, argv);\n \t\tgoto finish;\n \t}\n \n-\tif (argc <= i) {\n+\tif (i == 0) {\n \t\tfprintf(stderr, \"Nothing specified, nothing added.\\n\");\n \t\tfprintf(stderr, \"Maybe you wanted to say 'git add .'?\\n\");\n \t\treturn 0;\n \t}\n-\tpathspec = get_pathspec(prefix, argv + i);\n+\tpathspec = get_pathspec(prefix, argv);\n \n \tif (refresh_only) {\n \t\trefresh(verbose, pathspec);\n-- \n1.5.2.5\n"},{"id":"54776","messageId":"20071003231145.GF28188@artemis.corp","threadId":"10133","inReplyTo":"1191447902-27326-1-git-send-email-krh@redhat.com","subject":"Re: [PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-03T23:11:45Z","receivedAt":"2007-10-03T23:11:45Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Oct 03, 2007 at 09:45:01PM +0000, Kristian Høgsberg wrote:\n> The option parser takes argc, argv, an array of struct option\n> and a usage string. Each of the struct option elements in the array\n> describes a valid option, its type and a pointer to the location where the\n> value is written. The entry point is parse_options(), which scans through\n> the given argv, and matches each option there against the list of valid\n> options. During the scan, argv is rewritten to only contain the\n> non-option command line arguments and the number of these is returned.\n\n if we are going in that direction (and I believe it's a good one), we\nshould be sure that the model fits with other commands as well. And as I\nsaid on IRC, I believe the most \"horrible\" (as in complex) option parser\nin git is the one from git-grep.\n\n A migration of git-grep on that API should be tried first. If this\nworks well enough, I believe that the rest of the git commands will be\nmigrated easily enough. (with maybe small addition to parse-option.[hc]\nbut the hardcore things should have been met with git-grep already I\nthink).\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"54829","messageId":"1191509878.29379.2.camel@hinata.boston.redhat.com","threadId":"10133","inReplyTo":"20071003231145.GF28188@artemis.corp","subject":"Re: [PATCH] Add a simple option parser.","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-10-04T14:57:58Z","receivedAt":"2007-10-04T14:57:58Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"\nOn Thu, 2007-10-04 at 01:11 +0200, Pierre Habouzit wrote:\n> On Wed, Oct 03, 2007 at 09:45:01PM +0000, Kristian Høgsberg wrote:\n> > The option parser takes argc, argv, an array of struct option\n> > and a usage string. Each of the struct option elements in the array\n> > describes a valid option, its type and a pointer to the location where the\n> > value is written. The entry point is parse_options(), which scans through\n> > the given argv, and matches each option there against the list of valid\n> > options. During the scan, argv is rewritten to only contain the\n> > non-option command line arguments and the number of these is returned.\n> \n> if we are going in that direction (and I believe it's a good one), we\n> should be sure that the model fits with other commands as well. And as I\n> said on IRC, I believe the most \"horrible\" (as in complex) option parser\n> in git is the one from git-grep.\n> \n> A migration of git-grep on that API should be tried first. If this\n> works well enough, I believe that the rest of the git commands will be\n> migrated easily enough. (with maybe small addition to parse-option.[hc]\n> but the hardcore things should have been met with git-grep already I\n> think).\n\nI'm not sure - we can go with the current proposal and add new options\ntypes and probably the callback option type I suggested as we go. I\ndon't want to block builtin-commit on figuring out what the perfect\noption parser should look like and what I sent out earlier work for\ncommit. I think the way you handled the strbuf rewrites worked pretty\nwell; extending and rewriting the API as you put it to use in more and\nmore places. We can do the same thing with parse_options().\n\ncheers,\nKristian\n"},{"id":"54832","messageId":"20071004151532.GB5083@artemis.corp","threadId":"10133","inReplyTo":"1191509878.29379.2.camel@hinata.boston.redhat.com","subject":"Re: [PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-04T15:15:32Z","receivedAt":"2007-10-04T15:15:32Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Thu, Oct 04, 2007 at 02:57:58PM +0000, Kristian Høgsberg wrote:\n> \n> On Thu, 2007-10-04 at 01:11 +0200, Pierre Habouzit wrote:\n> > On Wed, Oct 03, 2007 at 09:45:01PM +0000, Kristian Høgsberg wrote:\n> > > The option parser takes argc, argv, an array of struct option\n> > > and a usage string. Each of the struct option elements in the array\n> > > describes a valid option, its type and a pointer to the location where the\n> > > value is written. The entry point is parse_options(), which scans through\n> > > the given argv, and matches each option there against the list of valid\n> > > options. During the scan, argv is rewritten to only contain the\n> > > non-option command line arguments and the number of these is returned.\n> > \n> > if we are going in that direction (and I believe it's a good one), we\n> > should be sure that the model fits with other commands as well. And as I\n> > said on IRC, I believe the most \"horrible\" (as in complex) option parser\n> > in git is the one from git-grep.\n> > \n> > A migration of git-grep on that API should be tried first. If this\n> > works well enough, I believe that the rest of the git commands will be\n> > migrated easily enough. (with maybe small addition to parse-option.[hc]\n> > but the hardcore things should have been met with git-grep already I\n> > think).\n> \n> I'm not sure - we can go with the current proposal and add new options\n> types and probably the callback option type I suggested as we go. I\n> don't want to block builtin-commit on figuring out what the perfect\n> option parser should look like and what I sent out earlier work for\n> commit. I think the way you handled the strbuf rewrites worked pretty\n> well; extending and rewriting the API as you put it to use in more and\n> more places. We can do the same thing with parse_options().\n\n Of course we can do that, or junio said that some people talked about\npopt some time ago. I understand that you don't want to block the\ngit-commit work, but doing things right from the beginning is often a\nbig win on the long term.\n\n I don't know popt, and I don't know if it has sufficient expressivity.\nFor sure I don't like getopt_long APIs at all, so if popt is as\ncumbersome, rolling our own based on the current parse_options you\npropose is probably a good choice.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"54851","messageId":"20071004163156.GD5083@artemis.corp","threadId":"10133","inReplyTo":"20071004151532.GB5083@artemis.corp","subject":"Re: [PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-04T16:31:56Z","receivedAt":"2007-10-04T16:31:56Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On jeu, oct 04, 2007 at 03:15:32 +0000, Pierre Habouzit wrote:\n> On Thu, Oct 04, 2007 at 02:57:58PM +0000, Kristian Høgsberg wrote:\n> > I'm not sure - we can go with the current proposal and add new options\n> > types and probably the callback option type I suggested as we go. I\n> > don't want to block builtin-commit on figuring out what the perfect\n> > option parser should look like and what I sent out earlier work for\n> > commit. I think the way you handled the strbuf rewrites worked pretty\n> > well; extending and rewriting the API as you put it to use in more and\n> > more places. We can do the same thing with parse_options().\n> \n> Of course we can do that, or junio said that some people talked about\n> popt some time ago. I understand that you don't want to block the\n> git-commit work, but doing things right from the beginning is often a\n> big win on the long term.\n> \n> I don't know popt, and I don't know if it has sufficient expressivity.\n> For sure I don't like getopt_long APIs at all, so if popt is as\n> cumbersome, rolling our own based on the current parse_options you\n> propose is probably a good choice.\n\n Okay, popt seems to be quite complicated, and depends upon gettext\n(which we may require as per survey results, but right now it seems a\nuseless dependency). Don't get me wrong, I'm sure it's very powerful,\nbut again, I believe we can have a 200 line ad-hoc module that fits what\ngit really needs, the less cumbersome way.\n\n So well, I'd be (I'm not in position to decide anything btw ;p) in\nfavor of pursuing the work into git-commit like you did, and ASAP it\ngets merged into next, I'm definitely willing to pursue a refactoring to\nuse it (now that strbufs seems to have been used where needed).\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"54852","messageId":"Pine.LNX.4.64.0710041736080.4174@racer.site","threadId":"10133","inReplyTo":"20071004163156.GD5083@artemis.corp","subject":"Re: [PATCH] Add a simple option parser.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-10-04T16:39:53Z","receivedAt":"2007-10-04T16:39:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 4 Oct 2007, Pierre Habouzit wrote:\n\n> Okay, popt seems to be quite complicated, and depends upon gettext\n\n... which makes me vote against popt ...\n\n> (which we may require as per survey results, but right now it seems a\n> useless dependency).\n\nNope. git-gui got a script doing the job of msgfmt, which was the only \npart of that beast known as gettext anyway.\n\nSo we will not require it for git-gui.\n\nAnd I do not see core git being i18n'ised. Ever.\n\nCiao,\nDscho\n"},{"id":"54931","messageId":"20071005100840.GI19879@artemis.corp","threadId":"10133","inReplyTo":"1191447902-27326-1-git-send-email-krh@redhat.com","subject":"Re: [PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-05T10:08:40Z","receivedAt":"2007-10-05T10:08:40Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Wed, Oct 03, 2007 at 09:45:01PM +0000, Kristian Høgsberg wrote:\n> +static int parse_one(const char **argv,\n> +\t\t struct option *options, int count,\n> +\t\t const char *usage_string)\n> +{\n> +\tconst char *eq, *arg, *value;\n> +\tint i, processed;\n\n gcc complains processed could be returned without being initialized\nfirst, so should be processed = 0; Even if it cannot occurs, it avoid\nraising eyebrows.\n\n> +\tcase OPTION_INTEGER:\n> +\t\tif (value == NULL) {\n> +\t\t\terror(\"option %s requires a value.\", argv);\n\n ^^^\n should probably be arg.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"54952","messageId":"20071005142140.GK19879@artemis.corp","threadId":"10133","inReplyTo":"1191447902-27326-1-git-send-email-krh@redhat.com","subject":"Re: [PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-05T14:21:40Z","receivedAt":"2007-10-05T14:21:40Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"> +/* Parse the given options against the list of known options. The\n> + * order of the option structs matters, in that ambiguous\n> + * abbreviations (eg, --in could be short for --include or\n> + * --interactive) are matched by the first option that share the\n> + * prefix.\n\n Do we really want that ?\n\n I do believe that it's a very bad idea, as it silently breaks. Most of\nthe command line switches people need to use have a short form, or their\nshell will complete it properly.\n\n A very interesting feature though, would be to finally be able to\nparse aggregated switches (`git rm -rf` anyone ?).\n\n I also believe that it's a pity that parse_options isn't able to\ngenerate the usage by itself. But we can add that later.\n\n I've though an alternate proposal, based on your work, for the first\npatch.\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"54953","messageId":"20071005142507.GL19879@artemis.corp","threadId":"10133","inReplyTo":"20071005142140.GK19879@artemis.corp","subject":"[ALTERNATE PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-05T14:25:07Z","receivedAt":"2007-10-05T14:25:07Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"The option parser takes argc, argv, an array of struct option\nand a usage string. Each of the struct option elements in the array\ndescribes a valid option, its type and a pointer to the location where the\nvalue is written. The entry point is parse_options(), which scans through\nthe given argv, and matches each option there against the list of valid\noptions. During the scan, argv is rewritten to only contain the\nnon-option command line arguments and the number of these is returned.\n\nAggregation of single switches is allowed:\n -rC0 is the same as -r -C 0 (supposing that -C wants an arg).\n\nBoolean switches automatically support the option with the same name,\nprefixed with 'no-' to disable the switch:\n --no-color / --color only need to have an entry for \"color\".\n\nLong options are supported either with '=' or without:\n --some-option=foo is the same as --some-option foo\n\nSigned-off-by: Kristian Høgsberg \nSigned-off-by: Pierre Habouzit \n---\n\nI'm sorry about the \"From\" I don't intend to \"steal\" the patch in any\nsense, it's just an alternate proposal.\n\noh and I don't grok what OPTION_LAST is for, so I left it apart, but\nit seems unused ?\n\n\n Makefile | 2 +-\n parse-options.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n parse-options.h | 29 ++++++++++\n 3 files changed, 184 insertions(+), 1 deletions(-)\n create mode 100644 parse-options.c\n create mode 100644 parse-options.h\n\ndiff --git a/Makefile b/Makefile\nindex 62bdac6..d90e959 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -310,7 +310,7 @@ LIB_OBJS = \\\n \talloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \\\n \tcolor.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \\\n \tconvert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \\\n-\ttransport.o bundle.o\n+\ttransport.o bundle.o parse-options.o\n \n BUILTIN_OBJS = \\\n \tbuiltin-add.o \\\ndiff --git a/parse-options.c b/parse-options.c\nnew file mode 100644\nindex 0000000..eb3ff40\n--- /dev/null\n+++ b/parse-options.c\n@@ -0,0 +1,154 @@\n+#include \"git-compat-util.h\"\n+#include \"parse-options.h\"\n+\n+struct optparse_t {\n+\tconst char **argv;\n+\tint argc;\n+\tconst char *opt;\n+};\n+\n+static inline const char *skippfx(const char *str, const char *prefix)\n+{\n+\tsize_t len = strlen(prefix);\n+\treturn strncmp(str, prefix, len) ? NULL : str + len;\n+}\n+\n+static int opterror(struct option *opt, const char *reason, int shorterr)\n+{\n+\tif (shorterr) {\n+\t\treturn error(\"switch `%c' %s\", opt->short_name, reason);\n+\t} else {\n+\t\treturn error(\"option `%s' %s\", opt->long_name, reason);\n+\t}\n+}\n+\n+static int get_value(struct optparse_t *p, struct option *opt,\n+\t\t\t\t\t int boolean, int shorterr)\n+{\n+\tswitch (opt->type) {\n+\t\tconst char *s;\n+\t\tint v;\n+\n+\t case OPTION_BOOLEAN:\n+\t\t*(int *)opt->value = boolean;\n+\t\treturn 0;\n+\n+\t case OPTION_STRING:\n+\t\tif (p->opt && *p->opt) {\n+\t\t\t*(const char **)opt->value = p->opt;\n+\t\t\tp->opt = NULL;\n+\t\t} else {\n+\t\t\tif (p->argc < 1)\n+\t\t\t\treturn opterror(opt, \"requires a value\", shorterr);\n+\t\t\t*(const char **)opt->value = *++p->argv;\n+\t\t\tp->argc--;\n+\t\t}\n+\t\treturn 0;\n+\n+\t case OPTION_INTEGER:\n+\t\tif (p->opt && *p->opt) {\n+\t\t\tv = strtol(p->opt, (char **)&s, 10);\n+\t\t\tp->opt = NULL;\n+\t\t} else {\n+\t\t\tif (p->argc < 1)\n+\t\t\t\treturn opterror(opt, \"requires a value\", shorterr);\n+\t\t\tv = strtol(*++p->argv, (char **)&s, 10);\n+\t\t\tp->argc--;\n+\t\t}\n+\t\tif (*s)\n+\t\t\treturn opterror(opt, \"expects a numerical value\", shorterr);\n+\t\t*(int *)opt->value = v;\n+\t\treturn 0;\n+\t}\n+\n+\tabort();\n+}\n+\n+static int parse_short_opt(struct optparse_t *p, struct option *options, int count)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < count; i++) {\n+\t\tif (options[i].short_name == *p->opt) {\n+\t\t\tp->opt++;\n+\t\t\treturn get_value(p, options + i, 1, 1);\n+\t\t}\n+\t}\n+\treturn error(\"unknown switch `%c'\", *p->opt);\n+}\n+\n+static int parse_long_opt(struct optparse_t *p, const char *arg,\n+ struct option *options, int count)\n+{\n+\tint boolean = 1;\n+\tint i;\n+\n+\tfor (i = 0; i < count; i++) {\n+\t\tconst char *rest;\n+\t\t\n+\t\tif (!options[i].long_name)\n+\t\t\tcontinue;\n+\n+\t\trest = skippfx(arg, options[i].long_name);\n+\t\tif (!rest && options[i].type == OPTION_BOOLEAN) {\n+\t\t\tif (!rest && skippfx(arg, \"no-\")) {\n+\t\t\t\trest = skippfx(arg + 3, options[i].long_name);\n+\t\t\t\tboolean = 0;\n+\t\t\t}\n+\t\t\tif (rest && *rest == '=')\n+\t\t\t\treturn opterror(options + i, \"takes no value\", 0);\n+\t\t}\n+\t\tif (!rest || (*rest && *rest != '='))\n+\t\t\tcontinue;\n+\t\tif (*rest) {\n+\t\t\tp->opt = rest;\n+\t\t}\n+\t\treturn get_value(p, options + i, boolean, 0);\n+\t}\n+\treturn error(\"unknown option `%s'\", arg);\n+}\n+\n+int parse_options(int argc, const char **argv,\n+\t\t struct option *options, int count,\n+\t\t const char *usage_string)\n+{\n+\tstruct optparse_t optp = { argv + 1, argc - 1, NULL };\n+\tint j = 0;\n+\n+\twhile (optp.argc) {\n+\t\tconst char *arg = optp.argv[0];\n+\n+\t\tif (*arg != '-' || !arg[1]) {\n+\t\t\targv[j++] = *optp.argv++;\n+\t\t\toptp.argc--;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (arg[1] != '-') {\n+\t\t\toptp.opt = arg + 1;\n+\t\t\twhile (*optp.opt) {\n+\t\t\t\tif (parse_short_opt(&optp, options, count) < 0) {\n+\t\t\t\t\tusage(usage_string);\n+\t\t\t\t\treturn -1;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t\toptp.argc--;\n+\t\t\toptp.argv++;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (!arg[2]) /* \"--\" */\n+\t\t\tbreak;\n+\n+\t\tif (parse_long_opt(&optp, arg + 2, options, count)) {\n+\t\t\tusage(usage_string);\n+\t\t\treturn -1;\n+\t\t}\n+\t\toptp.argc--;\n+\t\toptp.argv++;\n+\t}\n+\n+\tmemmove(argv + j, optp.argv, optp.argc * sizeof(argv));\n+\targv[j + optp.argc] = NULL;\n+\treturn j + optp.argc;\n+}\ndiff --git a/parse-options.h b/parse-options.h\nnew file mode 100644\nindex 0000000..e4749d0\n--- /dev/null\n+++ b/parse-options.h\n@@ -0,0 +1,29 @@\n+#ifndef PARSE_OPTIONS_H\n+#define PARSE_OPTIONS_H\n+\n+enum option_type {\n+\tOPTION_BOOLEAN,\n+\tOPTION_STRING,\n+\tOPTION_INTEGER,\n+#if 0\n+\tOPTION_LAST,\n+#endif\n+};\n+\n+struct option {\n+\tenum option_type type;\n+\tconst char *long_name;\n+\tchar short_name;\n+\tvoid *value;\n+};\n+\n+/* parse_options() will filter out the processed options and leave the\n+ * non-option argments in argv[]. The return value is the number of\n+ * arguments left in argv[].\n+ */\n+\n+extern int parse_options(int argc, const char **argv,\n+\t\t\t struct option *options, int count,\n+\t\t\t const char *usage_string);\n+\n+#endif\n-- \n1.5.3.4.1156.ga72f9d-dirty\n\n"},{"id":"54956","messageId":"20071005143014.GA18176@glandium.org","threadId":"10133","inReplyTo":"20071005142507.GL19879@artemis.corp","subject":"Re: [ALTERNATE PATCH] Add a simple option parser.","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2007-10-05T14:30:14Z","receivedAt":"2007-10-05T14:30:14Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Fri, Oct 05, 2007 at 04:25:07PM +0200, Pierre Habouzit wrote:\n> The option parser takes argc, argv, an array of struct option\n> and a usage string. Each of the struct option elements in the array\n> describes a valid option, its type and a pointer to the location where the\n> value is written. The entry point is parse_options(), which scans through\n> the given argv, and matches each option there against the list of valid\n> options. During the scan, argv is rewritten to only contain the\n> non-option command line arguments and the number of these is returned.\n> \n> Aggregation of single switches is allowed:\n> -rC0 is the same as -r -C 0 (supposing that -C wants an arg).\n\nI like options aggregation, but I'm not sure aggregating option arguments\nis a good idea... I can't even think of an application that does it.\n\nMike\n"},{"id":"54958","messageId":"20071005144540.GM19879@artemis.corp","threadId":"10133","inReplyTo":"20071005143014.GA18176@glandium.org","subject":"Re: [ALTERNATE PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-05T14:45:40Z","receivedAt":"2007-10-05T14:45:40Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Fri, Oct 05, 2007 at 02:30:14PM +0000, Mike Hommey wrote:\n> On Fri, Oct 05, 2007 at 04:25:07PM +0200, Pierre Habouzit wrote:\n> > The option parser takes argc, argv, an array of struct option\n> > and a usage string. Each of the struct option elements in the array\n> > describes a valid option, its type and a pointer to the location where the\n> > value is written. The entry point is parse_options(), which scans through\n> > the given argv, and matches each option there against the list of valid\n> > options. During the scan, argv is rewritten to only contain the\n> > non-option command line arguments and the number of these is returned.\n> > \n> > Aggregation of single switches is allowed:\n> > -rC0 is the same as -r -C 0 (supposing that -C wants an arg).\n> \n> I like options aggregation, but I'm not sure aggregating option arguments\n> is a good idea... I can't even think of an application that does it.\n\n You mean like `grep -A1` or `diff -u3` or `ls -w10` ?\n\ngetopt does that by default as well, so you may not have aware of it,\nbut it's how things work in your system already.\n\n btw `ls -rw10` works, though `ls -w10r` drops the 'r' silently. FWIW I\ndon't, in that case, the alternate patch I propose complains about \"10r\"\nnot being a valid integer, and that's because unlike getopt, the patch\nkrh proposed knows what an integer is ;)\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"54961","messageId":"86lkahwqsz.fsf@lola.quinscape.zz","threadId":"10133","inReplyTo":"20071005143014.GA18176@glandium.org","subject":"Re: [ALTERNATE PATCH] Add a simple option parser.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-10-05T14:59:24Z","receivedAt":"2007-10-05T14:59:24Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Mike Hommey writes:\n\n> On Fri, Oct 05, 2007 at 04:25:07PM +0200, Pierre Habouzit wrote:\n>> The option parser takes argc, argv, an array of struct option\n>> and a usage string. Each of the struct option elements in the array\n>> describes a valid option, its type and a pointer to the location where the\n>> value is written. The entry point is parse_options(), which scans through\n>> the given argv, and matches each option there against the list of valid\n>> options. During the scan, argv is rewritten to only contain the\n>> non-option command line arguments and the number of these is returned.\n>> \n>> Aggregation of single switches is allowed:\n>> -rC0 is the same as -r -C 0 (supposing that -C wants an arg).\n>\n> I like options aggregation, but I'm not sure aggregating option arguments\n> is a good idea... I can't even think of an application that does it.\n\nI think most allow this for the last option in a row. Tar is somewhat\nmore perverse with its non-option command string:\n\ntar xfzbv filename.tgz 40\n\nuses filename.tgz as the option argument for \"f\" and 40 for \"b\".\n\nNote that while tar accepts options instead of the initial command\nstring,\n\ntar -xfzbv filename.tgz 40\n\nwill _not_ work, while\n\ntar -xffilename.tgz -z -b40 -v\n\npresumably would (have no time to test this right now).\n\n\n-- \nDavid Kastrup\n"},{"id":"54964","messageId":"1191598424.7117.10.camel@hinata.boston.redhat.com","threadId":"10133","inReplyTo":"20071005142507.GL19879@artemis.corp","subject":"Re: [ALTERNATE PATCH] Add a simple option parser.","fromName":"Kristian Høgsberg","fromEmail":"krh@redhat.com","sentAt":"2007-10-05T15:33:44Z","receivedAt":"2007-10-05T15:33:44Z","isPatch":true,"sender":{"key":"krh@redhat.com","avatar":"https://gravatar.com/avatar/763dee6f9594ac474f725b137a39565792928e583ddf59b32befc2907409027e?d=mp&s=160"},"body":"On Fri, 2007-10-05 at 16:25 +0200, Pierre Habouzit wrote:\n> The option parser takes argc, argv, an array of struct option\n> and a usage string. Each of the struct option elements in the array\n> describes a valid option, its type and a pointer to the location where the\n> value is written. The entry point is parse_options(), which scans through\n> the given argv, and matches each option there against the list of valid\n> options. During the scan, argv is rewritten to only contain the\n> non-option command line arguments and the number of these is returned.\n> \n> Aggregation of single switches is allowed:\n> -rC0 is the same as -r -C 0 (supposing that -C wants an arg).\n> \n> Boolean switches automatically support the option with the same name,\n> prefixed with 'no-' to disable the switch:\n> --no-color / --color only need to have an entry for \"color\".\n> \n> Long options are supported either with '=' or without:\n> --some-option=foo is the same as --some-option foo\n\nThat looks great, works for me. One comment, though: it looks like\nyou're not sure whether to call these things \"options\" or \"switches\".\nWe should choose one and stick with it.\n\nAcked-by: Kristian Høgsberg \n\n> Signed-off-by: Pierre Habouzit \n> ---\n> \n> I'm sorry about the \"From\" I don't intend to \"steal\" the patch in any\n> sense, it's just an alternate proposal.\n\nNo worries, I'm glad to see this move forward.\n\n> oh and I don't grok what OPTION_LAST is for, so I left it apart, but\n> it seems unused ?\n\nOh, kill that. I used that as the option array terminator before we\nswitched to ARRAY_SIZE().\n\n> \n> Makefile | 2 +-\n> parse-options.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++\n> parse-options.h | 29 ++++++++++\n> 3 files changed, 184 insertions(+), 1 deletions(-)\n> create mode 100644 parse-options.c\n> create mode 100644 parse-options.h\n> \n> diff --git a/Makefile b/Makefile\n> index 62bdac6..d90e959 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -310,7 +310,7 @@ LIB_OBJS = \\\n> \talloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \\\n> \tcolor.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \\\n> \tconvert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \\\n> -\ttransport.o bundle.o\n> +\ttransport.o bundle.o parse-options.o\n> \n> BUILTIN_OBJS = \\\n> \tbuiltin-add.o \\\n> diff --git a/parse-options.c b/parse-options.c\n> new file mode 100644\n> index 0000000..eb3ff40\n> --- /dev/null\n> +++ b/parse-options.c\n> @@ -0,0 +1,154 @@\n> +#include \"git-compat-util.h\"\n> +#include \"parse-options.h\"\n> +\n> +struct optparse_t {\n> +\tconst char **argv;\n> +\tint argc;\n> +\tconst char *opt;\n> +};\n> +\n> +static inline const char *skippfx(const char *str, const char *prefix)\n> +{\n> +\tsize_t len = strlen(prefix);\n> +\treturn strncmp(str, prefix, len) ? NULL : str + len;\n> +}\n> +\n> +static int opterror(struct option *opt, const char *reason, int shorterr)\n> +{\n> +\tif (shorterr) {\n> +\t\treturn error(\"switch `%c' %s\", opt->short_name, reason);\n> +\t} else {\n> +\t\treturn error(\"option `%s' %s\", opt->long_name, reason);\n> +\t}\n> +}\n\noption/switch?\n\n> +static int get_value(struct optparse_t *p, struct option *opt,\n> +\t\t\t\t\t int boolean, int shorterr)\n> +{\n> +\tswitch (opt->type) {\n> +\t\tconst char *s;\n> +\t\tint v;\n> +\n> +\t case OPTION_BOOLEAN:\n> +\t\t*(int *)opt->value = boolean;\n> +\t\treturn 0;\n> +\n> +\t case OPTION_STRING:\n> +\t\tif (p->opt && *p->opt) {\n> +\t\t\t*(const char **)opt->value = p->opt;\n> +\t\t\tp->opt = NULL;\n> +\t\t} else {\n> +\t\t\tif (p->argc < 1)\n> +\t\t\t\treturn opterror(opt, \"requires a value\", shorterr);\n> +\t\t\t*(const char **)opt->value = *++p->argv;\n> +\t\t\tp->argc--;\n> +\t\t}\n> +\t\treturn 0;\n> +\n> +\t case OPTION_INTEGER:\n> +\t\tif (p->opt && *p->opt) {\n> +\t\t\tv = strtol(p->opt, (char **)&s, 10);\n> +\t\t\tp->opt = NULL;\n> +\t\t} else {\n> +\t\t\tif (p->argc < 1)\n> +\t\t\t\treturn opterror(opt, \"requires a value\", shorterr);\n> +\t\t\tv = strtol(*++p->argv, (char **)&s, 10);\n> +\t\t\tp->argc--;\n> +\t\t}\n> +\t\tif (*s)\n> +\t\t\treturn opterror(opt, \"expects a numerical value\", shorterr);\n> +\t\t*(int *)opt->value = v;\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tabort();\n> +}\n> +\n> +static int parse_short_opt(struct optparse_t *p, struct option *options, int count)\n> +{\n> +\tint i;\n> +\n> +\tfor (i = 0; i < count; i++) {\n> +\t\tif (options[i].short_name == *p->opt) {\n> +\t\t\tp->opt++;\n> +\t\t\treturn get_value(p, options + i, 1, 1);\n> +\t\t}\n> +\t}\n> +\treturn error(\"unknown switch `%c'\", *p->opt);\n> +}\n> +\n> +static int parse_long_opt(struct optparse_t *p, const char *arg,\n> + struct option *options, int count)\n> +{\n> +\tint boolean = 1;\n> +\tint i;\n> +\n> +\tfor (i = 0; i < count; i++) {\n> +\t\tconst char *rest;\n> +\t\t\n> +\t\tif (!options[i].long_name)\n> +\t\t\tcontinue;\n> +\n> +\t\trest = skippfx(arg, options[i].long_name);\n> +\t\tif (!rest && options[i].type == OPTION_BOOLEAN) {\n> +\t\t\tif (!rest && skippfx(arg, \"no-\")) {\n> +\t\t\t\trest = skippfx(arg + 3, options[i].long_name);\n> +\t\t\t\tboolean = 0;\n> +\t\t\t}\n> +\t\t\tif (rest && *rest == '=')\n> +\t\t\t\treturn opterror(options + i, \"takes no value\", 0);\n> +\t\t}\n> +\t\tif (!rest || (*rest && *rest != '='))\n> +\t\t\tcontinue;\n> +\t\tif (*rest) {\n> +\t\t\tp->opt = rest;\n> +\t\t}\n> +\t\treturn get_value(p, options + i, boolean, 0);\n> +\t}\n> +\treturn error(\"unknown option `%s'\", arg);\n> +}\n> +\n> +int parse_options(int argc, const char **argv,\n> +\t\t struct option *options, int count,\n> +\t\t const char *usage_string)\n> +{\n> +\tstruct optparse_t optp = { argv + 1, argc - 1, NULL };\n> +\tint j = 0;\n> +\n> +\twhile (optp.argc) {\n> +\t\tconst char *arg = optp.argv[0];\n> +\n> +\t\tif (*arg != '-' || !arg[1]) {\n> +\t\t\targv[j++] = *optp.argv++;\n> +\t\t\toptp.argc--;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tif (arg[1] != '-') {\n> +\t\t\toptp.opt = arg + 1;\n> +\t\t\twhile (*optp.opt) {\n> +\t\t\t\tif (parse_short_opt(&optp, options, count) < 0) {\n> +\t\t\t\t\tusage(usage_string);\n> +\t\t\t\t\treturn -1;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t\toptp.argc--;\n> +\t\t\toptp.argv++;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tif (!arg[2]) /* \"--\" */\n> +\t\t\tbreak;\n> +\n> +\t\tif (parse_long_opt(&optp, arg + 2, options, count)) {\n> +\t\t\tusage(usage_string);\n> +\t\t\treturn -1;\n> +\t\t}\n> +\t\toptp.argc--;\n> +\t\toptp.argv++;\n> +\t}\n> +\n> +\tmemmove(argv + j, optp.argv, optp.argc * sizeof(argv));\n> +\targv[j + optp.argc] = NULL;\n> +\treturn j + optp.argc;\n> +}\n> diff --git a/parse-options.h b/parse-options.h\n> new file mode 100644\n> index 0000000..e4749d0\n> --- /dev/null\n> +++ b/parse-options.h\n> @@ -0,0 +1,29 @@\n> +#ifndef PARSE_OPTIONS_H\n> +#define PARSE_OPTIONS_H\n> +\n> +enum option_type {\n> +\tOPTION_BOOLEAN,\n> +\tOPTION_STRING,\n> +\tOPTION_INTEGER,\n> +#if 0\n> +\tOPTION_LAST,\n> +#endif\n> +};\n> +\n> +struct option {\n> +\tenum option_type type;\n> +\tconst char *long_name;\n> +\tchar short_name;\n> +\tvoid *value;\n> +};\n> +\n> +/* parse_options() will filter out the processed options and leave the\n> + * non-option argments in argv[]. The return value is the number of\n> + * arguments left in argv[].\n> + */\n> +\n> +extern int parse_options(int argc, const char **argv,\n> +\t\t\t struct option *options, int count,\n> +\t\t\t const char *usage_string);\n> +\n> +#endif\n"},{"id":"54963","messageId":"598D5675D34BE349929AF5EDE9B03E2701624FD6@az33exm24.fsl.freescale.net","threadId":"10133","inReplyTo":"20071005144540.GM19879@artemis.corp","subject":"RE: [ALTERNATE PATCH] Add a simple option parser.","fromName":"Medve Emilian-EMMEDVE1","fromEmail":"emilian.medve@freescale.com","sentAt":"2007-10-05T15:45:36Z","receivedAt":"2007-10-05T15:45:36Z","isPatch":true,"sender":{"key":"emilian.medve@freescale.com","avatar":null},"body":"Hi,\n\n\nYou probably already considered and rejected the GNU argp parser. I used it before and I'd like to know reasons I should stay away from it.\n\n\nCheers,\nEmil.\n\n\n> -----Original Message-----\n> From: git-owner@vger.kernel.org \n> [mailto:git-owner@vger.kernel.org] On Behalf Of Pierre Habouzit\n> Sent: Friday, October 05, 2007 9:46 AM\n> To: Mike Hommey\n> Cc: Kristian Høgsberg; git@vger.kernel.org; Junio C Hamano\n> Subject: Re: [ALTERNATE PATCH] Add a simple option parser.\n> \n> On Fri, Oct 05, 2007 at 02:30:14PM +0000, Mike Hommey wrote:\n> > On Fri, Oct 05, 2007 at 04:25:07PM +0200, Pierre Habouzit \n> wrote:\n> > > The option parser takes argc, argv, an array of struct option\n> > > and a usage string. Each of the struct option elements \n> in the array\n> > > describes a valid option, its type and a pointer to the \n> location where the\n> > > value is written. The entry point is parse_options(), \n> which scans through\n> > > the given argv, and matches each option there against the \n> list of valid\n> > > options. During the scan, argv is rewritten to only contain the\n> > > non-option command line arguments and the number of these \n> is returned.\n> > > \n> > > Aggregation of single switches is allowed:\n> > > -rC0 is the same as -r -C 0 (supposing that -C wants an arg).\n> > \n> > I like options aggregation, but I'm not sure aggregating \n> option arguments\n> > is a good idea... I can't even think of an application that does it.\n> \n> You mean like `grep -A1` or `diff -u3` or `ls -w10` ?\n> \n> getopt does that by default as well, so you may not have aware of it,\n> but it's how things work in your system already.\n> \n> btw `ls -rw10` works, though `ls -w10r` drops the 'r' \n> silently. FWIW I\n> don't, in that case, the alternate patch I propose complains \n> about \"10r\"\n> not being a valid integer, and that's because unlike getopt, the patch\n> krh proposed knows what an integer is ;)\n> -- \n> ·O· Pierre Habouzit\n> ··O madcoder@debian.org\n> OOO \n> http://www.madism.org\n"},{"id":"54966","messageId":"20071005155453.GB20305@artemis.corp","threadId":"10133","inReplyTo":"1191598424.7117.10.camel@hinata.boston.redhat.com","subject":"Re: [ALTERNATE PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-05T15:54:53Z","receivedAt":"2007-10-05T15:54:53Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Fri, Oct 05, 2007 at 03:33:44PM +0000, Kristian Høgsberg wrote:\n> On Fri, 2007-10-05 at 16:25 +0200, Pierre Habouzit wrote:\n> > The option parser takes argc, argv, an array of struct option\n> > and a usage string. Each of the struct option elements in the array\n> > describes a valid option, its type and a pointer to the location where the\n> > value is written. The entry point is parse_options(), which scans through\n> > the given argv, and matches each option there against the list of valid\n> > options. During the scan, argv is rewritten to only contain the\n> > non-option command line arguments and the number of these is returned.\n> > \n> > Aggregation of single switches is allowed:\n> > -rC0 is the same as -r -C 0 (supposing that -C wants an arg).\n> > \n> > Boolean switches automatically support the option with the same name,\n> > prefixed with 'no-' to disable the switch:\n> > --no-color / --color only need to have an entry for \"color\".\n> > \n> > Long options are supported either with '=' or without:\n> > --some-option=foo is the same as --some-option foo\n> \n> That looks great, works for me. One comment, though: it looks like\n> you're not sure whether to call these things \"options\" or \"switches\".\n> We should choose one and stick with it.\n\n I use the word \"switch\" when it's a short_option, and \"option\" when\nit's a long one. But maybe the distinction doesn't make sense, and it's\na non-native speaker glitch. I don't care that much btw.\n\n> > oh and I don't grok what OPTION_LAST is for, so I left it apart, but\n> > it seems unused ?\n>\n> Oh, kill that. I used that as the option array terminator before we\n> switched to ARRAY_SIZE().\n\n Okay :)\n\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"54968","messageId":"20071005155647.GC20305@artemis.corp","threadId":"10133","inReplyTo":"598D5675D34BE349929AF5EDE9B03E2701624FD6@az33exm24.fsl.freescale.net","subject":"Re: [ALTERNATE PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-05T15:56:47Z","receivedAt":"2007-10-05T15:56:47Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On ven, oct 05, 2007 at 03:45:36 +0000, Medve Emilian-EMMEDVE1 wrote:\n> You probably already considered and rejected the GNU argp parser. I\n> used it before and I'd like to know reasons I should stay away from\n> it.\n\n Because it's GNU and that it's a heavy dependency to begin with.\nMoreover, getopt_long doesn't deal with argument types (like integers).\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"54969","messageId":"598D5675D34BE349929AF5EDE9B03E2701624FF2@az33exm24.fsl.freescale.net","threadId":"10133","inReplyTo":"20071005155647.GC20305@artemis.corp","subject":"RE: [ALTERNATE PATCH] Add a simple option parser.","fromName":"Medve Emilian-EMMEDVE1","fromEmail":"emilian.medve@freescale.com","sentAt":"2007-10-05T16:10:42Z","receivedAt":"2007-10-05T16:10:42Z","isPatch":true,"sender":{"key":"emilian.medve@freescale.com","avatar":null},"body":"Hi Pierre,\n\n> -----Original Message-----\n> From: Pierre Habouzit [mailto:madcoder@debian.org] \n> Sent: Friday, October 05, 2007 10:57 AM\n> To: Medve Emilian-EMMEDVE1\n> Cc: Mike Hommey; Kristian Høgsberg; git@vger.kernel.org; \n> Junio C Hamano\n> Subject: Re: [ALTERNATE PATCH] Add a simple option parser.\n> \n> On ven, oct 05, 2007 at 03:45:36 +0000, Medve Emilian-EMMEDVE1 wrote:\n> > You probably already considered and rejected the GNU argp parser. I\n> > used it before and I'd like to know reasons I should stay away from\n> > it.\n> \n> Because it's GNU and that it's a heavy dependency to begin with.\n\nSo it's more of a political decision then a technical one?\n\n> Moreover, getopt_long doesn't deal with argument types (like \n> integers).\n\nAFAIK, getopt_long in not argp.\n\n\nCheers,\nEmil.\n"},{"id":"54974","messageId":"86wsu1v8ha.fsf@lola.quinscape.zz","threadId":"10133","inReplyTo":"598D5675D34BE349929AF5EDE9B03E2701624FF2@az33exm24.fsl.freescale.net","subject":"Re: [ALTERNATE PATCH] Add a simple option parser.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-10-05T16:20:33Z","receivedAt":"2007-10-05T16:20:33Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"\"Medve Emilian-EMMEDVE1\" writes:\n\n> Hi Pierre,\n>\n>> -----Original Message-----\n>> From: Pierre Habouzit [mailto:madcoder@debian.org] \n>> Sent: Friday, October 05, 2007 10:57 AM\n>> To: Medve Emilian-EMMEDVE1\n>> Cc: Mike Hommey; Kristian Høgsberg; git@vger.kernel.org; \n>> Junio C Hamano\n>> Subject: Re: [ALTERNATE PATCH] Add a simple option parser.\n>> \n>> On ven, oct 05, 2007 at 03:45:36 +0000, Medve Emilian-EMMEDVE1 wrote:\n>> > You probably already considered and rejected the GNU argp parser. I\n>> > used it before and I'd like to know reasons I should stay away from\n>> > it.\n>> \n>> Because it's GNU and that it's a heavy dependency to begin with.\n>\n> So it's more of a political decision then a technical one?\n\nWell, if it is GNU then it is likely to mean GPLv3 (or GPLv3+) at some\npoint of time, though it should certainly be possible for now to still\nsecure a v2-licensed version (either GPL or LGPL).\n\nGNU also means a different coding and indentation style.\n\nPersonally, I couldn't care less about both points (I prefer the GNU\ncoding style anyway), but that's for the maintainer to decide, and one\nalso has to take into account the effect on developer motivation.\n\nAnd the typical git developer AFAICT prefers to consider themselves as\nunaligned with GNU and the FSF as much as possible.\n\n-- \nDavid Kastrup\n"},{"id":"54975","messageId":"alpine.LFD.0.999.0710050924530.23684@woody.linux-foundation.org","threadId":"10133","inReplyTo":"598D5675D34BE349929AF5EDE9B03E2701624FF2@az33exm24.fsl.freescale.net","subject":"RE: [ALTERNATE PATCH] Add a simple option parser.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-05T16:28:32Z","receivedAt":"2007-10-05T16:28:32Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 5 Oct 2007, Medve Emilian-EMMEDVE1 wrote:\n> > \n> > Because it's GNU and that it's a heavy dependency to begin with.\n> \n> So it's more of a political decision then a technical one?\n\nI'd *strongly* argue against new dependencies unless they buy us \nsomething major.\n\nWe've been good at cutting them down, including any required libraries \ninternally. We shouldn't add new ones.\n\nSo we'd have to include GNU getopt sources with the git tree, at which \npoint any advantage would be gone. Might as well include a private and \nsimpler version of our own.\n\n\t\tLinus\n"},{"id":"54978","messageId":"20071005163846.GE20305@artemis.corp","threadId":"10133","inReplyTo":"86wsu1v8ha.fsf@lola.quinscape.zz","subject":"Re: [ALTERNATE PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-05T16:38:46Z","receivedAt":"2007-10-05T16:38:46Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Fri, Oct 05, 2007 at 04:20:33PM +0000, David Kastrup wrote:\n> \"Medve Emilian-EMMEDVE1\" writes:\n> \n> > Hi Pierre,\n> >\n> >> -----Original Message-----\n> >> From: Pierre Habouzit [mailto:madcoder@debian.org] \n> >> Sent: Friday, October 05, 2007 10:57 AM\n> >> To: Medve Emilian-EMMEDVE1\n> >> Cc: Mike Hommey; Kristian Høgsberg; git@vger.kernel.org; \n> >> Junio C Hamano\n> >> Subject: Re: [ALTERNATE PATCH] Add a simple option parser.\n> >> \n> >> On ven, oct 05, 2007 at 03:45:36 +0000, Medve Emilian-EMMEDVE1 wrote:\n> >> > You probably already considered and rejected the GNU argp parser. I\n> >> > used it before and I'd like to know reasons I should stay away from\n> >> > it.\n> >> \n> >> Because it's GNU and that it's a heavy dependency to begin with.\n> >\n> > So it's more of a political decision then a technical one?\n> \n> Well, if it is GNU then it is likely to mean GPLv3 (or GPLv3+) at some\n> point of time, though it should certainly be possible for now to still\n> secure a v2-licensed version (either GPL or LGPL).\n\n That is an issue indeed.\n\n> And the typical git developer AFAICT prefers to consider themselves as\n> unaligned with GNU and the FSF as much as possible.\n\n And is nothing near reality in my case.\n\n The real issue is dependency and bloat. getopt_long would need the GNU\nimplementation, That I believe depends upon gettext, and argp is just\nbloated, and I'm not even sure it's distributed outside from the glibc\nanyways.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"54979","messageId":"598D5675D34BE349929AF5EDE9B03E270162501A@az33exm24.fsl.freescale.net","threadId":"10133","inReplyTo":"alpine.LFD.0.999.0710050924530.23684@woody.linux-foundation.org","subject":"RE: [ALTERNATE PATCH] Add a simple option parser.","fromName":"Medve Emilian-EMMEDVE1","fromEmail":"emilian.medve@freescale.com","sentAt":"2007-10-05T16:41:48Z","receivedAt":"2007-10-05T16:41:48Z","isPatch":true,"sender":{"key":"emilian.medve@freescale.com","avatar":null},"body":"Hello Linus,\n\n\n> On Fri, 5 Oct 2007, Medve Emilian-EMMEDVE1 wrote:\n> > > \n> > > Because it's GNU and that it's a heavy dependency to begin with.\n> > \n> > So it's more of a political decision then a technical one?\n> \n> I'd *strongly* argue against new dependencies unless they buy us \n> something major.\n> \n> We've been good at cutting them down, including any required \n> libraries \n> internally. We shouldn't add new ones.\n> \n> So we'd have to include GNU getopt sources with the git tree, \n> at which \n> point any advantage would be gone. Might as well include a \n> private and \n> simpler version of our own.\n\n\n>From what I understand argp is part of glibc.\n\n\nCheers,\nEmil.\n"},{"id":"54982","messageId":"20071005164923.GF20305@artemis.corp","threadId":"10133","inReplyTo":"598D5675D34BE349929AF5EDE9B03E270162501A@az33exm24.fsl.freescale.net","subject":"Re: [ALTERNATE PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-05T16:49:23Z","receivedAt":"2007-10-05T16:49:23Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Fri, Oct 05, 2007 at 04:41:48PM +0000, Medve Emilian-EMMEDVE1 wrote:\n> Hello Linus,\n> \n> \n> > On Fri, 5 Oct 2007, Medve Emilian-EMMEDVE1 wrote:\n> > > > \n> > > > Because it's GNU and that it's a heavy dependency to begin with.\n> > > \n> > > So it's more of a political decision then a technical one?\n> > \n> > I'd *strongly* argue against new dependencies unless they buy us\n> > something major.\n> > \n> > We've been good at cutting them down, including any required\n> > libraries internally. We shouldn't add new ones.\n> > \n> > So we'd have to include GNU getopt sources with the git tree, at\n> > which point any advantage would be gone. Might as well include a\n> > private and simpler version of our own.\n> \n> \n> From what I understand argp is part of glibc.\n\n And of course requiring the glibc would be a big step forward for the\nmsys (or AIX, or HP-UX, or …) port !\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"},{"id":"54983","messageId":"alpine.LFD.0.999.0710050950550.23684@woody.linux-foundation.org","threadId":"10133","inReplyTo":"598D5675D34BE349929AF5EDE9B03E270162501A@az33exm24.fsl.freescale.net","subject":"RE: [ALTERNATE PATCH] Add a simple option parser.","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-10-05T16:51:34Z","receivedAt":"2007-10-05T16:51:34Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Fri, 5 Oct 2007, Medve Emilian-EMMEDVE1 wrote:\n> \n> From what I understand argp is part of glibc.\n\nSo are you arguing that we include all of glibc just to get git to be \nportable?\n\nThat's even worse.\n\n\t\tLinus\n"},{"id":"55011","messageId":"20071006084611.GE3619MdfPADPa@greensroom.kotnet.org","threadId":"10133","inReplyTo":"20071005163846.GE20305@artemis.corp","subject":"Re: [ALTERNATE PATCH] Add a simple option parser.","fromName":"Sven Verdoolaege","fromEmail":"skimo@kotnet.org","sentAt":"2007-10-06T08:46:11Z","receivedAt":"2007-10-06T08:46:11Z","isPatch":true,"sender":{"key":"skimo@kotnet.org","avatar":null},"body":"On Fri, Oct 05, 2007 at 06:38:46PM +0200, Pierre Habouzit wrote:\n> The real issue is dependency and bloat. getopt_long would need the GNU\n> implementation, That I believe depends upon gettext, and argp is just\n> bloated, and I'm not even sure it's distributed outside from the glibc\n> anyways.\n\nIt's part of gnulib (http://savannah.gnu.org/git/?group=gnulib).\nIncluding argp (and all its dependencies) is as easy as running some\ngnulib command.\nIt does have some bloat, but for my project it was definitely more\nconvenient to include it than to write my own parser and the bloat\nis still acceptable I suppose...\n\nbash-3.00$ du lib m4\n572 lib\n208 m4\nbash-3.00$ du -s .\n20348 .\n\n(In a source tree without the git repo.)\n\nskimo\n"},{"id":"55071","messageId":"20071007170154.GG10024@artemis.corp","threadId":"10133","inReplyTo":"20071005142507.GL19879@artemis.corp","subject":"Re: [ALTERNATE PATCH] Add a simple option parser.","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-10-07T17:01:54Z","receivedAt":"2007-10-07T17:01:54Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":" FWIW this patch has some issues with long options parsing, I have a\nfix, but am trying to migrate more builtins to this parser to see how\nwell it behaves.\n\n-- \n·O· Pierre Habouzit\n··O madcoder@debian.org\nOOO http://www.madism.org\n"}]}