[PATCH/RFC 0/4] grep: support to match by line number

[PATCH/RFC 0/4] grep: support to match by line number

[Bert Wesarg]

This series will teach git grep to match at specified line numbers. This is
particular usefull, if you want to see the lines which emits warnings or errors
from a build run (where you only get the line number) and present it nicely to
the user with function and context lines.

The implementation is split-up into preperation patches which are only noise:

Bert Wesarg (4):
  grep: prepare for re-using the space of the regexp member in struct
    grep_pat

    This one moves the regexp member into an union, so that we can later re-use
    the space for the line number information.

  grep: pass current line number down to match_one_pattern

    To actually match at the line number, the function match_one_pattern needs
    to know the current line number, do it with this patch.

  grep: introduce pattern which matches at line number

    This implements the line number matching in the low level grep machinery.

  grep: provide option to match line number

    And lastly, this exposes the new feature by a new -@ option to git grep.

Patch 5 will than support comma separated line ranges as argument to -@, but
this work has not yet started.

 Documentation/git-grep.txt |    6 +++-
 builtin/grep.c             |   10 +++++
 grep.c                     |   87 ++++++++++++++++++++++++++++++--------------
 grep.h                     |   11 ++++--
 4 files changed, 83 insertions(+), 31 deletions(-)

-- 
1.7.5.349.gfeb1a

[PATCH/RFC 1/4] grep: prepare for re-using the space of the regexp member in struct grep_pat

[Bert Wesarg]

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 grep.c |   12 ++++++------
 grep.h |    4 +++-
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/grep.c b/grep.c
index 63c4280..b8eda9e 100644
--- a/grep.c
+++ b/grep.c
@@ -70,7 +70,7 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
 	if (p->fixed)
 		return;
 
-	err = regcomp(&p->regexp, p->pattern, opt->regflags);
+	err = regcomp(&p->u.regexp, p->pattern, opt->regflags);
 	if (err) {
 		char errbuf[1024];
 		char where[1024];
@@ -81,8 +81,8 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
 			sprintf(where, "%s, ", p->origin);
 		else
 			where[0] = 0;
-		regerror(err, &p->regexp, errbuf, 1024);
-		regfree(&p->regexp);
+		regerror(err, &p->u.regexp, errbuf, 1024);
+		regfree(&p->u.regexp);
 		die("%s'%s': %s", where, p->pattern, errbuf);
 	}
 }
@@ -320,7 +320,7 @@ void free_grep_patterns(struct grep_opt *opt)
 		case GREP_PATTERN: /* atom */
 		case GREP_PATTERN_HEAD:
 		case GREP_PATTERN_BODY:
-			regfree(&p->regexp);
+			regfree(&p->u.regexp);
 			break;
 		default:
 			break;
@@ -464,7 +464,7 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 	if (p->fixed)
 		hit = !fixmatch(p, bol, eol, pmatch);
 	else
-		hit = !regmatch(&p->regexp, bol, eol, pmatch, eflags);
+		hit = !regmatch(&p->u.regexp, bol, eol, pmatch, eflags);
 
 	if (hit && p->word_regexp) {
 		if ((pmatch[0].rm_so < 0) ||
@@ -794,7 +794,7 @@ static int look_ahead(struct grep_opt *opt,
 		if (p->fixed)
 			hit = !fixmatch(p, bol, bol + *left_p, &m);
 		else
-			hit = !regmatch(&p->regexp, bol, bol + *left_p, &m, 0);
+			hit = !regmatch(&p->u.regexp, bol, bol + *left_p, &m, 0);
 		if (!hit || m.rm_so < 0 || m.rm_eo < 0)
 			continue;
 		if (earliest < 0 || m.rm_so < earliest)
diff --git a/grep.h b/grep.h
index 06621fe..9912c11 100644
--- a/grep.h
+++ b/grep.h
@@ -32,7 +32,9 @@ struct grep_pat {
 	const char *pattern;
 	size_t patternlen;
 	enum grep_header_field field;
-	regex_t regexp;
+	union {
+		regex_t regexp;
+	} u;
 	unsigned fixed:1;
 	unsigned ignore_case:1;
 	unsigned word_regexp:1;
-- 
1.7.5.349.gfeb1a

[PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern

[Bert Wesarg]

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 grep.c |   42 +++++++++++++++++++++---------------------
 1 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/grep.c b/grep.c
index b8eda9e..f21b022 100644
--- a/grep.c
+++ b/grep.c
@@ -437,7 +437,7 @@ static struct {
 };
 
 static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
-			     enum grep_context ctx,
+			     unsigned lno, enum grep_context ctx,
 			     regmatch_t *pmatch, int eflags)
 {
 	int hit = 0;
@@ -516,7 +516,7 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 }
 
 static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,
-			   enum grep_context ctx, int collect_hits)
+			   unsigned lno, enum grep_context ctx, int collect_hits)
 {
 	int h = 0;
 	regmatch_t match;
@@ -528,25 +528,25 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,
 		h = 1;
 		break;
 	case GREP_NODE_ATOM:
-		h = match_one_pattern(x->u.atom, bol, eol, ctx, &match, 0);
+		h = match_one_pattern(x->u.atom, bol, eol, lno, ctx, &match, 0);
 		break;
 	case GREP_NODE_NOT:
-		h = !match_expr_eval(x->u.unary, bol, eol, ctx, 0);
+		h = !match_expr_eval(x->u.unary, bol, eol, lno, ctx, 0);
 		break;
 	case GREP_NODE_AND:
-		if (!match_expr_eval(x->u.binary.left, bol, eol, ctx, 0))
+		if (!match_expr_eval(x->u.binary.left, bol, eol, lno, ctx, 0))
 			return 0;
-		h = match_expr_eval(x->u.binary.right, bol, eol, ctx, 0);
+		h = match_expr_eval(x->u.binary.right, bol, eol, lno, ctx, 0);
 		break;
 	case GREP_NODE_OR:
 		if (!collect_hits)
 			return (match_expr_eval(x->u.binary.left,
-						bol, eol, ctx, 0) ||
+						bol, eol, lno, ctx, 0) ||
 				match_expr_eval(x->u.binary.right,
-						bol, eol, ctx, 0));
-		h = match_expr_eval(x->u.binary.left, bol, eol, ctx, 0);
+						bol, eol, lno, ctx, 0));
+		h = match_expr_eval(x->u.binary.left, bol, eol, lno, ctx, 0);
 		x->u.binary.left->hit |= h;
-		h |= match_expr_eval(x->u.binary.right, bol, eol, ctx, 1);
+		h |= match_expr_eval(x->u.binary.right, bol, eol, lno, ctx, 1);
 		break;
 	default:
 		die("Unexpected node type (internal error) %d", x->node);
@@ -557,36 +557,36 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,
 }
 
 static int match_expr(struct grep_opt *opt, char *bol, char *eol,
-		      enum grep_context ctx, int collect_hits)
+		      unsigned lno, enum grep_context ctx, int collect_hits)
 {
 	struct grep_expr *x = opt->pattern_expression;
-	return match_expr_eval(x, bol, eol, ctx, collect_hits);
+	return match_expr_eval(x, bol, eol, lno, ctx, collect_hits);
 }
 
 static int match_line(struct grep_opt *opt, char *bol, char *eol,
-		      enum grep_context ctx, int collect_hits)
+		      unsigned lno, enum grep_context ctx, int collect_hits)
 {
 	struct grep_pat *p;
 	regmatch_t match;
 
 	if (opt->extended)
-		return match_expr(opt, bol, eol, ctx, collect_hits);
+		return match_expr(opt, bol, eol, lno, ctx, collect_hits);
 
 	/* we do not call with collect_hits without being extended */
 	for (p = opt->pattern_list; p; p = p->next) {
-		if (match_one_pattern(p, bol, eol, ctx, &match, 0))
+		if (match_one_pattern(p, bol, eol, lno, ctx, &match, 0))
 			return 1;
 	}
 	return 0;
 }
 
 static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,
-			      enum grep_context ctx,
+			      unsigned lno, enum grep_context ctx,
 			      regmatch_t *pmatch, int eflags)
 {
 	regmatch_t match;
 
-	if (!match_one_pattern(p, bol, eol, ctx, &match, eflags))
+	if (!match_one_pattern(p, bol, eol, lno, ctx, &match, eflags))
 		return 0;
 	if (match.rm_so < 0 || match.rm_eo < 0)
 		return 0;
@@ -601,7 +601,7 @@ static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,
 	return 1;
 }
 
-static int next_match(struct grep_opt *opt, char *bol, char *eol,
+static int next_match(struct grep_opt *opt, char *bol, char *eol, unsigned lno,
 		      enum grep_context ctx, regmatch_t *pmatch, int eflags)
 {
 	struct grep_pat *p;
@@ -614,7 +614,7 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol,
 			case GREP_PATTERN: /* atom */
 			case GREP_PATTERN_HEAD:
 			case GREP_PATTERN_BODY:
-				hit |= match_next_pattern(p, bol, eol, ctx,
+				hit |= match_next_pattern(p, bol, eol, lno, ctx,
 							  pmatch, eflags);
 				break;
 			default:
@@ -667,7 +667,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
 		else if (sign == '=')
 			line_color = opt->color_function;
 		*eol = '\0';
-		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
+		while (next_match(opt, bol, eol, lno, ctx, &match, eflags)) {
 			if (match.rm_so == match.rm_eo)
 				break;
 
@@ -911,7 +911,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 		if ((ctx == GREP_CONTEXT_HEAD) && (eol == bol))
 			ctx = GREP_CONTEXT_BODY;
 
-		hit = match_line(opt, bol, eol, ctx, collect_hits);
+		hit = match_line(opt, bol, eol, lno, ctx, collect_hits);
 		*eol = ch;
 
 		if (collect_hits)
-- 
1.7.5.349.gfeb1a

[PATCH/RFC 3/4] grep: introduce pattern which matches at line number

[Bert Wesarg]

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 grep.c |   33 +++++++++++++++++++++++++++++++++
 grep.h |    7 +++++--
 2 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/grep.c b/grep.c
index f21b022..cbb3757 100644
--- a/grep.c
+++ b/grep.c
@@ -87,6 +87,15 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
 	}
 }
 
+static void parse_lno(struct grep_pat *p, struct grep_opt *opt)
+{
+	char *eon;
+
+	p->u.lno = strtoul(p->pattern, &eon, 10);
+	if (*eon || p->u.lno == 0)
+		die("'%s': Invalid number for line match", p->pattern);
+}
+
 static struct grep_expr *compile_pattern_or(struct grep_pat **);
 static struct grep_expr *compile_pattern_atom(struct grep_pat **list)
 {
@@ -100,6 +109,7 @@ static struct grep_expr *compile_pattern_atom(struct grep_pat **list)
 	case GREP_PATTERN: /* atom */
 	case GREP_PATTERN_HEAD:
 	case GREP_PATTERN_BODY:
+	case GREP_LNO:
 		x = xcalloc(1, sizeof (struct grep_expr));
 		x->node = GREP_NODE_ATOM;
 		x->u.atom = p;
@@ -264,6 +274,9 @@ void compile_grep_patterns(struct grep_opt *opt)
 		case GREP_PATTERN_BODY:
 			compile_regexp(p, opt);
 			break;
+		case GREP_LNO:
+			parse_lno(p, opt);
+			break;
 		default:
 			opt->extended = 1;
 			break;
@@ -297,6 +310,7 @@ static void free_pattern_expr(struct grep_expr *x)
 	switch (x->node) {
 	case GREP_NODE_TRUE:
 	case GREP_NODE_ATOM:
+	case GREP_NODE_LNO:
 		break;
 	case GREP_NODE_NOT:
 		free_pattern_expr(x->u.unary);
@@ -436,6 +450,21 @@ static struct {
 	{ "committer ", 10 },
 };
 
+static int match_lno(struct grep_pat *p, char *bol, char *eol, unsigned lno,
+		     regmatch_t *pmatch)
+{
+	if (p->u.lno == lno) {
+		/*
+		 * because coloring will stop at so == eo, match at the end
+		 * of line, so that the whole line can be colored
+		 */
+		pmatch->rm_so = eol - bol;
+		pmatch->rm_eo = eol - bol;
+		return 1;
+	}
+	return 0;
+}
+
 static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 			     unsigned lno, enum grep_context ctx,
 			     regmatch_t *pmatch, int eflags)
@@ -444,6 +473,9 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
 	int saved_ch = 0;
 	const char *start = bol;
 
+	if (p->token == GREP_LNO)
+		return match_lno(p, bol, eol, lno, pmatch);
+
 	if ((p->token != GREP_PATTERN) &&
 	    ((p->token == GREP_PATTERN_HEAD) != (ctx == GREP_CONTEXT_HEAD)))
 		return 0;
@@ -614,6 +646,7 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol, unsigned lno,
 			case GREP_PATTERN: /* atom */
 			case GREP_PATTERN_HEAD:
 			case GREP_PATTERN_BODY:
+			case GREP_LNO:
 				hit |= match_next_pattern(p, bol, eol, lno, ctx,
 							  pmatch, eflags);
 				break;
diff --git a/grep.h b/grep.h
index 9912c11..41821f3 100644
--- a/grep.h
+++ b/grep.h
@@ -10,7 +10,8 @@ enum grep_pat_token {
 	GREP_OPEN_PAREN,
 	GREP_CLOSE_PAREN,
 	GREP_NOT,
-	GREP_OR
+	GREP_OR,
+	GREP_LNO
 };
 
 enum grep_context {
@@ -34,6 +35,7 @@ struct grep_pat {
 	enum grep_header_field field;
 	union {
 		regex_t regexp;
+		unsigned lno;
 	} u;
 	unsigned fixed:1;
 	unsigned ignore_case:1;
@@ -45,7 +47,8 @@ enum grep_expr_node {
 	GREP_NODE_NOT,
 	GREP_NODE_AND,
 	GREP_NODE_TRUE,
-	GREP_NODE_OR
+	GREP_NODE_OR,
+	GREP_NODE_LNO
 };
 
 struct grep_expr {
-- 
1.7.5.349.gfeb1a

[PATCH/RFC 4/4] grep: provide option to match line number

[Bert Wesarg]

Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
---
 Documentation/git-grep.txt |    6 +++++-
 builtin/grep.c             |   10 ++++++++++
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index d7523b3..05deb58 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -21,7 +21,7 @@ SYNOPSIS
 	   [--color[=<when>] | --no-color]
 	   [-A <post-context>] [-B <pre-context>] [-C <context>]
 	   [-f <file>] [-e] <pattern>
-	   [--and|--or|--not|(|)|-e <pattern>...]
+	   [--and|--or|--not|(|)|-e <pattern>|-@ <line>...]
 	   [--cached | --no-index | <tree>...]
 	   [--] [<pathspec>...]
 
@@ -168,6 +168,10 @@ OPTIONS
 	scripts passing user input to grep.  Multiple patterns are
 	combined by 'or'.
 
+-@::
+	The next parameter is a line number. This special pattern matches
+	this line of the input file.
+
 --and::
 --or::
 --not::
diff --git a/builtin/grep.c b/builtin/grep.c
index 10a1f65..8b11c87 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -733,6 +733,14 @@ static int pattern_callback(const struct option *opt, const char *arg,
 	return 0;
 }
 
+static int lno_pattern_callback(const struct option *opt, const char *arg,
+				int unset)
+{
+	struct grep_opt *grep_opt = opt->value;
+	append_grep_pattern(grep_opt, arg, "-@ option", 0, GREP_LNO);
+	return 0;
+}
+
 static int help_callback(const struct option *opt, const char *arg, int unset)
 {
 	return -1;
@@ -816,6 +824,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			"read patterns from file", file_callback),
 		{ OPTION_CALLBACK, 'e', NULL, &opt, "pattern",
 			"match <pattern>", PARSE_OPT_NONEG, pattern_callback },
+		{ OPTION_CALLBACK, '@', NULL, &opt, "lno",
+			"match <line>", PARSE_OPT_NONEG, lno_pattern_callback },
 		{ OPTION_CALLBACK, 0, "and", &opt, NULL,
 		  "combine patterns specified with -e",
 		  PARSE_OPT_NOARG | PARSE_OPT_NONEG, and_callback },
-- 
1.7.5.349.gfeb1a

Re: [PATCH/RFC 0/4] grep: support to match by line number

[Sverre Rabbelier]

Heya,

On Mon, May 2, 2011 at 13:39, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> This series will teach git grep to match at specified line numbers. This is
> particular usefull, if you want to see the lines which emits warnings or errors
> from a build run (where you only get the line number) and present it nicely to
> the user with function and context lines.

Can you give a concrete example of how you'd use this? I'm not sure I
understand the described use case.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH/RFC 0/4] grep: support to match by line number

[Bert Wesarg]

On Mon, May 2, 2011 at 13:54, Sverre Rabbelier <srabbelier@gmail.com> wrote:
> Heya,
>
> On Mon, May 2, 2011 at 13:39, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
>> This series will teach git grep to match at specified line numbers. This is
>> particular usefull, if you want to see the lines which emits warnings or errors
>> from a build run (where you only get the line number) and present it nicely to
>> the user with function and context lines.
>
> Can you give a concrete example of how you'd use this? I'm not sure I
> understand the described use case.

I parse the output of compiling a file, there may be warnings or even
errors, I collect the given line numbers, and then call 'git grep -p
-C -n (-@ <lno>)+ -- <path>' to show me what these warnings are about.

For example clang 2.9 (while clang is a bad example, because it prints
the offending line already, but gcc does not generate any warnings on
master currently) gives the following warning (which is probably worth
a patch, btw):

    CC grep.o
grep.c:231:16: warning: comparison of unsigned enum expression < 0 is
always false [-Wtautological-compare]
                if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)
                    ~~~~~~~~ ^ ~
1 warning generated.

I can than call 'git grep -3 -n -h -p -@ 231 -- grep.c', and get:

218=static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
--
228-    for (p = opt->header_list; p; p = p->next) {
229-            if (p->token != GREP_PATTERN_HEAD)
230-                    die("bug: a non-header pattern in grep header list.");
231:            if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)
232-                    die("bug: unknown header field %d", p->field);
233-            compile_regexp(p, opt);
234-    }

Another use case would be to restrict the matching to a block of
lines, this would need the mentioned patch 5, which than will support
line ranges, so that you can say 'match foo, but only in the first 10
lines'.

Bert

>
> --
> Cheers,
>
> Sverre Rabbelier
>

Re: [PATCH/RFC 1/4] grep: prepare for re-using the space of the regexp member in struct grep_pat

[Thiago Farina]

On Mon, May 2, 2011 at 8:39 AM, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
>  grep.c |   12 ++++++------
>  grep.h |    4 +++-
>  2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/grep.c b/grep.c
> index 63c4280..b8eda9e 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -70,7 +70,7 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
>        if (p->fixed)
>                return;
>
> -       err = regcomp(&p->regexp, p->pattern, opt->regflags);
> +       err = regcomp(&p->u.regexp, p->pattern, opt->regflags);
>        if (err) {
>                char errbuf[1024];
>                char where[1024];
> @@ -81,8 +81,8 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
>                        sprintf(where, "%s, ", p->origin);
>                else
>                        where[0] = 0;
> -               regerror(err, &p->regexp, errbuf, 1024);
> -               regfree(&p->regexp);
> +               regerror(err, &p->u.regexp, errbuf, 1024);
> +               regfree(&p->u.regexp);
>                die("%s'%s': %s", where, p->pattern, errbuf);
>        }
>  }
> @@ -320,7 +320,7 @@ void free_grep_patterns(struct grep_opt *opt)
>                case GREP_PATTERN: /* atom */
>                case GREP_PATTERN_HEAD:
>                case GREP_PATTERN_BODY:
> -                       regfree(&p->regexp);
> +                       regfree(&p->u.regexp);
>                        break;
>                default:
>                        break;
> @@ -464,7 +464,7 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
>        if (p->fixed)
>                hit = !fixmatch(p, bol, eol, pmatch);
>        else
> -               hit = !regmatch(&p->regexp, bol, eol, pmatch, eflags);
> +               hit = !regmatch(&p->u.regexp, bol, eol, pmatch, eflags);
>
>        if (hit && p->word_regexp) {
>                if ((pmatch[0].rm_so < 0) ||
> @@ -794,7 +794,7 @@ static int look_ahead(struct grep_opt *opt,
>                if (p->fixed)
>                        hit = !fixmatch(p, bol, bol + *left_p, &m);
>                else
> -                       hit = !regmatch(&p->regexp, bol, bol + *left_p, &m, 0);
> +                       hit = !regmatch(&p->u.regexp, bol, bol + *left_p, &m, 0);
>                if (!hit || m.rm_so < 0 || m.rm_eo < 0)
>                        continue;
>                if (earliest < 0 || m.rm_so < earliest)
> diff --git a/grep.h b/grep.h
> index 06621fe..9912c11 100644
> --- a/grep.h
> +++ b/grep.h
> @@ -32,7 +32,9 @@ struct grep_pat {
>        const char *pattern;
>        size_t patternlen;
>        enum grep_header_field field;
> -       regex_t regexp;
> +       union {
> +               regex_t regexp;
> +       } u;

Instead of u, would be worth to rename it to something more descriptive?

>        unsigned fixed:1;
>        unsigned ignore_case:1;
>        unsigned word_regexp:1;
> --
> 1.7.5.349.gfeb1a
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern

[Thiago Farina]

On Mon, May 2, 2011 at 8:39 AM, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
>  grep.c |   42 +++++++++++++++++++++---------------------
>  1 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/grep.c b/grep.c
> index b8eda9e..f21b022 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -437,7 +437,7 @@ static struct {
>  };
>
>  static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
> -                            enum grep_context ctx,
> +                            unsigned lno, enum grep_context ctx,

I'd rename lno to line_nr, so it's more clearer. Also, I'd add the new
paramenter at the end, not in some random position (or was there some
particular reason to put here but not in the end?).

>                             regmatch_t *pmatch, int eflags)
>  {
>        int hit = 0;
> @@ -516,7 +516,7 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
>  }
>
>  static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,
> -                          enum grep_context ctx, int collect_hits)
> +                          unsigned lno, enum grep_context ctx, int collect_hits)
>  {
>        int h = 0;
>        regmatch_t match;
> @@ -528,25 +528,25 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,
>                h = 1;
>                break;
>        case GREP_NODE_ATOM:
> -               h = match_one_pattern(x->u.atom, bol, eol, ctx, &match, 0);
> +               h = match_one_pattern(x->u.atom, bol, eol, lno, ctx, &match, 0);
>                break;
>        case GREP_NODE_NOT:
> -               h = !match_expr_eval(x->u.unary, bol, eol, ctx, 0);
> +               h = !match_expr_eval(x->u.unary, bol, eol, lno, ctx, 0);
>                break;
>        case GREP_NODE_AND:
> -               if (!match_expr_eval(x->u.binary.left, bol, eol, ctx, 0))
> +               if (!match_expr_eval(x->u.binary.left, bol, eol, lno, ctx, 0))
>                        return 0;
> -               h = match_expr_eval(x->u.binary.right, bol, eol, ctx, 0);
> +               h = match_expr_eval(x->u.binary.right, bol, eol, lno, ctx, 0);
>                break;
>        case GREP_NODE_OR:
>                if (!collect_hits)
>                        return (match_expr_eval(x->u.binary.left,
> -                                               bol, eol, ctx, 0) ||
> +                                               bol, eol, lno, ctx, 0) ||
>                                match_expr_eval(x->u.binary.right,
> -                                               bol, eol, ctx, 0));
> -               h = match_expr_eval(x->u.binary.left, bol, eol, ctx, 0);
> +                                               bol, eol, lno, ctx, 0));
> +               h = match_expr_eval(x->u.binary.left, bol, eol, lno, ctx, 0);
>                x->u.binary.left->hit |= h;
> -               h |= match_expr_eval(x->u.binary.right, bol, eol, ctx, 1);
> +               h |= match_expr_eval(x->u.binary.right, bol, eol, lno, ctx, 1);
>                break;
>        default:
>                die("Unexpected node type (internal error) %d", x->node);
> @@ -557,36 +557,36 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,
>  }
>
>  static int match_expr(struct grep_opt *opt, char *bol, char *eol,
> -                     enum grep_context ctx, int collect_hits)
> +                     unsigned lno, enum grep_context ctx, int collect_hits)
>  {
>        struct grep_expr *x = opt->pattern_expression;
> -       return match_expr_eval(x, bol, eol, ctx, collect_hits);
> +       return match_expr_eval(x, bol, eol, lno, ctx, collect_hits);
>  }
>
>  static int match_line(struct grep_opt *opt, char *bol, char *eol,
> -                     enum grep_context ctx, int collect_hits)
> +                     unsigned lno, enum grep_context ctx, int collect_hits)
>  {
>        struct grep_pat *p;
>        regmatch_t match;
>
>        if (opt->extended)
> -               return match_expr(opt, bol, eol, ctx, collect_hits);
> +               return match_expr(opt, bol, eol, lno, ctx, collect_hits);
>
>        /* we do not call with collect_hits without being extended */
>        for (p = opt->pattern_list; p; p = p->next) {
> -               if (match_one_pattern(p, bol, eol, ctx, &match, 0))
> +               if (match_one_pattern(p, bol, eol, lno, ctx, &match, 0))
>                        return 1;
>        }
>        return 0;
>  }
>
>  static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,
> -                             enum grep_context ctx,
> +                             unsigned lno, enum grep_context ctx,
>                              regmatch_t *pmatch, int eflags)
>  {
>        regmatch_t match;
>
> -       if (!match_one_pattern(p, bol, eol, ctx, &match, eflags))
> +       if (!match_one_pattern(p, bol, eol, lno, ctx, &match, eflags))
>                return 0;
>        if (match.rm_so < 0 || match.rm_eo < 0)
>                return 0;
> @@ -601,7 +601,7 @@ static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,
>        return 1;
>  }
>
> -static int next_match(struct grep_opt *opt, char *bol, char *eol,
> +static int next_match(struct grep_opt *opt, char *bol, char *eol, unsigned lno,
>                      enum grep_context ctx, regmatch_t *pmatch, int eflags)
>  {
>        struct grep_pat *p;
> @@ -614,7 +614,7 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol,
>                        case GREP_PATTERN: /* atom */
>                        case GREP_PATTERN_HEAD:
>                        case GREP_PATTERN_BODY:
> -                               hit |= match_next_pattern(p, bol, eol, ctx,
> +                               hit |= match_next_pattern(p, bol, eol, lno, ctx,
>                                                          pmatch, eflags);
>                                break;
>                        default:
> @@ -667,7 +667,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,
>                else if (sign == '=')
>                        line_color = opt->color_function;
>                *eol = '\0';
> -               while (next_match(opt, bol, eol, ctx, &match, eflags)) {
> +               while (next_match(opt, bol, eol, lno, ctx, &match, eflags)) {
>                        if (match.rm_so == match.rm_eo)
>                                break;
>
> @@ -911,7 +911,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
>                if ((ctx == GREP_CONTEXT_HEAD) && (eol == bol))
>                        ctx = GREP_CONTEXT_BODY;
>
> -               hit = match_line(opt, bol, eol, ctx, collect_hits);
> +               hit = match_line(opt, bol, eol, lno, ctx, collect_hits);
>                *eol = ch;
>
>                if (collect_hits)
> --
> 1.7.5.349.gfeb1a
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH/RFC 3/4] grep: introduce pattern which matches at line number

[Thiago Farina]

On Mon, May 2, 2011 at 8:39 AM, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
> ---
>  grep.c |   33 +++++++++++++++++++++++++++++++++
>  grep.h |    7 +++++--
>  2 files changed, 38 insertions(+), 2 deletions(-)
>
> diff --git a/grep.c b/grep.c
> index f21b022..cbb3757 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -87,6 +87,15 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
>        }
>  }
>
> +static void parse_lno(struct grep_pat *p, struct grep_opt *opt)
Maybe parse_line_number?

> +{
> +       char *eon;
> +
> +       p->u.lno = strtoul(p->pattern, &eon, 10);
> +       if (*eon || p->u.lno == 0)
> +               die("'%s': Invalid number for line match", p->pattern);
> +}
> +
>  static struct grep_expr *compile_pattern_or(struct grep_pat **);
>  static struct grep_expr *compile_pattern_atom(struct grep_pat **list)
>  {
> @@ -100,6 +109,7 @@ static struct grep_expr *compile_pattern_atom(struct grep_pat **list)
>        case GREP_PATTERN: /* atom */
>        case GREP_PATTERN_HEAD:
>        case GREP_PATTERN_BODY:
> +       case GREP_LNO:
>                x = xcalloc(1, sizeof (struct grep_expr));
>                x->node = GREP_NODE_ATOM;
>                x->u.atom = p;
> @@ -264,6 +274,9 @@ void compile_grep_patterns(struct grep_opt *opt)
>                case GREP_PATTERN_BODY:
>                        compile_regexp(p, opt);
>                        break;
> +               case GREP_LNO:
> +                       parse_lno(p, opt);
> +                       break;
>                default:
>                        opt->extended = 1;
>                        break;
> @@ -297,6 +310,7 @@ static void free_pattern_expr(struct grep_expr *x)
>        switch (x->node) {
>        case GREP_NODE_TRUE:
>        case GREP_NODE_ATOM:
> +       case GREP_NODE_LNO:
>                break;
>        case GREP_NODE_NOT:
>                free_pattern_expr(x->u.unary);
> @@ -436,6 +450,21 @@ static struct {
>        { "committer ", 10 },
>  };
>
> +static int match_lno(struct grep_pat *p, char *bol, char *eol, unsigned lno,
> +                    regmatch_t *pmatch)
> +{
> +       if (p->u.lno == lno) {
> +               /*
> +                * because coloring will stop at so == eo, match at the end
> +                * of line, so that the whole line can be colored
> +                */
> +               pmatch->rm_so = eol - bol;
> +               pmatch->rm_eo = eol - bol;
> +               return 1;
> +       }
> +       return 0;
> +}
> +
>  static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
>                             unsigned lno, enum grep_context ctx,
>                             regmatch_t *pmatch, int eflags)
> @@ -444,6 +473,9 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
>        int saved_ch = 0;
>        const char *start = bol;
>
> +       if (p->token == GREP_LNO)
> +               return match_lno(p, bol, eol, lno, pmatch);
> +
>        if ((p->token != GREP_PATTERN) &&
>            ((p->token == GREP_PATTERN_HEAD) != (ctx == GREP_CONTEXT_HEAD)))
>                return 0;
> @@ -614,6 +646,7 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol, unsigned lno,
>                        case GREP_PATTERN: /* atom */
>                        case GREP_PATTERN_HEAD:
>                        case GREP_PATTERN_BODY:
> +                       case GREP_LNO:
>                                hit |= match_next_pattern(p, bol, eol, lno, ctx,
>                                                          pmatch, eflags);
>                                break;
> diff --git a/grep.h b/grep.h
> index 9912c11..41821f3 100644
> --- a/grep.h
> +++ b/grep.h
> @@ -10,7 +10,8 @@ enum grep_pat_token {
>        GREP_OPEN_PAREN,
>        GREP_CLOSE_PAREN,
>        GREP_NOT,
> -       GREP_OR
> +       GREP_OR,
> +       GREP_LNO

Maybe GREP_LINE_NR?


>  };
>
>  enum grep_context {
> @@ -34,6 +35,7 @@ struct grep_pat {
>        enum grep_header_field field;
>        union {
>                regex_t regexp;
> +               unsigned lno;
>        } u;
>        unsigned fixed:1;
>        unsigned ignore_case:1;
> @@ -45,7 +47,8 @@ enum grep_expr_node {
>        GREP_NODE_NOT,
>        GREP_NODE_AND,
>        GREP_NODE_TRUE,
> -       GREP_NODE_OR
> +       GREP_NODE_OR,
> +       GREP_NODE_LNO
>  };
>
>  struct grep_expr {
> --
> 1.7.5.349.gfeb1a
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH/RFC 1/4] grep: prepare for re-using the space of the regexp member in struct grep_pat

[Bert Wesarg]

On Mon, May 2, 2011 at 15:27, Thiago Farina <tfransosi@gmail.com> wrote:
> On Mon, May 2, 2011 at 8:39 AM, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
>> diff --git a/grep.h b/grep.h
>> index 06621fe..9912c11 100644
>> --- a/grep.h
>> +++ b/grep.h
>> @@ -32,7 +32,9 @@ struct grep_pat {
>>        const char *pattern;
>>        size_t patternlen;
>>        enum grep_header_field field;
>> -       regex_t regexp;
>> +       union {
>> +               regex_t regexp;
>> +       } u;
>
> Instead of u, would be worth to rename it to something more descriptive?

I followed struct grep_expr, where u is also used. But I'm open for a
better name, if you have one.

Bert

Re: [PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern

[Bert Wesarg]

On Mon, May 2, 2011 at 15:30, Thiago Farina <tfransosi@gmail.com> wrote:
> On Mon, May 2, 2011 at 8:39 AM, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
>> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>> ---
>>  grep.c |   42 +++++++++++++++++++++---------------------
>>  1 files changed, 21 insertions(+), 21 deletions(-)
>>
>> diff --git a/grep.c b/grep.c
>> index b8eda9e..f21b022 100644
>> --- a/grep.c
>> +++ b/grep.c
>> @@ -437,7 +437,7 @@ static struct {
>>  };
>>
>>  static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,
>> -                            enum grep_context ctx,
>> +                            unsigned lno, enum grep_context ctx,
>
> I'd rename lno to line_nr, so it's more clearer. Also, I'd add the new
> paramenter at the end, not in some random position (or was there some
> particular reason to put here but not in the end?).

lno is already in use in this file, so I followed here the precept.
The position was chosen, because bol/eol is the line content to match,
so the line number should probably be near the content parameter.

Bert

>
>>                             regmatch_t *pmatch, int eflags)
>>  {
>>        int hit = 0;

Re: [PATCH/RFC 3/4] grep: introduce pattern which matches at line number

[Bert Wesarg]

On Mon, May 2, 2011 at 15:33, Thiago Farina <tfransosi@gmail.com> wrote:
> On Mon, May 2, 2011 at 8:39 AM, Bert Wesarg <bert.wesarg@googlemail.com> wrote:
>> Signed-off-by: Bert Wesarg <bert.wesarg@googlemail.com>
>> ---
>>  grep.c |   33 +++++++++++++++++++++++++++++++++
>>  grep.h |    7 +++++--
>>  2 files changed, 38 insertions(+), 2 deletions(-)
>>
>> diff --git a/grep.c b/grep.c
>> index f21b022..cbb3757 100644
>> --- a/grep.c
>> +++ b/grep.c
>> @@ -87,6 +87,15 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)
>>        }
>>  }
>>
>> +static void parse_lno(struct grep_pat *p, struct grep_opt *opt)
> Maybe parse_line_number?
>
>> +{
>> +       char *eon;
>> +
>> +       p->u.lno = strtoul(p->pattern, &eon, 10);
>> +       if (*eon || p->u.lno == 0)
>> +               die("'%s': Invalid number for line match", p->pattern);
>> +}
>> +
>>  static struct grep_expr *compile_pattern_or(struct grep_pat **);
>>  static struct grep_expr *compile_pattern_atom(struct grep_pat **list)
>>  {
>> diff --git a/grep.h b/grep.h
>> index 9912c11..41821f3 100644
>> --- a/grep.h
>> +++ b/grep.h
>> @@ -10,7 +10,8 @@ enum grep_pat_token {
>>        GREP_OPEN_PAREN,
>>        GREP_CLOSE_PAREN,
>>        GREP_NOT,
>> -       GREP_OR
>> +       GREP_OR,
>> +       GREP_LNO
>
> Maybe GREP_LINE_NR?
>
>
>>  };
>>
>>  enum grep_context {

Yeah, I must admit that these symbols should probably get a more
descriptive name.

Bert

Re: [PATCH/RFC 0/4] grep: support to match by line number

[Junio C Hamano]

I am personally not thrilled by what this series attempts to do, but first
a few questions:

 - Are there existing non-git "grep" implementations that do this?

 - If yes:
   - what command option letter do they use to specify line number?
   - do they not support a range notation (e.g. -@ 25-30,32-40)?
   - what do they do when given more than one file?

 - If no:
   - why not?  Is it a sign that this is ill-thought out misfeature?
   - perhaps people use something like "sed -n -e 25,30p file" and be
     happy?

Re: [PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern

[Junio C Hamano]

Bert Wesarg <bert.wesarg@googlemail.com> writes:

> lno is already in use in this file, so I followed here the precept.
> The position was chosen, because bol/eol is the line content to match,
> so the line number should probably be near the content parameter.

The choice of "lno" is fine (and probably better than line_nr) as variable
and enum names in the context of this file.

If we were to do this change, that is.

Re: [PATCH/RFC 0/4] grep: support to match by line number

[Junio C Hamano]

Bert Wesarg <bert.wesarg@googlemail.com> writes:

>> Can you give a concrete example of how you'd use this? I'm not sure I
>> understand the described use case.
>
> I parse the output of compiling a file, there may be warnings or even
> errors, I collect the given line numbers, and then call 'git grep -p
> -C -n (-@ <lno>)+ -- <path>' to show me what these warnings are about.

What would you do next after doing that?  Open the file in your editor and
jump to the line found by the grep?  Oh, wait, that's the same line as
your compiler already found for you.

And when you open the file in your editor, doesn't it show offending line
in the middle of the screen?  You can even fix it up right there.  Isn't
your editor wonderful?

The extra "grep -@" step in-between looks like a totally made-up use case
that is not interesting nor convincing.

Re: [PATCH/RFC 0/4] grep: support to match by line number

[Jakub Narebski]

Junio C Hamano <gitster@pobox.com> writes:

> I am personally not thrilled by what this series attempts to do, but first
> a few questions:
> 
>  - Are there existing non-git "grep" implementations that do this?
> 
>  - If yes:
>    - what command option letter do they use to specify line number?
>    - do they not support a range notation (e.g. -@ 25-30,32-40)?
>    - what do they do when given more than one file?

I'm not sure if it is exactly the same as '-@' in Bert proposal, but
"ack", grep-like text finder in Perl (http://betterthangrep.com or
http://p3rl.org/ack), includes the following command line option:

  --line=NUM

    Only print line NUM of each file. Multiple lines can be given with
    multiple --line options or as a comma separated list
    (--line=3,5,7). --line=4-7 also works. The lines are always output in
    ascending order, no matter the order given on the command line.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [PATCH/RFC 0/4] grep: support to match by line number

[Bert Wesarg]

On Mon, May 2, 2011 at 18:38, Junio C Hamano <gitster@pobox.com> wrote:
> I am personally not thrilled by what this series attempts to do, but first
> a few questions:
>
>  - Are there existing non-git "grep" implementations that do this?

I didn't know of any, until Jakub mentioned the ack tool. And I didn't
look for one. I will answer you 'yes'-questions only in context of my
proposal.

>
>  - If yes:
>   - what command option letter do they use to specify line number?

I have no strong argument for -@, it was short and -l and -L are
taken, but I wont use this that often on the command line, so a long
option like '--line=' is ok for me too.

>   - do they not support a range notation (e.g. -@ 25-30,32-40)?

That was already on my todo list and also mentioned in the last paragraph.

>   - what do they do when given more than one file?

Like the content patterns, they try to match.

>
>  - If no:
>   - why not?  Is it a sign that this is ill-thought out misfeature?

Printing only some lines of a file isn't that hard, and there are
obviously some standard tools which do this fine, but combining this
with the -C and --show-function feature isn't easy. So extending grep
with this feature sounds like the best bet.

>   - perhaps people use something like "sed -n -e 25,30p file" and be
>     happy?

How would you combine this with git grep HEAD or with multiple files?

Bert

Re: [PATCH/RFC 0/4] grep: support to match by line number

[Junio C Hamano]

Bert Wesarg <bert.wesarg@googlemail.com> writes:

>>  - Are there existing non-git "grep" implementations that do this?
>
> I didn't know of any, until Jakub mentioned the ack tool. And I didn't
> look for one. I will answer you 'yes'-questions only in context of my
> proposal.

Thanks, but these questions are simple sanity checks done by contrasting
your design with _existing practices_, if any.  It is not useful to answer
what you did here, only to contrast your design with itself.

Also we can read them from your implementation ;-).

>>   - perhaps people use something like "sed -n -e 25,30p file" and be
>>     happy?
>
> How would you combine this with git grep HEAD or with multiple files?

Now how did you get the compiler error messages from your example from the
files in HEAD commit without checking them out?

Besides, that question is still part of figuring out prior art done in the
git-unaware grep, so it is irrelevant that sed or other people's grep
cannot peek into HEAD.

Re: [PATCH/RFC 0/4] grep: support to match by line number

[Bert Wesarg]

On Mon, May 2, 2011 at 18:46, Junio C Hamano <gitster@pobox.com> wrote:
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>>> Can you give a concrete example of how you'd use this? I'm not sure I
>>> understand the described use case.
>>
>> I parse the output of compiling a file, there may be warnings or even
>> errors, I collect the given line numbers, and then call 'git grep -p
>> -C -n (-@ <lno>)+ -- <path>' to show me what these warnings are about.
>
> What would you do next after doing that?  Open the file in your editor and
> jump to the line found by the grep?  Oh, wait, that's the same line as
> your compiler already found for you.

A build run will probably produce many warnings or errors (think of
-j) for many files, so you need to handle more than one file. After
the build run, you get a list of files which have warnings/errors,
selecting one of the files will give you the original messages
including the grep -@ output, from there you can select which message
you want to consider opening in the editor, so grep -@ helps you in
your decision what to open in your editor. If this would all be in a
terminal your original output would be far away anyhow.

I haven't implemented this 'workflow' for the terminal, but for a GUI,
so I can jump to original output easily and open the file in the
editor easily too. Prior to this I did this all on the command line,
but with recursive make this become too cumbersome.

>
> And when you open the file in your editor, doesn't it show offending line
> in the middle of the screen?  You can even fix it up right there.  Isn't
> your editor wonderful?
>
> The extra "grep -@" step in-between looks like a totally made-up use case
> that is not interesting nor convincing.
>
>

Visually parsing many warnings/errors of one file by successively
jumping to the offending line in the editor doesn't sound very
productive to me, but having all offending lines with context and
--show-functions in one output does sound.

Bert

Re: [PATCH/RFC 0/4] grep: support to match by line number

[Junio C Hamano]

Bert Wesarg <bert.wesarg@googlemail.com> writes:

> A build run will probably produce many warnings or errors (think of
> -j) for many files, so you need to handle more than one file. After
> the build run, you get a list of files which have warnings/errors,
> selecting one of the files will give you the original messages
> including the grep -@ output, from there you can select which message
> you want to consider opening in the editor, so grep -@ helps you in
> your decision what to open in your editor.

Now I got you talking ;-).

Wouldn't that workflow suggest that each invocation of -@ must be strongly
paired with each path?  After all, your hello.c having an error at line 100
does not have much to do with which lines in your goodbye.c are buggy.

Which has larger implications by opening a big can of worms.  Just a few
examples:

 - Should "git grep -@1 -@3 foo bar" show line 1 of foo and line 3 of bar,
   or should both lines 1 and 3 from both files be shown?  Why would
   anybody want to see line 3 of foo or line 1 of bar in such a case?

 - Later part of command line arguments on standard git command set are
   pathspecs, not necessarily individual paths.  You could for example say
   "git grep -@1 -@3 directory/". What should happen?  Should the first
   line of the first file in directory and the third line of the second
   file in directory be shown?  What would happen to the remaining files
   in the directory/?  You could replace "directory/" with "'*.c'".

   Perhaps use of -@ need to tighten the command line parsing to make sure
   that the user specified exact number of concrete filenames, not globs
   nor leading path prefixes, and otherwise error out?

I like "this range from this file, that range from that file" as a
concept.  It has far wider application than just in the context of "grep",
and I want to see us do it right from the beginning if we _were_ to design
it.  That is why I am poking this discussion to make sure I can solicit
deep enough thinking from people in the design phase.

Re: [PATCH/RFC 0/4] grep: support to match by line number

[Bert Wesarg]

On Mon, May 2, 2011 at 21:08, Junio C Hamano <gitster@pobox.com> wrote:
> Bert Wesarg <bert.wesarg@googlemail.com> writes:
>
>>>  - Are there existing non-git "grep" implementations that do this?
>>
>> I didn't know of any, until Jakub mentioned the ack tool. And I didn't
>> look for one. I will answer you 'yes'-questions only in context of my
>> proposal.
>
> Thanks, but these questions are simple sanity checks done by contrasting
> your design with _existing practices_, if any.  It is not useful to answer
> what you did here, only to contrast your design with itself.

I should probably have written 'I *can only* answer...', but I
appreciate your questions anyhow, even if I can't answer them all.

>
> Also we can read them from your implementation ;-).
>
>>>   - perhaps people use something like "sed -n -e 25,30p file" and be
>>>     happy?
>>
>> How would you combine this with git grep HEAD or with multiple files?
>
> Now how did you get the compiler error messages from your example from the
> files in HEAD commit without checking them out?

Haven't you heard of the git-gcc extension yet. Just kidding and point
taken. But I think/hope there are some more use cases than my own for
this feature, else I wouldn't have send the patches out.

Bert

>
> Besides, that question is still part of figuring out prior art done in the
> git-unaware grep, so it is irrelevant that sed or other people's grep
> cannot peek into HEAD.
>