Re: [RFC] Re: Convert 'git blame' to parse_options()

From

Johannes Schindelin <johannes.schindelin@gmx.de>

Date

Jun 23, 2008, 12:26 UTC

Message-ID

<alpine.DEB.1.00.0806231312130.6440@racer>

In-Reply-To

<20080623082223.GA12130@artemis.madism.org>

Hi,

On Mon, 23 Jun 2008, Pierre Habouzit wrote:

> This "PARSE_OPT_IGNORE_UNRECOGNIZED" thing has been discussed many times 
> in the past, but it just doesn't fly.
> 
> Though to help migrations we can probably introduce a new parse option
> construct that would be a callback that is responsible for dealing with
> "things" the upper level parser doesn't know about, something where the
> callback could be:
> 
> enum {
>     FLAG_ERROR = -1,
>     FLAG_NOT_FOR_ME,
>     FLAG_IS_FOR_ME,
>     FLAG_AND_VALUE_ARE_FOR_ME,
> }
> 
> int (*parse_opt_unknown_cb)(int shortopt, const char *longopt,
>                             const char *value, void *priv);

However, I think it buys us more trouble than it saves us.

Thinking about the recursive approach again, I came up with this POC:

Subtables can recurse into subtables, too.

Example:

	#define null ((struct diff_options *)NULL)
	struct option diff__options[] = {
		OPT_INTEGER('l', NULL, &null->rename_limit,
			"set rename limit"),
		[...]
	};

	struct option diff_files__options[] = {
		[...]
		OPTION_OPTIONS(diff__options, &revopt.diff_options),
		[...]
	};

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 parse-options.c      |   42 ++++++++++++++++++++++++++++++++++++++++++
 parse-options.h      |    5 +++++
 test-parse-options.c |   16 ++++++++++++++++
 3 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index b8bde2b..c13c503 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1,5 +1,6 @@
 #include "git-compat-util.h"
 #include "parse-options.h"
+#include "cache.h"
 
 #define OPT_SHORT 1
 #define OPT_UNSET 2
@@ -247,6 +248,41 @@ void check_typos(const char *arg, const struct option *options)
 	}
 }
 
+struct growable_options {
+	int nr, alloc;
+	struct option *options;
+};
+
+static void *add_base_var(void *value, void *base_var)
+{
+	return (void *)(((char *)base_var) +
+			(((char *)value) - ((char *)NULL)));
+}
+
+static void expand_option_tables(const struct option *options, void *base_var,
+		struct growable_options *result, int add_option_end)
+{
+	for (; options->type != OPTION_END; options++) {
+		if (options->type == OPTION_OPTIONS)
+			expand_option_tables(options->option_table,
+					add_base_var(options->value, base_var),
+					result, 0);
+		else {
+			ALLOC_GROW(result->options,
+					result->nr + 1, result->alloc);
+			result->options[result->nr] = *options;
+			if (base_var)
+				result->options[result->nr].value =
+					add_base_var(options->value, base_var);
+			result->nr++;
+		}
+	}
+	if (add_option_end) {
+		ALLOC_GROW(result->options, result->nr + 1, result->alloc);
+		result->options[result->nr++].type = OPTION_END;
+	}
+}
+
 static NORETURN void usage_with_options_internal(const char * const *,
                                                  const struct option *, int);
 
@@ -254,6 +290,10 @@ int parse_options(int argc, const char **argv, const struct option *options,
                   const char * const usagestr[], int flags)
 {
 	struct optparse_t args = { argv + 1, argv, argc - 1, 0, NULL };
+	struct growable_options growable_options = { 0, 0, NULL };
+
+	expand_option_tables(options, NULL, &growable_options, 1);
+	options = growable_options.options;
 
 	for (; args.argc; args.argc--, args.argv++) {
 		const char *arg = args.argv[0];
@@ -298,6 +338,8 @@ int parse_options(int argc, const char **argv, const struct option *options,
 			usage_with_options(usagestr, options);
 	}
 
+	free(growable_options.options);
+
 	memmove(args.out + args.cpidx, args.argv, args.argc * sizeof(*args.out));
 	args.out[args.cpidx + args.argc] = NULL;
 	return args.cpidx + args.argc;
diff --git a/parse-options.h b/parse-options.h
index 4ee443d..be6e858 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -15,6 +15,8 @@ enum parse_opt_type {
 	OPTION_STRING,
 	OPTION_INTEGER,
 	OPTION_CALLBACK,
+	/* recursing into another options table */
+	OPTION_OPTIONS,
 };
 
 enum parse_opt_flags {
@@ -83,6 +85,7 @@ struct option {
 	int flags;
 	parse_opt_cb *callback;
 	intptr_t defval;
+	struct option *option_table;
 };
 
 #define OPT_END()                   { OPTION_END }
@@ -99,6 +102,8 @@ struct option {
 	  parse_opt_approxidate_cb }
 #define OPT_CALLBACK(s, l, v, a, h, f) \
 	{ OPTION_CALLBACK, (s), (l), (v), (a), (h), 0, (f) }
+#define OPT_OPTIONS(table, base_var) \
+	{ OPTION_OPTIONS, 0, NULL, base_var, NULL, NULL, 0, NULL, 0, table }
 
 /* parse_options() will filter out the processed options and leave the
  * non-option argments in argv[].
diff --git a/test-parse-options.c b/test-parse-options.c
index 2a79e72..5032e71 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -18,8 +18,21 @@ int length_callback(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+struct some_struct {
+	int an_int;
+	char *a_string;
+};
+#define some_struct_null ((struct some_struct *)NULL)
+struct option an_option_table[] = {
+	OPT_GROUP("An option table"),
+	OPT_INTEGER('Y', NULL, &some_struct_null->an_int, "an integer"),
+	OPT_STRING(0, "a-string", &some_struct_null->a_string,
+		"a-string", "get another string"),
+};
+
 int main(int argc, const char **argv)
 {
+	struct some_struct some = { 0, NULL };
 	const char *usage[] = {
 		"test-parse-options <options>",
 		NULL
@@ -42,6 +55,7 @@ int main(int argc, const char **argv)
 		OPT_STRING('o', NULL, &string, "str", "get another string"),
 		OPT_SET_PTR(0, "default-string", &string,
 			"set string to default", (unsigned long)"default"),
+		OPT_OPTIONS(an_option_table, (void *)&some),
 		OPT_GROUP("Magic arguments"),
 		OPT_ARGUMENT("quux", "means --quux"),
 		OPT_GROUP("Standard options"),
@@ -62,6 +76,8 @@ int main(int argc, const char **argv)
 	printf("verbose: %d\n", verbose);
 	printf("quiet: %s\n", quiet ? "yes" : "no");
 	printf("dry run: %s\n", dry_run ? "yes" : "no");
+	printf("an int: %d\n", some.an_int);
+	printf("a string: %s\n", some.a_string);
 
 	for (i = 0; i < argc; i++)
 		printf("arg %02d: %s\n", i, argv[i]);

-- 
1.5.6.127.g3fb9f
-- snap --


Maybe this is food for thought.

Ciao,
Dscho