[PATCH 0/5] grep parseopt conversion

[PATCH 0/5] grep parseopt conversion

[René Scharfe]

This series converts git-grep to parseopt.  The first three patches
extend the parser to accept inverse bit options, numerical options
and options without dashes:

   [PATCH 1/5] parseopt: add OPT_NEGBIT
   [PATCH 2/5] parseopt: add OPT_NUMBER_CALLBACK
   [PATCH 3/5] parseopt: add PARSE_OPT_NODASH

The fourth patch is a cleanup:

   [PATCH 4/5] grep: remove global variable builtin_grep

The last patch uses the added infrastructure in git-grep:

   [PATCH 5/5] grep: use parseopt

 Documentation/technical/api-parse-options.txt |   12 +
 builtin-grep.c                                |  416 +++++++++++--------------
 grep.h                                        |   28 +-
 parse-options.c                               |   63 ++++-
 parse-options.h                               |   14 +-
 t/t0040-parse-options.sh                      |   56 ++++-
 test-parse-options.c                          |   11 +
 7 files changed, 348 insertions(+), 252 deletions(-)

[PATCH 1/5] parseopt: add OPT_NEGBIT

[René Scharfe]

Add OPTION_NEGBIT and OPT_NEGBIT, mirroring OPTION_BIT and OPT_BIT.
OPT_NEGBIT can be used together with OPT_BIT to define two options
that cancel each other out.

Note: this patch removes the reminder from the test script because
it adds a test for --no-or4 and there already was one for --or4.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 Documentation/technical/api-parse-options.txt |    4 +++
 parse-options.c                               |    8 ++++++
 parse-options.h                               |    2 +
 t/t0040-parse-options.sh                      |   31 +++++++++++++++++++++++-
 test-parse-options.c                          |    1 +
 5 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index e66ca9f..794194b 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -137,6 +137,10 @@ There are some macros to easily define options:
 	Introduce a boolean option.
 	If used, `int_var` is bitwise-ored with `mask`.
 
+`OPT_NEGBIT(short, long, &int_var, description, mask)`::
+	Introduce a boolean option.
+	If used, `int_var` is bitwise-anded with the inverted `mask`.
+
 `OPT_SET_INT(short, long, &int_var, description, integer)`::
 	Introduce a boolean option.
 	If used, set `int_var` to `integer`.
diff --git a/parse-options.c b/parse-options.c
index cf71bcf..a8c05e3 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -50,6 +50,7 @@ static int get_value(struct parse_opt_ctx_t *p,
 			/* FALLTHROUGH */
 		case OPTION_BOOLEAN:
 		case OPTION_BIT:
+		case OPTION_NEGBIT:
 		case OPTION_SET_INT:
 		case OPTION_SET_PTR:
 			return opterror(opt, "takes no value", flags);
@@ -66,6 +67,13 @@ static int get_value(struct parse_opt_ctx_t *p,
 			*(int *)opt->value |= opt->defval;
 		return 0;
 
+	case OPTION_NEGBIT:
+		if (unset)
+			*(int *)opt->value |= opt->defval;
+		else
+			*(int *)opt->value &= ~opt->defval;
+		return 0;
+
 	case OPTION_BOOLEAN:
 		*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
 		return 0;
diff --git a/parse-options.h b/parse-options.h
index b54eec1..f1e2452 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -8,6 +8,7 @@ enum parse_opt_type {
 	OPTION_GROUP,
 	/* options with no arguments */
 	OPTION_BIT,
+	OPTION_NEGBIT,
 	OPTION_BOOLEAN, /* _INCR would have been a better name */
 	OPTION_SET_INT,
 	OPTION_SET_PTR,
@@ -93,6 +94,7 @@ struct option {
 #define OPT_ARGUMENT(l, h)          { OPTION_ARGUMENT, 0, (l), NULL, NULL, (h) }
 #define OPT_GROUP(h)                { OPTION_GROUP, 0, NULL, NULL, NULL, (h) }
 #define OPT_BIT(s, l, v, h, b)      { OPTION_BIT, (s), (l), (v), NULL, (h), 0, NULL, (b) }
+#define OPT_NEGBIT(s, l, v, h, b)   { OPTION_NEGBIT, (s), (l), (v), NULL, (h), 0, NULL, (b) }
 #define OPT_BOOLEAN(s, l, v, h)     { OPTION_BOOLEAN, (s), (l), (v), NULL, (h) }
 #define OPT_SET_INT(s, l, v, h, i)  { OPTION_SET_INT, (s), (l), (v), NULL, (h), 0, NULL, (i) }
 #define OPT_SET_PTR(s, l, v, h, p)  { OPTION_SET_PTR, (s), (l), (v), NULL, (h), 0, NULL, (p) }
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index e38241c..9054ed6 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -12,6 +12,7 @@ usage: test-parse-options <options>
 
     -b, --boolean         get a boolean
     -4, --or4             bitwise-or boolean with ...0100
+    --neg-or4             same as --no-or4
 
     -i, --integer <n>     get a integer
     -j <n>                get a integer, too
@@ -245,7 +246,33 @@ test_expect_success 'OPT_BIT() and OPT_SET_INT() work' '
 	test_cmp expect output
 '
 
-# --or4
-# --no-or4
+test_expect_success 'OPT_NEGBIT() and OPT_SET_INT() work' '
+	test-parse-options --set23 -bbbbb --neg-or4 > output 2> output.err &&
+	test ! -s output.err &&
+	test_cmp expect output
+'
+
+cat > expect <<EOF
+boolean: 6
+integer: 0
+timestamp: 0
+string: (not set)
+abbrev: 7
+verbose: 0
+quiet: no
+dry run: no
+EOF
+
+test_expect_success 'OPT_BIT() works' '
+	test-parse-options -bb --or4 > output 2> output.err &&
+	test ! -s output.err &&
+	test_cmp expect output
+'
+
+test_expect_success 'OPT_NEGBIT() works' '
+	test-parse-options -bb --no-neg-or4 > output 2> output.err &&
+	test ! -s output.err &&
+	test_cmp expect output
+'
 
 test_done
diff --git a/test-parse-options.c b/test-parse-options.c
index 61d2c39..eddc026 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -29,6 +29,7 @@ int main(int argc, const char **argv)
 		OPT_BOOLEAN('b', "boolean", &boolean, "get a boolean"),
 		OPT_BIT('4', "or4", &boolean,
 			"bitwise-or boolean with ...0100", 4),
+		OPT_NEGBIT(0, "neg-or4", &boolean, "same as --no-or4", 4),
 		OPT_GROUP(""),
 		OPT_INTEGER('i', "integer", &integer, "get a integer"),
 		OPT_INTEGER('j', NULL, &integer, "get a integer, too"),
-- 
1.6.3

Re: [PATCH 1/5] parseopt: add OPT_NEGBIT

[Jeff King]

On Thu, May 07, 2009 at 09:44:17PM +0200, René Scharfe wrote:

> Add OPTION_NEGBIT and OPT_NEGBIT, mirroring OPTION_BIT and OPT_BIT.
> OPT_NEGBIT can be used together with OPT_BIT to define two options
> that cancel each other out.

Nice, this neatly fixes the problem with --no-no-empty-directory in
"ls-files" discussed here:

  http://article.gmane.org/gmane.comp.version-control.git/112607

We just need this patch on top of René's 1/5:

-- >8 --
Subject: [PATCH] ls-files: make --no-empty-directory properly negatable

This option was specified to parseopt as an OPT_BIT; however, we
actually want to _set_ the bit on --no-empty-directory. Thus the
existing implementation used --no-empty-directory, and required
--no-no-empty-directory to negate it.

Now that OPT_NEGBIT exists, we can properly support it as
--empty-directory and --no-empty-directory (but of course
still defaulting to showing empty directories).

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin-ls-files.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index da2daf4..3d59b0e 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -454,7 +454,7 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 		OPT_BIT(0, "directory", &dir.flags,
 			"show 'other' directories' name only",
 			DIR_SHOW_OTHER_DIRECTORIES),
-		OPT_BIT(0, "no-empty-directory", &dir.flags,
+		OPT_NEGBIT(0, "empty-directory", &dir.flags,
 			"don't show empty directories",
 			DIR_HIDE_EMPTY_DIRECTORIES),
 		OPT_BOOLEAN('u', "unmerged", &show_unmerged,
-- 
1.6.3.201.g5d2a4.dirty

[PATCH 2/5] parseopt: add OPT_NUMBER_CALLBACK

[René Scharfe]

Add a way to recognize numerical options.  The number is passed to
a callback function as a string.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 Documentation/technical/api-parse-options.txt |    8 +++++++
 parse-options.c                               |   26 ++++++++++++++++++++++++-
 parse-options.h                               |    4 +++
 t/t0040-parse-options.sh                      |   18 +++++++++++++++++
 test-parse-options.c                          |    8 +++++++
 5 files changed, 63 insertions(+), 1 deletions(-)

diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index 794194b..beca98d 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -170,6 +170,14 @@ There are some macros to easily define options:
 `OPT_ARGUMENT(long, description)`::
 	Introduce a long-option argument that will be kept in `argv[]`.
 
+`OPT_NUMBER_CALLBACK(&var, description, func_ptr)`::
+	Recognize numerical options like -123 and feed the integer as
+	if it was an argument to the function given by `func_ptr`.
+	The result will be put into `var`.  There can be only one such
+	option definition.  It cannot be negated and it takes no
+	arguments.  Short options that happen to be digits take
+	precedence over it.
+
 
 The last element of the array must be `OPT_END()`.
 
diff --git a/parse-options.c b/parse-options.c
index a8c05e3..aaa218d 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -129,11 +129,33 @@ static int get_value(struct parse_opt_ctx_t *p,
 
 static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *options)
 {
+	const struct option *numopt = NULL;
+
 	for (; options->type != OPTION_END; options++) {
 		if (options->short_name == *p->opt) {
 			p->opt = p->opt[1] ? p->opt + 1 : NULL;
 			return get_value(p, options, OPT_SHORT);
 		}
+
+		/*
+		 * Handle the numerical option later, explicit one-digit
+		 * options take precedence over it.
+		 */
+		if (options->type == OPTION_NUMBER)
+			numopt = options;
+	}
+	if (numopt && isdigit(*p->opt)) {
+		size_t len = 1;
+		char *arg;
+		int rc;
+
+		while (isdigit(p->opt[len]))
+			len++;
+		arg = xmemdupz(p->opt, len);
+		p->opt = p->opt[len] ? p->opt + len : NULL;
+		rc = (*numopt->callback)(numopt, arg, 0) ? (-1) : 0;
+		free(arg);
+		return rc;
 	}
 	return -2;
 }
@@ -411,6 +433,8 @@ int usage_with_options_internal(const char * const *usagestr,
 			pos += fprintf(stderr, ", ");
 		if (opts->long_name)
 			pos += fprintf(stderr, "--%s", opts->long_name);
+		if (opts->type == OPTION_NUMBER)
+			pos += fprintf(stderr, "-NUM");
 
 		switch (opts->type) {
 		case OPTION_ARGUMENT:
@@ -447,7 +471,7 @@ int usage_with_options_internal(const char * const *usagestr,
 					pos += fprintf(stderr, " ...");
 			}
 			break;
-		default: /* OPTION_{BIT,BOOLEAN,SET_INT,SET_PTR} */
+		default: /* OPTION_{BIT,BOOLEAN,NUMBER,SET_INT,SET_PTR} */
 			break;
 		}
 
diff --git a/parse-options.h b/parse-options.h
index f1e2452..77919a7 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -6,6 +6,7 @@ enum parse_opt_type {
 	OPTION_END,
 	OPTION_ARGUMENT,
 	OPTION_GROUP,
+	OPTION_NUMBER,
 	/* options with no arguments */
 	OPTION_BIT,
 	OPTION_NEGBIT,
@@ -105,6 +106,9 @@ 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_NUMBER_CALLBACK(v, h, f) \
+	{ OPTION_NUMBER, 0, NULL, (v), NULL, (h), \
+	  PARSE_OPT_NOARG | PARSE_OPT_NONEG, (f) }
 
 /* parse_options() will filter out the processed options and leave the
  * non-option arguments in argv[].
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 9054ed6..8ca62ef 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -30,6 +30,7 @@ String options
 
 Magic arguments
     --quux                means --quux
+    -NUM                  set integer to NUM
 
 Standard options
     --abbrev[=<n>]        use <n> digits to display SHA-1s
@@ -275,4 +276,21 @@ test_expect_success 'OPT_NEGBIT() works' '
 	test_cmp expect output
 '
 
+cat > expect <<EOF
+boolean: 0
+integer: 12345
+timestamp: 0
+string: (not set)
+abbrev: 7
+verbose: 0
+quiet: no
+dry run: no
+EOF
+
+test_expect_success 'OPT_NUMBER_CALLBACK() works' '
+	test-parse-options -12345 > output 2> output.err &&
+	test ! -s output.err &&
+	test_cmp expect output
+'
+
 test_done
diff --git a/test-parse-options.c b/test-parse-options.c
index eddc026..d46eac2 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -19,6 +19,12 @@ int length_callback(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
+int number_callback(const struct option *opt, const char *arg, int unset)
+{
+	*(int *)opt->value = strtol(arg, NULL, 10);
+	return 0;
+}
+
 int main(int argc, const char **argv)
 {
 	const char *usage[] = {
@@ -46,6 +52,8 @@ int main(int argc, const char **argv)
 			"set string to default", (unsigned long)"default"),
 		OPT_GROUP("Magic arguments"),
 		OPT_ARGUMENT("quux", "means --quux"),
+		OPT_NUMBER_CALLBACK(&integer, "set integer to NUM",
+			number_callback),
 		OPT_GROUP("Standard options"),
 		OPT__ABBREV(&abbrev),
 		OPT__VERBOSE(&verbose),
-- 
1.6.3

[PATCH 3/5] parseopt: add PARSE_OPT_NODASH

[René Scharfe]

Add support for options that don't start with a dash.  Initially, they
don't accept arguments and can only be short options, i.e. consist of a
single character.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 parse-options.c          |   29 +++++++++++++++++++++++++++--
 parse-options.h          |    8 ++++++--
 t/t0040-parse-options.sh |    7 +++++++
 test-parse-options.c     |    2 ++
 4 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/parse-options.c b/parse-options.c
index aaa218d..c52b8cc 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -245,6 +245,25 @@ is_abbreviated:
 	return -2;
 }
 
+static int parse_nodash_opt(struct parse_opt_ctx_t *p, const char *arg,
+			    const struct option *options)
+{
+	for (; options->type != OPTION_END; options++) {
+		if (!(options->flags & PARSE_OPT_NODASH))
+			continue;
+		if ((options->flags & PARSE_OPT_OPTARG) ||
+		    !(options->flags & PARSE_OPT_NOARG))
+			die("BUG: dashless options don't support arguments");
+		if (!(options->flags & PARSE_OPT_NONEG))
+			die("BUG: dashless options don't support negation");
+		if (options->long_name)
+			die("BUG: dashless options can't be long");
+		if (options->short_name == arg[0] && arg[1] == '\0')
+			return get_value(p, options, OPT_SHORT);
+	}
+	return -2;
+}
+
 static void check_typos(const char *arg, const struct option *options)
 {
 	if (strlen(arg) < 3)
@@ -295,6 +314,8 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
 		const char *arg = ctx->argv[0];
 
 		if (*arg != '-' || !arg[1]) {
+			if (parse_nodash_opt(ctx, arg, options) == 0)
+				continue;
 			if (ctx->flags & PARSE_OPT_STOP_AT_NON_OPTION)
 				break;
 			ctx->out[ctx->cpidx++] = ctx->argv[0];
@@ -427,8 +448,12 @@ int usage_with_options_internal(const char * const *usagestr,
 			continue;
 
 		pos = fprintf(stderr, "    ");
-		if (opts->short_name)
-			pos += fprintf(stderr, "-%c", opts->short_name);
+		if (opts->short_name) {
+			if (opts->flags & PARSE_OPT_NODASH)
+				pos += fprintf(stderr, "%c", opts->short_name);
+			else
+				pos += fprintf(stderr, "-%c", opts->short_name);
+		}
 		if (opts->long_name && opts->short_name)
 			pos += fprintf(stderr, ", ");
 		if (opts->long_name)
diff --git a/parse-options.h b/parse-options.h
index 77919a7..919b9b4 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -33,6 +33,7 @@ enum parse_opt_option_flags {
 	PARSE_OPT_NONEG   = 4,
 	PARSE_OPT_HIDDEN  = 8,
 	PARSE_OPT_LASTARG_DEFAULT = 16,
+	PARSE_OPT_NODASH = 32,
 };
 
 struct option;
@@ -66,8 +67,11 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
  *   PARSE_OPT_OPTARG: says that the argument is optional (not for BOOLEANs)
  *   PARSE_OPT_NOARG: says that this option takes no argument, for CALLBACKs
  *   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_HIDDEN: this option is skipped in the default usage, and
+ *                     shown only in the full usage.
+ *   PARSE_OPT_LASTARG_DEFAULT: if no argument is given, the default value
+ *                              is used.
+ *   PARSE_OPT_NODASH: this option doesn't start with a dash.
  *
  * `callback`::
  *   pointer to the callback to use for OPTION_CALLBACK.
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 8ca62ef..a40c123 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -31,6 +31,7 @@ String options
 Magic arguments
     --quux                means --quux
     -NUM                  set integer to NUM
+    +                     same as -b
 
 Standard options
     --abbrev[=<n>]        use <n> digits to display SHA-1s
@@ -276,6 +277,12 @@ test_expect_success 'OPT_NEGBIT() works' '
 	test_cmp expect output
 '
 
+test_expect_success 'OPT_BOOLEAN() with PARSE_OPT_NODASH works' '
+	test-parse-options + + + + + + > output 2> output.err &&
+	test ! -s output.err &&
+	test_cmp expect output
+'
+
 cat > expect <<EOF
 boolean: 0
 integer: 12345
diff --git a/test-parse-options.c b/test-parse-options.c
index d46eac2..e0669dc 100644
--- a/test-parse-options.c
+++ b/test-parse-options.c
@@ -54,6 +54,8 @@ int main(int argc, const char **argv)
 		OPT_ARGUMENT("quux", "means --quux"),
 		OPT_NUMBER_CALLBACK(&integer, "set integer to NUM",
 			number_callback),
+		{ OPTION_BOOLEAN, '+', NULL, &boolean, NULL, "same as -b",
+		  PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH },
 		OPT_GROUP("Standard options"),
 		OPT__ABBREV(&abbrev),
 		OPT__VERBOSE(&verbose),
-- 
1.6.3

[PATCH 4/5] grep: remove global variable builtin_grep

[René Scharfe]

Replace the only global variable in builtin-grep.c, builtin_grep, by a
local one and a function parameter with reversed meaning.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 builtin-grep.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index f88a912..620399f 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -20,8 +20,6 @@
 #endif
 #endif
 
-static int builtin_grep;
-
 static int grep_config(const char *var, const char *value, void *cb)
 {
 	struct grep_opt *opt = cb;
@@ -432,7 +430,8 @@ static int external_grep(struct grep_opt *opt, const char **paths, int cached)
 }
 #endif
 
-static int grep_cache(struct grep_opt *opt, const char **paths, int cached)
+static int grep_cache(struct grep_opt *opt, const char **paths, int cached,
+		      int external_grep_allowed)
 {
 	int hit = 0;
 	int nr;
@@ -444,7 +443,7 @@ static int grep_cache(struct grep_opt *opt, const char **paths, int cached)
 	 * we grep through the checked-out files. It tends to
 	 * be a lot more optimized
 	 */
-	if (!cached && !builtin_grep) {
+	if (!cached && external_grep_allowed) {
 		hit = external_grep(opt, paths, cached);
 		if (hit >= 0)
 			return hit;
@@ -574,6 +573,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 {
 	int hit = 0;
 	int cached = 0;
+	int external_grep_allowed = 1;
 	int seen_dashdash = 0;
 	struct grep_opt opt;
 	struct object_array list = { 0, 0, NULL };
@@ -612,7 +612,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!strcmp("--no-ext-grep", arg)) {
-			builtin_grep = 1;
+			external_grep_allowed = 0;
 			continue;
 		}
 		if (!strcmp("-a", arg) ||
@@ -823,7 +823,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	}
 
 	if (opt.color && !opt.color_external)
-		builtin_grep = 1;
+		external_grep_allowed = 0;
 	if (!opt.pattern_list)
 		die("no pattern given.");
 	if ((opt.regflags != REG_NEWLINE) && opt.fixed)
@@ -874,7 +874,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	if (!list.nr) {
 		if (!cached)
 			setup_work_tree();
-		return !grep_cache(&opt, paths, cached);
+		return !grep_cache(&opt, paths, cached, external_grep_allowed);
 	}
 
 	if (cached)
-- 
1.6.3

[PATCH 5/5] grep: use parseopt

[René Scharfe]

Convert git-grep to parseopt.

The bitfields in struct grep_opt are converted to full ints,
increasing its size.  This shouldn't be a problem as there is only a
single instance in memory.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 builtin-grep.c |  406 +++++++++++++++++++++++++-------------------------------
 grep.h         |   28 ++--
 2 files changed, 194 insertions(+), 240 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 620399f..169a91c 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -10,6 +10,7 @@
 #include "tag.h"
 #include "tree-walk.h"
 #include "builtin.h"
+#include "parse-options.h"
 #include "grep.h"
 
 #ifndef NO_EXTERNAL_GREP
@@ -20,6 +21,11 @@
 #endif
 #endif
 
+static char const * const grep_usage[] = {
+	"git grep [options] [-e] <pattern> [<rev>...] [[--] path...]",
+	NULL
+};
+
 static int grep_config(const char *var, const char *value, void *cb)
 {
 	struct grep_opt *opt = cb;
@@ -559,15 +565,86 @@ static int grep_object(struct grep_opt *opt, const char **paths,
 	die("unable to grep from object of type %s", typename(obj->type));
 }
 
-static const char builtin_grep_usage[] =
-"git grep <option>* [-e] <pattern> <rev>* [[--] <path>...]";
+int context_callback(const struct option *opt, const char *arg, int unset)
+{
+	struct grep_opt *grep_opt = opt->value;
+	int value;
+	const char *endp;
+
+	if (unset) {
+		grep_opt->pre_context = grep_opt->post_context = 0;
+		return 0;
+	}
+	value = strtol(arg, (char **)&endp, 10);
+	if (*endp) {
+		return error("switch `%c' expects a numerical value",
+			     opt->short_name);
+	}
+	grep_opt->pre_context = grep_opt->post_context = value;
+	return 0;
+}
+
+int file_callback(const struct option *opt, const char *arg, int unset)
+{
+	struct grep_opt *grep_opt = opt->value;
+	FILE *patterns;
+	int lno = 0;
+	struct strbuf sb;
+
+	patterns = fopen(arg, "r");
+	if (!patterns)
+		die("'%s': %s", arg, strerror(errno));
+	while (strbuf_getline(&sb, patterns, '\n') == 0) {
+		/* ignore empty line like grep does */
+		if (sb.len == 0)
+			continue;
+		append_grep_pattern(grep_opt, strbuf_detach(&sb, NULL), arg,
+				    ++lno, GREP_PATTERN);
+	}
+	fclose(patterns);
+	strbuf_release(&sb);
+	return 0;
+}
+
+int not_callback(const struct option *opt, const char *arg, int unset)
+{
+	struct grep_opt *grep_opt = opt->value;
+	append_grep_pattern(grep_opt, "--not", "command line", 0, GREP_NOT);
+	return 0;
+}
+
+int and_callback(const struct option *opt, const char *arg, int unset)
+{
+	struct grep_opt *grep_opt = opt->value;
+	append_grep_pattern(grep_opt, "--and", "command line", 0, GREP_AND);
+	return 0;
+}
+
+int open_callback(const struct option *opt, const char *arg, int unset)
+{
+	struct grep_opt *grep_opt = opt->value;
+	append_grep_pattern(grep_opt, "(", "command line", 0, GREP_OPEN_PAREN);
+	return 0;
+}
+
+int close_callback(const struct option *opt, const char *arg, int unset)
+{
+	struct grep_opt *grep_opt = opt->value;
+	append_grep_pattern(grep_opt, ")", "command line", 0, GREP_CLOSE_PAREN);
+	return 0;
+}
+
+int pattern_callback(const struct option *opt, const char *arg, int unset)
+{
+	struct grep_opt *grep_opt = opt->value;
+	append_grep_pattern(grep_opt, arg, "-e option", 0, GREP_PATTERN);
+	return 0;
+}
 
-static const char emsg_invalid_context_len[] =
-"%s: invalid context length argument";
-static const char emsg_missing_context_len[] =
-"missing context length argument";
-static const char emsg_missing_argument[] =
-"option requires an argument -%s";
+int help_callback(const struct option *opt, const char *arg, int unset)
+{
+	return -1;
+}
 
 int cmd_grep(int argc, const char **argv, const char *prefix)
 {
@@ -579,6 +656,89 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	struct object_array list = { 0, 0, NULL };
 	const char **paths = NULL;
 	int i;
+	int dummy;
+	struct option options[] = {
+		OPT_BOOLEAN(0, "cached", &cached,
+			"search in index instead of in the work tree"),
+		OPT_GROUP(""),
+		OPT_BOOLEAN('v', "invert-match", &opt.invert,
+			"show non-matching lines"),
+		OPT_BIT('i', "ignore-case", &opt.regflags,
+			"case insensitive matching", REG_ICASE),
+		OPT_BOOLEAN('w', "word-regexp", &opt.word_regexp,
+			"match patterns only at word boundaries"),
+		OPT_SET_INT('a', "text", &opt.binary,
+			"process binary files as text", GREP_BINARY_TEXT),
+		OPT_SET_INT('I', NULL, &opt.binary,
+			"don't match patterns in binary files",
+			GREP_BINARY_NOMATCH),
+		OPT_GROUP(""),
+		OPT_BIT('E', "extended-regexp", &opt.regflags,
+			"use extended POSIX regular expressions", REG_EXTENDED),
+		OPT_NEGBIT('G', "basic-regexp", &opt.regflags,
+			"use basic POSIX regular expressions (default)",
+			REG_EXTENDED),
+		OPT_BOOLEAN('F', "fixed-strings", &opt.fixed,
+			"interpret patterns as fixed strings"),
+		OPT_GROUP(""),
+		OPT_BOOLEAN('n', NULL, &opt.linenum, "show line numbers"),
+		OPT_NEGBIT('h', NULL, &opt.pathname, "don't show filenames", 1),
+		OPT_BIT('H', NULL, &opt.pathname, "show filenames", 1),
+		OPT_NEGBIT(0, "full-name", &opt.relative,
+			"show filenames relative to top directory", 1),
+		OPT_BOOLEAN('l', "files-with-matches", &opt.name_only,
+			"show only filenames instead of matching lines"),
+		OPT_BOOLEAN(0, "name-only", &opt.name_only,
+			"synonym for --files-with-matches"),
+		OPT_BOOLEAN('L', "files-without-match",
+			&opt.unmatch_name_only,
+			"show only the names of files without match"),
+		OPT_BOOLEAN('z', "null", &opt.null_following_name,
+			"print NUL after filenames"),
+		OPT_BOOLEAN('c', "count", &opt.count,
+			"show the number of matches instead of matching lines"),
+		OPT_SET_INT(0, "color", &opt.color, "highlight matches", 1),
+		OPT_GROUP(""),
+		OPT_CALLBACK('C', NULL, &opt, "n",
+			"show <n> context lines before and after matches",
+			context_callback),
+		OPT_INTEGER('B', NULL, &opt.pre_context,
+			"show <n> context lines before matches"),
+		OPT_INTEGER('A', NULL, &opt.post_context,
+			"show <n> context lines after matches"),
+		OPT_NUMBER_CALLBACK(&opt, "shortcut for -C NUM",
+			context_callback),
+		OPT_GROUP(""),
+		OPT_CALLBACK('f', NULL, &opt, "file",
+			"read patterns from file", file_callback),
+		{ OPTION_CALLBACK, 'e', NULL, &opt, "pattern",
+			"match <pattern>", PARSE_OPT_NONEG, pattern_callback },
+		{ OPTION_CALLBACK, 0, "and", &opt, NULL,
+		  "combine patterns specified with -e",
+		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, and_callback },
+		OPT_BOOLEAN(0, "or", &dummy, ""),
+		{ OPTION_CALLBACK, 0, "not", &opt, NULL, "",
+		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, not_callback },
+		{ OPTION_CALLBACK, '(', NULL, &opt, NULL, "",
+		  PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH,
+		  open_callback },
+		{ OPTION_CALLBACK, ')', NULL, &opt, NULL, "",
+		  PARSE_OPT_NOARG | PARSE_OPT_NONEG | PARSE_OPT_NODASH,
+		  close_callback },
+		OPT_BOOLEAN(0, "all-match", &opt.all_match,
+			"show only matches from files that match all patterns"),
+		OPT_GROUP(""),
+#if NO_EXTERNAL_GREP
+		OPT_BOOLEAN(0, "ext-grep", &external_grep_allowed,
+			"allow calling of grep(1) (ignored by this build)"),
+#else
+		OPT_BOOLEAN(0, "ext-grep", &external_grep_allowed,
+			"allow calling of grep(1) (default)"),
+#endif
+		{ OPTION_CALLBACK, 0, "help-all", &options, NULL, "show usage",
+		  PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, help_callback },
+		OPT_END()
+	};
 
 	memset(&opt, 0, sizeof(opt));
 	opt.prefix_length = (prefix && *prefix) ? strlen(prefix) : 0;
@@ -603,223 +763,17 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	 * unrecognized non option is the beginning of the refs list
 	 * that continues up to the -- (if exists), and then paths.
 	 */
-
-	while (1 < argc) {
-		const char *arg = argv[1];
-		argc--; argv++;
-		if (!strcmp("--cached", arg)) {
-			cached = 1;
-			continue;
-		}
-		if (!strcmp("--no-ext-grep", arg)) {
-			external_grep_allowed = 0;
-			continue;
-		}
-		if (!strcmp("-a", arg) ||
-		    !strcmp("--text", arg)) {
-			opt.binary = GREP_BINARY_TEXT;
-			continue;
-		}
-		if (!strcmp("-i", arg) ||
-		    !strcmp("--ignore-case", arg)) {
-			opt.regflags |= REG_ICASE;
-			continue;
-		}
-		if (!strcmp("-I", arg)) {
-			opt.binary = GREP_BINARY_NOMATCH;
-			continue;
-		}
-		if (!strcmp("-v", arg) ||
-		    !strcmp("--invert-match", arg)) {
-			opt.invert = 1;
-			continue;
-		}
-		if (!strcmp("-E", arg) ||
-		    !strcmp("--extended-regexp", arg)) {
-			opt.regflags |= REG_EXTENDED;
-			continue;
-		}
-		if (!strcmp("-F", arg) ||
-		    !strcmp("--fixed-strings", arg)) {
-			opt.fixed = 1;
-			continue;
-		}
-		if (!strcmp("-G", arg) ||
-		    !strcmp("--basic-regexp", arg)) {
-			opt.regflags &= ~REG_EXTENDED;
-			continue;
-		}
-		if (!strcmp("-n", arg)) {
-			opt.linenum = 1;
-			continue;
-		}
-		if (!strcmp("-h", arg)) {
-			opt.pathname = 0;
-			continue;
-		}
-		if (!strcmp("-H", arg)) {
-			opt.pathname = 1;
-			continue;
-		}
-		if (!strcmp("-l", arg) ||
-		    !strcmp("--name-only", arg) ||
-		    !strcmp("--files-with-matches", arg)) {
-			opt.name_only = 1;
-			continue;
-		}
-		if (!strcmp("-L", arg) ||
-		    !strcmp("--files-without-match", arg)) {
-			opt.unmatch_name_only = 1;
-			continue;
-		}
-		if (!strcmp("-z", arg) ||
-		    !strcmp("--null", arg)) {
-			opt.null_following_name = 1;
-			continue;
-		}
-		if (!strcmp("-c", arg) ||
-		    !strcmp("--count", arg)) {
-			opt.count = 1;
-			continue;
-		}
-		if (!strcmp("-w", arg) ||
-		    !strcmp("--word-regexp", arg)) {
-			opt.word_regexp = 1;
-			continue;
-		}
-		if (!prefixcmp(arg, "-A") ||
-		    !prefixcmp(arg, "-B") ||
-		    !prefixcmp(arg, "-C") ||
-		    (arg[0] == '-' && '1' <= arg[1] && arg[1] <= '9')) {
-			unsigned num;
-			const char *scan;
-			switch (arg[1]) {
-			case 'A': case 'B': case 'C':
-				if (!arg[2]) {
-					if (argc <= 1)
-						die(emsg_missing_context_len);
-					scan = *++argv;
-					argc--;
-				}
-				else
-					scan = arg + 2;
-				break;
-			default:
-				scan = arg + 1;
-				break;
-			}
-			if (strtoul_ui(scan, 10, &num))
-				die(emsg_invalid_context_len, scan);
-			switch (arg[1]) {
-			case 'A':
-				opt.post_context = num;
-				break;
-			default:
-			case 'C':
-				opt.post_context = num;
-			case 'B':
-				opt.pre_context = num;
-				break;
-			}
-			continue;
-		}
-		if (!strcmp("-f", arg)) {
-			FILE *patterns;
-			int lno = 0;
-			char buf[1024];
-			if (argc <= 1)
-				die(emsg_missing_argument, arg);
-			patterns = fopen(argv[1], "r");
-			if (!patterns)
-				die("'%s': %s", argv[1], strerror(errno));
-			while (fgets(buf, sizeof(buf), patterns)) {
-				int len = strlen(buf);
-				if (len && buf[len-1] == '\n')
-					buf[len-1] = 0;
-				/* ignore empty line like grep does */
-				if (!buf[0])
-					continue;
-				append_grep_pattern(&opt, xstrdup(buf),
-						    argv[1], ++lno,
-						    GREP_PATTERN);
-			}
-			fclose(patterns);
-			argv++;
-			argc--;
-			continue;
-		}
-		if (!strcmp("--not", arg)) {
-			append_grep_pattern(&opt, arg, "command line", 0,
-					    GREP_NOT);
-			continue;
-		}
-		if (!strcmp("--and", arg)) {
-			append_grep_pattern(&opt, arg, "command line", 0,
-					    GREP_AND);
-			continue;
-		}
-		if (!strcmp("--or", arg))
-			continue; /* no-op */
-		if (!strcmp("(", arg)) {
-			append_grep_pattern(&opt, arg, "command line", 0,
-					    GREP_OPEN_PAREN);
-			continue;
-		}
-		if (!strcmp(")", arg)) {
-			append_grep_pattern(&opt, arg, "command line", 0,
-					    GREP_CLOSE_PAREN);
-			continue;
-		}
-		if (!strcmp("--all-match", arg)) {
-			opt.all_match = 1;
-			continue;
-		}
-		if (!strcmp("-e", arg)) {
-			if (1 < argc) {
-				append_grep_pattern(&opt, argv[1],
-						    "-e option", 0,
-						    GREP_PATTERN);
-				argv++;
-				argc--;
-				continue;
-			}
-			die(emsg_missing_argument, arg);
-		}
-		if (!strcmp("--full-name", arg)) {
-			opt.relative = 0;
-			continue;
-		}
-		if (!strcmp("--color", arg)) {
-			opt.color = 1;
-			continue;
-		}
-		if (!strcmp("--no-color", arg)) {
-			opt.color = 0;
-			continue;
-		}
-		if (!strcmp("--", arg)) {
-			/* later processing wants to have this at argv[1] */
-			argv--;
-			argc++;
-			break;
-		}
-		if (*arg == '-')
-			usage(builtin_grep_usage);
-
-		/* First unrecognized non-option token */
-		if (!opt.pattern_list) {
-			append_grep_pattern(&opt, arg, "command line", 0,
-					    GREP_PATTERN);
-			break;
-		}
-		else {
-			/* We are looking at the first path or rev;
-			 * it is found at argv[1] after leaving the
-			 * loop.
-			 */
-			argc++; argv--;
-			break;
-		}
+	argc = parse_options(argc, argv, options, grep_usage,
+			     PARSE_OPT_KEEP_DASHDASH |
+			     PARSE_OPT_STOP_AT_NON_OPTION |
+			     PARSE_OPT_NO_INTERNAL_HELP);
+
+	/* First unrecognized non-option token */
+	if (argc > 0 && !opt.pattern_list) {
+		append_grep_pattern(&opt, argv[0], "command line", 0,
+				    GREP_PATTERN);
+		argv++;
+		argc--;
 	}
 
 	if (opt.color && !opt.color_external)
@@ -831,7 +785,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	compile_grep_patterns(&opt);
 
 	/* Check revs and then paths */
-	for (i = 1; i < argc; i++) {
+	for (i = 0; i < argc; i++) {
 		const char *arg = argv[i];
 		unsigned char sha1[20];
 		/* Is it a rev? */
diff --git a/grep.h b/grep.h
index a67005d..464e272 100644
--- a/grep.h
+++ b/grep.h
@@ -61,23 +61,23 @@ struct grep_opt {
 	struct grep_expr *pattern_expression;
 	int prefix_length;
 	regex_t regexp;
-	unsigned linenum:1;
-	unsigned invert:1;
-	unsigned status_only:1;
-	unsigned name_only:1;
-	unsigned unmatch_name_only:1;
-	unsigned count:1;
-	unsigned word_regexp:1;
-	unsigned fixed:1;
-	unsigned all_match:1;
+	int linenum;
+	int invert;
+	int status_only;
+	int name_only;
+	int unmatch_name_only;
+	int count;
+	int word_regexp;
+	int fixed;
+	int all_match;
 #define GREP_BINARY_DEFAULT	0
 #define GREP_BINARY_NOMATCH	1
 #define GREP_BINARY_TEXT	2
-	unsigned binary:2;
-	unsigned extended:1;
-	unsigned relative:1;
-	unsigned pathname:1;
-	unsigned null_following_name:1;
+	int binary;
+	int extended;
+	int relative;
+	int pathname;
+	int null_following_name;
 	int color;
 	char color_match[COLOR_MAXLEN];
 	const char *color_external;
-- 
1.6.3

Re: [PATCH 5/5] grep: use parseopt

[Stephen Boyd]

René Scharfe wrote:
> +int context_callback(const struct option *opt, const char *arg, int unset)
> +{

Shouldn't these callbacks be static?

Re: [PATCH 5/5] grep: use parseopt

[René Scharfe]

Stephen Boyd schrieb:
> René Scharfe wrote:
>> +int context_callback(const struct option *opt, const char *arg, int unset)
>> +{
> 
> Shouldn't these callbacks be static?

Oh, yes, they should.

René

[PATCH] grep: make callback functions static

[René Scharfe]

Suggested by Stephen Boyd: make the callback functions used for option
parsing static.

Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
Patch applies to next.

 builtin-grep.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 169a91c..5308b34 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -565,7 +565,8 @@ static int grep_object(struct grep_opt *opt, const char **paths,
 	die("unable to grep from object of type %s", typename(obj->type));
 }
 
-int context_callback(const struct option *opt, const char *arg, int unset)
+static int context_callback(const struct option *opt, const char *arg,
+			    int unset)
 {
 	struct grep_opt *grep_opt = opt->value;
 	int value;
@@ -584,7 +585,7 @@ int context_callback(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
-int file_callback(const struct option *opt, const char *arg, int unset)
+static int file_callback(const struct option *opt, const char *arg, int unset)
 {
 	struct grep_opt *grep_opt = opt->value;
 	FILE *patterns;
@@ -606,42 +607,43 @@ int file_callback(const struct option *opt, const char *arg, int unset)
 	return 0;
 }
 
-int not_callback(const struct option *opt, const char *arg, int unset)
+static int not_callback(const struct option *opt, const char *arg, int unset)
 {
 	struct grep_opt *grep_opt = opt->value;
 	append_grep_pattern(grep_opt, "--not", "command line", 0, GREP_NOT);
 	return 0;
 }
 
-int and_callback(const struct option *opt, const char *arg, int unset)
+static int and_callback(const struct option *opt, const char *arg, int unset)
 {
 	struct grep_opt *grep_opt = opt->value;
 	append_grep_pattern(grep_opt, "--and", "command line", 0, GREP_AND);
 	return 0;
 }
 
-int open_callback(const struct option *opt, const char *arg, int unset)
+static int open_callback(const struct option *opt, const char *arg, int unset)
 {
 	struct grep_opt *grep_opt = opt->value;
 	append_grep_pattern(grep_opt, "(", "command line", 0, GREP_OPEN_PAREN);
 	return 0;
 }
 
-int close_callback(const struct option *opt, const char *arg, int unset)
+static int close_callback(const struct option *opt, const char *arg, int unset)
 {
 	struct grep_opt *grep_opt = opt->value;
 	append_grep_pattern(grep_opt, ")", "command line", 0, GREP_CLOSE_PAREN);
 	return 0;
 }
 
-int pattern_callback(const struct option *opt, const char *arg, int unset)
+static int pattern_callback(const struct option *opt, const char *arg,
+			    int unset)
 {
 	struct grep_opt *grep_opt = opt->value;
 	append_grep_pattern(grep_opt, arg, "-e option", 0, GREP_PATTERN);
 	return 0;
 }
 
-int help_callback(const struct option *opt, const char *arg, int unset)
+static int help_callback(const struct option *opt, const char *arg, int unset)
 {
 	return -1;
 }
-- 
1.6.3.1