{"thread":{"id":"27236","subject":"[PATCH/RFC 0/4] grep: support to match by line number","startedAt":"2011-05-02T11:39:09Z","lastAt":"2011-05-02T19:33:48Z","messageCount":22,"participants":["Bert Wesarg","Sverre Rabbelier","Thiago Farina","Junio C Hamano","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"166847","messageId":"cover.1304318972.git.bert.wesarg@googlemail.com","threadId":"27236","inReplyTo":null,"subject":"[PATCH/RFC 0/4] grep: support to match by line number","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2011-05-02T11:39:09Z","receivedAt":"2011-05-02T11:39:09Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"This series will teach git grep to match at specified line numbers. This is\nparticular usefull, if you want to see the lines which emits warnings or errors\nfrom a build run (where you only get the line number) and present it nicely to\nthe user with function and context lines.\n\nThe implementation is split-up into preperation patches which are only noise:\n\nBert Wesarg (4):\n grep: prepare for re-using the space of the regexp member in struct\n grep_pat\n\n This one moves the regexp member into an union, so that we can later re-use\n the space for the line number information.\n\n grep: pass current line number down to match_one_pattern\n\n To actually match at the line number, the function match_one_pattern needs\n to know the current line number, do it with this patch.\n\n grep: introduce pattern which matches at line number\n\n This implements the line number matching in the low level grep machinery.\n\n grep: provide option to match line number\n\n And lastly, this exposes the new feature by a new -@ option to git grep.\n\nPatch 5 will than support comma separated line ranges as argument to -@, but\nthis work has not yet started.\n\n Documentation/git-grep.txt | 6 +++-\n builtin/grep.c | 10 +++++\n grep.c | 87 ++++++++++++++++++++++++++++++--------------\n grep.h | 11 ++++--\n 4 files changed, 83 insertions(+), 31 deletions(-)\n\n-- \n1.7.5.349.gfeb1a\n"},{"id":"166851","messageId":"f768ea6e107cdd229a18df0bac3bf583eb1f9fc5.1304321122.git.bert.wesarg@googlemail.com","threadId":"27236","inReplyTo":"cover.1304318972.git.bert.wesarg@googlemail.com","subject":"[PATCH/RFC 1/4] grep: prepare for re-using the space of the regexp member in struct grep_pat","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2011-05-02T11:39:10Z","receivedAt":"2011-05-02T11:39:10Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Signed-off-by: Bert Wesarg \n---\n grep.c | 12 ++++++------\n grep.h | 4 +++-\n 2 files changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex 63c4280..b8eda9e 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -70,7 +70,7 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n \tif (p->fixed)\n \t\treturn;\n \n-\terr = regcomp(&p->regexp, p->pattern, opt->regflags);\n+\terr = regcomp(&p->u.regexp, p->pattern, opt->regflags);\n \tif (err) {\n \t\tchar errbuf[1024];\n \t\tchar where[1024];\n@@ -81,8 +81,8 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n \t\t\tsprintf(where, \"%s, \", p->origin);\n \t\telse\n \t\t\twhere[0] = 0;\n-\t\tregerror(err, &p->regexp, errbuf, 1024);\n-\t\tregfree(&p->regexp);\n+\t\tregerror(err, &p->u.regexp, errbuf, 1024);\n+\t\tregfree(&p->u.regexp);\n \t\tdie(\"%s'%s': %s\", where, p->pattern, errbuf);\n \t}\n }\n@@ -320,7 +320,7 @@ void free_grep_patterns(struct grep_opt *opt)\n \t\tcase GREP_PATTERN: /* atom */\n \t\tcase GREP_PATTERN_HEAD:\n \t\tcase GREP_PATTERN_BODY:\n-\t\t\tregfree(&p->regexp);\n+\t\t\tregfree(&p->u.regexp);\n \t\t\tbreak;\n \t\tdefault:\n \t\t\tbreak;\n@@ -464,7 +464,7 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n \tif (p->fixed)\n \t\thit = !fixmatch(p, bol, eol, pmatch);\n \telse\n-\t\thit = !regmatch(&p->regexp, bol, eol, pmatch, eflags);\n+\t\thit = !regmatch(&p->u.regexp, bol, eol, pmatch, eflags);\n \n \tif (hit && p->word_regexp) {\n \t\tif ((pmatch[0].rm_so < 0) ||\n@@ -794,7 +794,7 @@ static int look_ahead(struct grep_opt *opt,\n \t\tif (p->fixed)\n \t\t\thit = !fixmatch(p, bol, bol + *left_p, &m);\n \t\telse\n-\t\t\thit = !regmatch(&p->regexp, bol, bol + *left_p, &m, 0);\n+\t\t\thit = !regmatch(&p->u.regexp, bol, bol + *left_p, &m, 0);\n \t\tif (!hit || m.rm_so < 0 || m.rm_eo < 0)\n \t\t\tcontinue;\n \t\tif (earliest < 0 || m.rm_so < earliest)\ndiff --git a/grep.h b/grep.h\nindex 06621fe..9912c11 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -32,7 +32,9 @@ struct grep_pat {\n \tconst char *pattern;\n \tsize_t patternlen;\n \tenum grep_header_field field;\n-\tregex_t regexp;\n+\tunion {\n+\t\tregex_t regexp;\n+\t} u;\n \tunsigned fixed:1;\n \tunsigned ignore_case:1;\n \tunsigned word_regexp:1;\n-- \n1.7.5.349.gfeb1a\n"},{"id":"166848","messageId":"2681b60988c7c4d059f83df368395eca0520012c.1304321122.git.bert.wesarg@googlemail.com","threadId":"27236","inReplyTo":"f768ea6e107cdd229a18df0bac3bf583eb1f9fc5.1304321122.git.bert.wesarg@googlemail.com","subject":"[PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2011-05-02T11:39:11Z","receivedAt":"2011-05-02T11:39:11Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Signed-off-by: Bert Wesarg \n---\n grep.c | 42 +++++++++++++++++++++---------------------\n 1 files changed, 21 insertions(+), 21 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex b8eda9e..f21b022 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -437,7 +437,7 @@ static struct {\n };\n \n static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n-\t\t\t enum grep_context ctx,\n+\t\t\t unsigned lno, enum grep_context ctx,\n \t\t\t regmatch_t *pmatch, int eflags)\n {\n \tint hit = 0;\n@@ -516,7 +516,7 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n }\n \n static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n-\t\t\t enum grep_context ctx, int collect_hits)\n+\t\t\t unsigned lno, enum grep_context ctx, int collect_hits)\n {\n \tint h = 0;\n \tregmatch_t match;\n@@ -528,25 +528,25 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n \t\th = 1;\n \t\tbreak;\n \tcase GREP_NODE_ATOM:\n-\t\th = match_one_pattern(x->u.atom, bol, eol, ctx, &match, 0);\n+\t\th = match_one_pattern(x->u.atom, bol, eol, lno, ctx, &match, 0);\n \t\tbreak;\n \tcase GREP_NODE_NOT:\n-\t\th = !match_expr_eval(x->u.unary, bol, eol, ctx, 0);\n+\t\th = !match_expr_eval(x->u.unary, bol, eol, lno, ctx, 0);\n \t\tbreak;\n \tcase GREP_NODE_AND:\n-\t\tif (!match_expr_eval(x->u.binary.left, bol, eol, ctx, 0))\n+\t\tif (!match_expr_eval(x->u.binary.left, bol, eol, lno, ctx, 0))\n \t\t\treturn 0;\n-\t\th = match_expr_eval(x->u.binary.right, bol, eol, ctx, 0);\n+\t\th = match_expr_eval(x->u.binary.right, bol, eol, lno, ctx, 0);\n \t\tbreak;\n \tcase GREP_NODE_OR:\n \t\tif (!collect_hits)\n \t\t\treturn (match_expr_eval(x->u.binary.left,\n-\t\t\t\t\t\tbol, eol, ctx, 0) ||\n+\t\t\t\t\t\tbol, eol, lno, ctx, 0) ||\n \t\t\t\tmatch_expr_eval(x->u.binary.right,\n-\t\t\t\t\t\tbol, eol, ctx, 0));\n-\t\th = match_expr_eval(x->u.binary.left, bol, eol, ctx, 0);\n+\t\t\t\t\t\tbol, eol, lno, ctx, 0));\n+\t\th = match_expr_eval(x->u.binary.left, bol, eol, lno, ctx, 0);\n \t\tx->u.binary.left->hit |= h;\n-\t\th |= match_expr_eval(x->u.binary.right, bol, eol, ctx, 1);\n+\t\th |= match_expr_eval(x->u.binary.right, bol, eol, lno, ctx, 1);\n \t\tbreak;\n \tdefault:\n \t\tdie(\"Unexpected node type (internal error) %d\", x->node);\n@@ -557,36 +557,36 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n }\n \n static int match_expr(struct grep_opt *opt, char *bol, char *eol,\n-\t\t enum grep_context ctx, int collect_hits)\n+\t\t unsigned lno, enum grep_context ctx, int collect_hits)\n {\n \tstruct grep_expr *x = opt->pattern_expression;\n-\treturn match_expr_eval(x, bol, eol, ctx, collect_hits);\n+\treturn match_expr_eval(x, bol, eol, lno, ctx, collect_hits);\n }\n \n static int match_line(struct grep_opt *opt, char *bol, char *eol,\n-\t\t enum grep_context ctx, int collect_hits)\n+\t\t unsigned lno, enum grep_context ctx, int collect_hits)\n {\n \tstruct grep_pat *p;\n \tregmatch_t match;\n \n \tif (opt->extended)\n-\t\treturn match_expr(opt, bol, eol, ctx, collect_hits);\n+\t\treturn match_expr(opt, bol, eol, lno, ctx, collect_hits);\n \n \t/* we do not call with collect_hits without being extended */\n \tfor (p = opt->pattern_list; p; p = p->next) {\n-\t\tif (match_one_pattern(p, bol, eol, ctx, &match, 0))\n+\t\tif (match_one_pattern(p, bol, eol, lno, ctx, &match, 0))\n \t\t\treturn 1;\n \t}\n \treturn 0;\n }\n \n static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,\n-\t\t\t enum grep_context ctx,\n+\t\t\t unsigned lno, enum grep_context ctx,\n \t\t\t regmatch_t *pmatch, int eflags)\n {\n \tregmatch_t match;\n \n-\tif (!match_one_pattern(p, bol, eol, ctx, &match, eflags))\n+\tif (!match_one_pattern(p, bol, eol, lno, ctx, &match, eflags))\n \t\treturn 0;\n \tif (match.rm_so < 0 || match.rm_eo < 0)\n \t\treturn 0;\n@@ -601,7 +601,7 @@ static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,\n \treturn 1;\n }\n \n-static int next_match(struct grep_opt *opt, char *bol, char *eol,\n+static int next_match(struct grep_opt *opt, char *bol, char *eol, unsigned lno,\n \t\t enum grep_context ctx, regmatch_t *pmatch, int eflags)\n {\n \tstruct grep_pat *p;\n@@ -614,7 +614,7 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol,\n \t\t\tcase GREP_PATTERN: /* atom */\n \t\t\tcase GREP_PATTERN_HEAD:\n \t\t\tcase GREP_PATTERN_BODY:\n-\t\t\t\thit |= match_next_pattern(p, bol, eol, ctx,\n+\t\t\t\thit |= match_next_pattern(p, bol, eol, lno, ctx,\n \t\t\t\t\t\t\t pmatch, eflags);\n \t\t\t\tbreak;\n \t\t\tdefault:\n@@ -667,7 +667,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n \t\telse if (sign == '=')\n \t\t\tline_color = opt->color_function;\n \t\t*eol = '\\0';\n-\t\twhile (next_match(opt, bol, eol, ctx, &match, eflags)) {\n+\t\twhile (next_match(opt, bol, eol, lno, ctx, &match, eflags)) {\n \t\t\tif (match.rm_so == match.rm_eo)\n \t\t\t\tbreak;\n \n@@ -911,7 +911,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,\n \t\tif ((ctx == GREP_CONTEXT_HEAD) && (eol == bol))\n \t\t\tctx = GREP_CONTEXT_BODY;\n \n-\t\thit = match_line(opt, bol, eol, ctx, collect_hits);\n+\t\thit = match_line(opt, bol, eol, lno, ctx, collect_hits);\n \t\t*eol = ch;\n \n \t\tif (collect_hits)\n-- \n1.7.5.349.gfeb1a\n"},{"id":"166849","messageId":"f85d783d43bfe7f9e9f23a10e72e8a2a83033cac.1304321122.git.bert.wesarg@googlemail.com","threadId":"27236","inReplyTo":"2681b60988c7c4d059f83df368395eca0520012c.1304321122.git.bert.wesarg@googlemail.com","subject":"[PATCH/RFC 3/4] grep: introduce pattern which matches at line number","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2011-05-02T11:39:12Z","receivedAt":"2011-05-02T11:39:12Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Signed-off-by: Bert Wesarg \n---\n grep.c | 33 +++++++++++++++++++++++++++++++++\n grep.h | 7 +++++--\n 2 files changed, 38 insertions(+), 2 deletions(-)\n\ndiff --git a/grep.c b/grep.c\nindex f21b022..cbb3757 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -87,6 +87,15 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n \t}\n }\n \n+static void parse_lno(struct grep_pat *p, struct grep_opt *opt)\n+{\n+\tchar *eon;\n+\n+\tp->u.lno = strtoul(p->pattern, &eon, 10);\n+\tif (*eon || p->u.lno == 0)\n+\t\tdie(\"'%s': Invalid number for line match\", p->pattern);\n+}\n+\n static struct grep_expr *compile_pattern_or(struct grep_pat **);\n static struct grep_expr *compile_pattern_atom(struct grep_pat **list)\n {\n@@ -100,6 +109,7 @@ static struct grep_expr *compile_pattern_atom(struct grep_pat **list)\n \tcase GREP_PATTERN: /* atom */\n \tcase GREP_PATTERN_HEAD:\n \tcase GREP_PATTERN_BODY:\n+\tcase GREP_LNO:\n \t\tx = xcalloc(1, sizeof (struct grep_expr));\n \t\tx->node = GREP_NODE_ATOM;\n \t\tx->u.atom = p;\n@@ -264,6 +274,9 @@ void compile_grep_patterns(struct grep_opt *opt)\n \t\tcase GREP_PATTERN_BODY:\n \t\t\tcompile_regexp(p, opt);\n \t\t\tbreak;\n+\t\tcase GREP_LNO:\n+\t\t\tparse_lno(p, opt);\n+\t\t\tbreak;\n \t\tdefault:\n \t\t\topt->extended = 1;\n \t\t\tbreak;\n@@ -297,6 +310,7 @@ static void free_pattern_expr(struct grep_expr *x)\n \tswitch (x->node) {\n \tcase GREP_NODE_TRUE:\n \tcase GREP_NODE_ATOM:\n+\tcase GREP_NODE_LNO:\n \t\tbreak;\n \tcase GREP_NODE_NOT:\n \t\tfree_pattern_expr(x->u.unary);\n@@ -436,6 +450,21 @@ static struct {\n \t{ \"committer \", 10 },\n };\n \n+static int match_lno(struct grep_pat *p, char *bol, char *eol, unsigned lno,\n+\t\t regmatch_t *pmatch)\n+{\n+\tif (p->u.lno == lno) {\n+\t\t/*\n+\t\t * because coloring will stop at so == eo, match at the end\n+\t\t * of line, so that the whole line can be colored\n+\t\t */\n+\t\tpmatch->rm_so = eol - bol;\n+\t\tpmatch->rm_eo = eol - bol;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n \t\t\t unsigned lno, enum grep_context ctx,\n \t\t\t regmatch_t *pmatch, int eflags)\n@@ -444,6 +473,9 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n \tint saved_ch = 0;\n \tconst char *start = bol;\n \n+\tif (p->token == GREP_LNO)\n+\t\treturn match_lno(p, bol, eol, lno, pmatch);\n+\n \tif ((p->token != GREP_PATTERN) &&\n \t ((p->token == GREP_PATTERN_HEAD) != (ctx == GREP_CONTEXT_HEAD)))\n \t\treturn 0;\n@@ -614,6 +646,7 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol, unsigned lno,\n \t\t\tcase GREP_PATTERN: /* atom */\n \t\t\tcase GREP_PATTERN_HEAD:\n \t\t\tcase GREP_PATTERN_BODY:\n+\t\t\tcase GREP_LNO:\n \t\t\t\thit |= match_next_pattern(p, bol, eol, lno, ctx,\n \t\t\t\t\t\t\t pmatch, eflags);\n \t\t\t\tbreak;\ndiff --git a/grep.h b/grep.h\nindex 9912c11..41821f3 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -10,7 +10,8 @@ enum grep_pat_token {\n \tGREP_OPEN_PAREN,\n \tGREP_CLOSE_PAREN,\n \tGREP_NOT,\n-\tGREP_OR\n+\tGREP_OR,\n+\tGREP_LNO\n };\n \n enum grep_context {\n@@ -34,6 +35,7 @@ struct grep_pat {\n \tenum grep_header_field field;\n \tunion {\n \t\tregex_t regexp;\n+\t\tunsigned lno;\n \t} u;\n \tunsigned fixed:1;\n \tunsigned ignore_case:1;\n@@ -45,7 +47,8 @@ enum grep_expr_node {\n \tGREP_NODE_NOT,\n \tGREP_NODE_AND,\n \tGREP_NODE_TRUE,\n-\tGREP_NODE_OR\n+\tGREP_NODE_OR,\n+\tGREP_NODE_LNO\n };\n \n struct grep_expr {\n-- \n1.7.5.349.gfeb1a\n"},{"id":"166850","messageId":"1028f8b6b8da69b8cb80e39a543b0775bc4d935a.1304321122.git.bert.wesarg@googlemail.com","threadId":"27236","inReplyTo":"f85d783d43bfe7f9e9f23a10e72e8a2a83033cac.1304321122.git.bert.wesarg@googlemail.com","subject":"[PATCH/RFC 4/4] grep: provide option to match line number","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2011-05-02T11:39:13Z","receivedAt":"2011-05-02T11:39:13Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"Signed-off-by: Bert Wesarg \n---\n Documentation/git-grep.txt | 6 +++++-\n builtin/grep.c | 10 ++++++++++\n 2 files changed, 15 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex d7523b3..05deb58 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -21,7 +21,7 @@ SYNOPSIS\n \t [--color[=] | --no-color]\n \t [-A ] [-B ] [-C ]\n \t [-f ] [-e] \n-\t [--and|--or|--not|(|)|-e ...]\n+\t [--and|--or|--not|(|)|-e |-@ ...]\n \t [--cached | --no-index | ...]\n \t [--] [...]\n \n@@ -168,6 +168,10 @@ OPTIONS\n \tscripts passing user input to grep. Multiple patterns are\n \tcombined by 'or'.\n \n+-@::\n+\tThe next parameter is a line number. This special pattern matches\n+\tthis line of the input file.\n+\n --and::\n --or::\n --not::\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 10a1f65..8b11c87 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -733,6 +733,14 @@ static int pattern_callback(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n+static int lno_pattern_callback(const struct option *opt, const char *arg,\n+\t\t\t\tint unset)\n+{\n+\tstruct grep_opt *grep_opt = opt->value;\n+\tappend_grep_pattern(grep_opt, arg, \"-@ option\", 0, GREP_LNO);\n+\treturn 0;\n+}\n+\n static int help_callback(const struct option *opt, const char *arg, int unset)\n {\n \treturn -1;\n@@ -816,6 +824,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\t\"read patterns from file\", file_callback),\n \t\t{ OPTION_CALLBACK, 'e', NULL, &opt, \"pattern\",\n \t\t\t\"match \", PARSE_OPT_NONEG, pattern_callback },\n+\t\t{ OPTION_CALLBACK, '@', NULL, &opt, \"lno\",\n+\t\t\t\"match \", PARSE_OPT_NONEG, lno_pattern_callback },\n \t\t{ OPTION_CALLBACK, 0, \"and\", &opt, NULL,\n \t\t \"combine patterns specified with -e\",\n \t\t PARSE_OPT_NOARG | PARSE_OPT_NONEG, and_callback },\n-- \n1.7.5.349.gfeb1a\n"},{"id":"166854","messageId":"BANLkTi=NOaWN56qBACoTE2WQWFM3rHXfWA@mail.gmail.com","threadId":"27236","inReplyTo":"cover.1304318972.git.bert.wesarg@googlemail.com","subject":"Re: [PATCH/RFC 0/4] grep: support to match by line number","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-05-02T11:54:37Z","receivedAt":"2011-05-02T11:54:37Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, May 2, 2011 at 13:39, Bert Wesarg wrote:\n> This series will teach git grep to match at specified line numbers. This is\n> particular usefull, if you want to see the lines which emits warnings or errors\n> from a build run (where you only get the line number) and present it nicely to\n> the user with function and context lines.\n\nCan you give a concrete example of how you'd use this? I'm not sure I\nunderstand the described use case.\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"166855","messageId":"BANLkTikvYLKKj_Wd0EOxZh+vANxju4CV6Q@mail.gmail.com","threadId":"27236","inReplyTo":"BANLkTi=NOaWN56qBACoTE2WQWFM3rHXfWA@mail.gmail.com","subject":"Re: [PATCH/RFC 0/4] grep: support to match by line number","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2011-05-02T12:20:10Z","receivedAt":"2011-05-02T12:20:10Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Mon, May 2, 2011 at 13:54, Sverre Rabbelier wrote:\n> Heya,\n>\n> On Mon, May 2, 2011 at 13:39, Bert Wesarg wrote:\n>> This series will teach git grep to match at specified line numbers. This is\n>> particular usefull, if you want to see the lines which emits warnings or errors\n>> from a build run (where you only get the line number) and present it nicely to\n>> the user with function and context lines.\n>\n> Can you give a concrete example of how you'd use this? I'm not sure I\n> understand the described use case.\n\nI parse the output of compiling a file, there may be warnings or even\nerrors, I collect the given line numbers, and then call 'git grep -p\n-C -n (-@ )+ -- ' to show me what these warnings are about.\n\nFor example clang 2.9 (while clang is a bad example, because it prints\nthe offending line already, but gcc does not generate any warnings on\nmaster currently) gives the following warning (which is probably worth\na patch, btw):\n\n CC grep.o\ngrep.c:231:16: warning: comparison of unsigned enum expression < 0 is\nalways false [-Wtautological-compare]\n if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)\n ~~~~~~~~ ^ ~\n1 warning generated.\n\nI can than call 'git grep -3 -n -h -p -@ 231 -- grep.c', and get:\n\n218=static struct grep_expr *prep_header_patterns(struct grep_opt *opt)\n--\n228- for (p = opt->header_list; p; p = p->next) {\n229- if (p->token != GREP_PATTERN_HEAD)\n230- die(\"bug: a non-header pattern in grep header list.\");\n231: if (p->field < 0 || GREP_HEADER_FIELD_MAX <= p->field)\n232- die(\"bug: unknown header field %d\", p->field);\n233- compile_regexp(p, opt);\n234- }\n\nAnother use case would be to restrict the matching to a block of\nlines, this would need the mentioned patch 5, which than will support\nline ranges, so that you can say 'match foo, but only in the first 10\nlines'.\n\nBert\n\n>\n> --\n> Cheers,\n>\n> Sverre Rabbelier\n>\n"},{"id":"166860","messageId":"BANLkTi=5Rr2t-u9dJCg6EpcbfEALJDY4wQ@mail.gmail.com","threadId":"27236","inReplyTo":"f768ea6e107cdd229a18df0bac3bf583eb1f9fc5.1304321122.git.bert.wesarg@googlemail.com","subject":"Re: [PATCH/RFC 1/4] grep: prepare for re-using the space of the regexp member in struct grep_pat","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2011-05-02T13:27:28Z","receivedAt":"2011-05-02T13:27:28Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Mon, May 2, 2011 at 8:39 AM, Bert Wesarg wrote:\n> Signed-off-by: Bert Wesarg \n> ---\n>  grep.c |   12 ++++++------\n>  grep.h |    4 +++-\n>  2 files changed, 9 insertions(+), 7 deletions(-)\n>\n> diff --git a/grep.c b/grep.c\n> index 63c4280..b8eda9e 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -70,7 +70,7 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n>        if (p->fixed)\n>                return;\n>\n> -       err = regcomp(&p->regexp, p->pattern, opt->regflags);\n> +       err = regcomp(&p->u.regexp, p->pattern, opt->regflags);\n>        if (err) {\n>                char errbuf[1024];\n>                char where[1024];\n> @@ -81,8 +81,8 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n>                        sprintf(where, \"%s, \", p->origin);\n>                else\n>                        where[0] = 0;\n> -               regerror(err, &p->regexp, errbuf, 1024);\n> -               regfree(&p->regexp);\n> +               regerror(err, &p->u.regexp, errbuf, 1024);\n> +               regfree(&p->u.regexp);\n>                die(\"%s'%s': %s\", where, p->pattern, errbuf);\n>        }\n>  }\n> @@ -320,7 +320,7 @@ void free_grep_patterns(struct grep_opt *opt)\n>                case GREP_PATTERN: /* atom */\n>                case GREP_PATTERN_HEAD:\n>                case GREP_PATTERN_BODY:\n> -                       regfree(&p->regexp);\n> +                       regfree(&p->u.regexp);\n>                        break;\n>                default:\n>                        break;\n> @@ -464,7 +464,7 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n>        if (p->fixed)\n>                hit = !fixmatch(p, bol, eol, pmatch);\n>        else\n> -               hit = !regmatch(&p->regexp, bol, eol, pmatch, eflags);\n> +               hit = !regmatch(&p->u.regexp, bol, eol, pmatch, eflags);\n>\n>        if (hit && p->word_regexp) {\n>                if ((pmatch[0].rm_so < 0) ||\n> @@ -794,7 +794,7 @@ static int look_ahead(struct grep_opt *opt,\n>                if (p->fixed)\n>                        hit = !fixmatch(p, bol, bol + *left_p, &m);\n>                else\n> -                       hit = !regmatch(&p->regexp, bol, bol + *left_p, &m, 0);\n> +                       hit = !regmatch(&p->u.regexp, bol, bol + *left_p, &m, 0);\n>                if (!hit || m.rm_so < 0 || m.rm_eo < 0)\n>                        continue;\n>                if (earliest < 0 || m.rm_so < earliest)\n> diff --git a/grep.h b/grep.h\n> index 06621fe..9912c11 100644\n> --- a/grep.h\n> +++ b/grep.h\n> @@ -32,7 +32,9 @@ struct grep_pat {\n>        const char *pattern;\n>        size_t patternlen;\n>        enum grep_header_field field;\n> -       regex_t regexp;\n> +       union {\n> +               regex_t regexp;\n> +       } u;\n\nInstead of u, would be worth to rename it to something more descriptive?\n\n>        unsigned fixed:1;\n>        unsigned ignore_case:1;\n>        unsigned word_regexp:1;\n> --\n> 1.7.5.349.gfeb1a\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"166861","messageId":"BANLkTi=HtStcEFezT9sEeRFzsGw1mMyg7Q@mail.gmail.com","threadId":"27236","inReplyTo":"2681b60988c7c4d059f83df368395eca0520012c.1304321122.git.bert.wesarg@googlemail.com","subject":"Re: [PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2011-05-02T13:30:16Z","receivedAt":"2011-05-02T13:30:16Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Mon, May 2, 2011 at 8:39 AM, Bert Wesarg wrote:\n> Signed-off-by: Bert Wesarg \n> ---\n>  grep.c |   42 +++++++++++++++++++++---------------------\n>  1 files changed, 21 insertions(+), 21 deletions(-)\n>\n> diff --git a/grep.c b/grep.c\n> index b8eda9e..f21b022 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -437,7 +437,7 @@ static struct {\n>  };\n>\n>  static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n> -                            enum grep_context ctx,\n> +                            unsigned lno, enum grep_context ctx,\n\nI'd rename lno to line_nr, so it's more clearer. Also, I'd add the new\nparamenter at the end, not in some random position (or was there some\nparticular reason to put here but not in the end?).\n\n>                             regmatch_t *pmatch, int eflags)\n>  {\n>        int hit = 0;\n> @@ -516,7 +516,7 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n>  }\n>\n>  static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n> -                          enum grep_context ctx, int collect_hits)\n> +                          unsigned lno, enum grep_context ctx, int collect_hits)\n>  {\n>        int h = 0;\n>        regmatch_t match;\n> @@ -528,25 +528,25 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n>                h = 1;\n>                break;\n>        case GREP_NODE_ATOM:\n> -               h = match_one_pattern(x->u.atom, bol, eol, ctx, &match, 0);\n> +               h = match_one_pattern(x->u.atom, bol, eol, lno, ctx, &match, 0);\n>                break;\n>        case GREP_NODE_NOT:\n> -               h = !match_expr_eval(x->u.unary, bol, eol, ctx, 0);\n> +               h = !match_expr_eval(x->u.unary, bol, eol, lno, ctx, 0);\n>                break;\n>        case GREP_NODE_AND:\n> -               if (!match_expr_eval(x->u.binary.left, bol, eol, ctx, 0))\n> +               if (!match_expr_eval(x->u.binary.left, bol, eol, lno, ctx, 0))\n>                        return 0;\n> -               h = match_expr_eval(x->u.binary.right, bol, eol, ctx, 0);\n> +               h = match_expr_eval(x->u.binary.right, bol, eol, lno, ctx, 0);\n>                break;\n>        case GREP_NODE_OR:\n>                if (!collect_hits)\n>                        return (match_expr_eval(x->u.binary.left,\n> -                                               bol, eol, ctx, 0) ||\n> +                                               bol, eol, lno, ctx, 0) ||\n>                                match_expr_eval(x->u.binary.right,\n> -                                               bol, eol, ctx, 0));\n> -               h = match_expr_eval(x->u.binary.left, bol, eol, ctx, 0);\n> +                                               bol, eol, lno, ctx, 0));\n> +               h = match_expr_eval(x->u.binary.left, bol, eol, lno, ctx, 0);\n>                x->u.binary.left->hit |= h;\n> -               h |= match_expr_eval(x->u.binary.right, bol, eol, ctx, 1);\n> +               h |= match_expr_eval(x->u.binary.right, bol, eol, lno, ctx, 1);\n>                break;\n>        default:\n>                die(\"Unexpected node type (internal error) %d\", x->node);\n> @@ -557,36 +557,36 @@ static int match_expr_eval(struct grep_expr *x, char *bol, char *eol,\n>  }\n>\n>  static int match_expr(struct grep_opt *opt, char *bol, char *eol,\n> -                     enum grep_context ctx, int collect_hits)\n> +                     unsigned lno, enum grep_context ctx, int collect_hits)\n>  {\n>        struct grep_expr *x = opt->pattern_expression;\n> -       return match_expr_eval(x, bol, eol, ctx, collect_hits);\n> +       return match_expr_eval(x, bol, eol, lno, ctx, collect_hits);\n>  }\n>\n>  static int match_line(struct grep_opt *opt, char *bol, char *eol,\n> -                     enum grep_context ctx, int collect_hits)\n> +                     unsigned lno, enum grep_context ctx, int collect_hits)\n>  {\n>        struct grep_pat *p;\n>        regmatch_t match;\n>\n>        if (opt->extended)\n> -               return match_expr(opt, bol, eol, ctx, collect_hits);\n> +               return match_expr(opt, bol, eol, lno, ctx, collect_hits);\n>\n>        /* we do not call with collect_hits without being extended */\n>        for (p = opt->pattern_list; p; p = p->next) {\n> -               if (match_one_pattern(p, bol, eol, ctx, &match, 0))\n> +               if (match_one_pattern(p, bol, eol, lno, ctx, &match, 0))\n>                        return 1;\n>        }\n>        return 0;\n>  }\n>\n>  static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,\n> -                             enum grep_context ctx,\n> +                             unsigned lno, enum grep_context ctx,\n>                              regmatch_t *pmatch, int eflags)\n>  {\n>        regmatch_t match;\n>\n> -       if (!match_one_pattern(p, bol, eol, ctx, &match, eflags))\n> +       if (!match_one_pattern(p, bol, eol, lno, ctx, &match, eflags))\n>                return 0;\n>        if (match.rm_so < 0 || match.rm_eo < 0)\n>                return 0;\n> @@ -601,7 +601,7 @@ static int match_next_pattern(struct grep_pat *p, char *bol, char *eol,\n>        return 1;\n>  }\n>\n> -static int next_match(struct grep_opt *opt, char *bol, char *eol,\n> +static int next_match(struct grep_opt *opt, char *bol, char *eol, unsigned lno,\n>                      enum grep_context ctx, regmatch_t *pmatch, int eflags)\n>  {\n>        struct grep_pat *p;\n> @@ -614,7 +614,7 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol,\n>                        case GREP_PATTERN: /* atom */\n>                        case GREP_PATTERN_HEAD:\n>                        case GREP_PATTERN_BODY:\n> -                               hit |= match_next_pattern(p, bol, eol, ctx,\n> +                               hit |= match_next_pattern(p, bol, eol, lno, ctx,\n>                                                          pmatch, eflags);\n>                                break;\n>                        default:\n> @@ -667,7 +667,7 @@ static void show_line(struct grep_opt *opt, char *bol, char *eol,\n>                else if (sign == '=')\n>                        line_color = opt->color_function;\n>                *eol = '\\0';\n> -               while (next_match(opt, bol, eol, ctx, &match, eflags)) {\n> +               while (next_match(opt, bol, eol, lno, ctx, &match, eflags)) {\n>                        if (match.rm_so == match.rm_eo)\n>                                break;\n>\n> @@ -911,7 +911,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,\n>                if ((ctx == GREP_CONTEXT_HEAD) && (eol == bol))\n>                        ctx = GREP_CONTEXT_BODY;\n>\n> -               hit = match_line(opt, bol, eol, ctx, collect_hits);\n> +               hit = match_line(opt, bol, eol, lno, ctx, collect_hits);\n>                *eol = ch;\n>\n>                if (collect_hits)\n> --\n> 1.7.5.349.gfeb1a\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"166862","messageId":"BANLkTinUWbhCKyfpimUuvCsxnCmRi3E=dg@mail.gmail.com","threadId":"27236","inReplyTo":"f85d783d43bfe7f9e9f23a10e72e8a2a83033cac.1304321122.git.bert.wesarg@googlemail.com","subject":"Re: [PATCH/RFC 3/4] grep: introduce pattern which matches at line number","fromName":"Thiago Farina","fromEmail":"tfransosi@gmail.com","sentAt":"2011-05-02T13:33:04Z","receivedAt":"2011-05-02T13:33:04Z","isPatch":true,"sender":{"key":"tfransosi@gmail.com","avatar":"https://avatars.githubusercontent.com/u/970071?v=4"},"body":"On Mon, May 2, 2011 at 8:39 AM, Bert Wesarg wrote:\n> Signed-off-by: Bert Wesarg \n> ---\n>  grep.c |   33 +++++++++++++++++++++++++++++++++\n>  grep.h |    7 +++++--\n>  2 files changed, 38 insertions(+), 2 deletions(-)\n>\n> diff --git a/grep.c b/grep.c\n> index f21b022..cbb3757 100644\n> --- a/grep.c\n> +++ b/grep.c\n> @@ -87,6 +87,15 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n>        }\n>  }\n>\n> +static void parse_lno(struct grep_pat *p, struct grep_opt *opt)\nMaybe parse_line_number?\n\n> +{\n> +       char *eon;\n> +\n> +       p->u.lno = strtoul(p->pattern, &eon, 10);\n> +       if (*eon || p->u.lno == 0)\n> +               die(\"'%s': Invalid number for line match\", p->pattern);\n> +}\n> +\n>  static struct grep_expr *compile_pattern_or(struct grep_pat **);\n>  static struct grep_expr *compile_pattern_atom(struct grep_pat **list)\n>  {\n> @@ -100,6 +109,7 @@ static struct grep_expr *compile_pattern_atom(struct grep_pat **list)\n>        case GREP_PATTERN: /* atom */\n>        case GREP_PATTERN_HEAD:\n>        case GREP_PATTERN_BODY:\n> +       case GREP_LNO:\n>                x = xcalloc(1, sizeof (struct grep_expr));\n>                x->node = GREP_NODE_ATOM;\n>                x->u.atom = p;\n> @@ -264,6 +274,9 @@ void compile_grep_patterns(struct grep_opt *opt)\n>                case GREP_PATTERN_BODY:\n>                        compile_regexp(p, opt);\n>                        break;\n> +               case GREP_LNO:\n> +                       parse_lno(p, opt);\n> +                       break;\n>                default:\n>                        opt->extended = 1;\n>                        break;\n> @@ -297,6 +310,7 @@ static void free_pattern_expr(struct grep_expr *x)\n>        switch (x->node) {\n>        case GREP_NODE_TRUE:\n>        case GREP_NODE_ATOM:\n> +       case GREP_NODE_LNO:\n>                break;\n>        case GREP_NODE_NOT:\n>                free_pattern_expr(x->u.unary);\n> @@ -436,6 +450,21 @@ static struct {\n>        { \"committer \", 10 },\n>  };\n>\n> +static int match_lno(struct grep_pat *p, char *bol, char *eol, unsigned lno,\n> +                    regmatch_t *pmatch)\n> +{\n> +       if (p->u.lno == lno) {\n> +               /*\n> +                * because coloring will stop at so == eo, match at the end\n> +                * of line, so that the whole line can be colored\n> +                */\n> +               pmatch->rm_so = eol - bol;\n> +               pmatch->rm_eo = eol - bol;\n> +               return 1;\n> +       }\n> +       return 0;\n> +}\n> +\n>  static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n>                             unsigned lno, enum grep_context ctx,\n>                             regmatch_t *pmatch, int eflags)\n> @@ -444,6 +473,9 @@ static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n>        int saved_ch = 0;\n>        const char *start = bol;\n>\n> +       if (p->token == GREP_LNO)\n> +               return match_lno(p, bol, eol, lno, pmatch);\n> +\n>        if ((p->token != GREP_PATTERN) &&\n>            ((p->token == GREP_PATTERN_HEAD) != (ctx == GREP_CONTEXT_HEAD)))\n>                return 0;\n> @@ -614,6 +646,7 @@ static int next_match(struct grep_opt *opt, char *bol, char *eol, unsigned lno,\n>                        case GREP_PATTERN: /* atom */\n>                        case GREP_PATTERN_HEAD:\n>                        case GREP_PATTERN_BODY:\n> +                       case GREP_LNO:\n>                                hit |= match_next_pattern(p, bol, eol, lno, ctx,\n>                                                          pmatch, eflags);\n>                                break;\n> diff --git a/grep.h b/grep.h\n> index 9912c11..41821f3 100644\n> --- a/grep.h\n> +++ b/grep.h\n> @@ -10,7 +10,8 @@ enum grep_pat_token {\n>        GREP_OPEN_PAREN,\n>        GREP_CLOSE_PAREN,\n>        GREP_NOT,\n> -       GREP_OR\n> +       GREP_OR,\n> +       GREP_LNO\n\nMaybe GREP_LINE_NR?\n\n\n>  };\n>\n>  enum grep_context {\n> @@ -34,6 +35,7 @@ struct grep_pat {\n>        enum grep_header_field field;\n>        union {\n>                regex_t regexp;\n> +               unsigned lno;\n>        } u;\n>        unsigned fixed:1;\n>        unsigned ignore_case:1;\n> @@ -45,7 +47,8 @@ enum grep_expr_node {\n>        GREP_NODE_NOT,\n>        GREP_NODE_AND,\n>        GREP_NODE_TRUE,\n> -       GREP_NODE_OR\n> +       GREP_NODE_OR,\n> +       GREP_NODE_LNO\n>  };\n>\n>  struct grep_expr {\n> --\n> 1.7.5.349.gfeb1a\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"166870","messageId":"BANLkTimMOg45VrfftbMWQdu6gkF35gwZBw@mail.gmail.com","threadId":"27236","inReplyTo":"BANLkTi=5Rr2t-u9dJCg6EpcbfEALJDY4wQ@mail.gmail.com","subject":"Re: [PATCH/RFC 1/4] grep: prepare for re-using the space of the regexp member in struct grep_pat","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2011-05-02T14:25:11Z","receivedAt":"2011-05-02T14:25:11Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Mon, May 2, 2011 at 15:27, Thiago Farina wrote:\n> On Mon, May 2, 2011 at 8:39 AM, Bert Wesarg wrote:\n>> diff --git a/grep.h b/grep.h\n>> index 06621fe..9912c11 100644\n>> --- a/grep.h\n>> +++ b/grep.h\n>> @@ -32,7 +32,9 @@ struct grep_pat {\n>>        const char *pattern;\n>>        size_t patternlen;\n>>        enum grep_header_field field;\n>> -       regex_t regexp;\n>> +       union {\n>> +               regex_t regexp;\n>> +       } u;\n>\n> Instead of u, would be worth to rename it to something more descriptive?\n\nI followed struct grep_expr, where u is also used. But I'm open for a\nbetter name, if you have one.\n\nBert\n"},{"id":"166871","messageId":"BANLkTimr7A64OYB51kSWe7pkjYaiOZy-ZQ@mail.gmail.com","threadId":"27236","inReplyTo":"BANLkTi=HtStcEFezT9sEeRFzsGw1mMyg7Q@mail.gmail.com","subject":"Re: [PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2011-05-02T14:29:31Z","receivedAt":"2011-05-02T14:29:31Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Mon, May 2, 2011 at 15:30, Thiago Farina wrote:\n> On Mon, May 2, 2011 at 8:39 AM, Bert Wesarg wrote:\n>> Signed-off-by: Bert Wesarg \n>> ---\n>>  grep.c |   42 +++++++++++++++++++++---------------------\n>>  1 files changed, 21 insertions(+), 21 deletions(-)\n>>\n>> diff --git a/grep.c b/grep.c\n>> index b8eda9e..f21b022 100644\n>> --- a/grep.c\n>> +++ b/grep.c\n>> @@ -437,7 +437,7 @@ static struct {\n>>  };\n>>\n>>  static int match_one_pattern(struct grep_pat *p, char *bol, char *eol,\n>> -                            enum grep_context ctx,\n>> +                            unsigned lno, enum grep_context ctx,\n>\n> I'd rename lno to line_nr, so it's more clearer. Also, I'd add the new\n> paramenter at the end, not in some random position (or was there some\n> particular reason to put here but not in the end?).\n\nlno is already in use in this file, so I followed here the precept.\nThe position was chosen, because bol/eol is the line content to match,\nso the line number should probably be near the content parameter.\n\nBert\n\n>\n>>                             regmatch_t *pmatch, int eflags)\n>>  {\n>>        int hit = 0;\n"},{"id":"166872","messageId":"BANLkTi=qKGdZpU+-aG3tuM8mssb6FnmpHw@mail.gmail.com","threadId":"27236","inReplyTo":"BANLkTinUWbhCKyfpimUuvCsxnCmRi3E=dg@mail.gmail.com","subject":"Re: [PATCH/RFC 3/4] grep: introduce pattern which matches at line number","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2011-05-02T14:32:09Z","receivedAt":"2011-05-02T14:32:09Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Mon, May 2, 2011 at 15:33, Thiago Farina wrote:\n> On Mon, May 2, 2011 at 8:39 AM, Bert Wesarg wrote:\n>> Signed-off-by: Bert Wesarg \n>> ---\n>>  grep.c |   33 +++++++++++++++++++++++++++++++++\n>>  grep.h |    7 +++++--\n>>  2 files changed, 38 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/grep.c b/grep.c\n>> index f21b022..cbb3757 100644\n>> --- a/grep.c\n>> +++ b/grep.c\n>> @@ -87,6 +87,15 @@ static void compile_regexp(struct grep_pat *p, struct grep_opt *opt)\n>>        }\n>>  }\n>>\n>> +static void parse_lno(struct grep_pat *p, struct grep_opt *opt)\n> Maybe parse_line_number?\n>\n>> +{\n>> +       char *eon;\n>> +\n>> +       p->u.lno = strtoul(p->pattern, &eon, 10);\n>> +       if (*eon || p->u.lno == 0)\n>> +               die(\"'%s': Invalid number for line match\", p->pattern);\n>> +}\n>> +\n>>  static struct grep_expr *compile_pattern_or(struct grep_pat **);\n>>  static struct grep_expr *compile_pattern_atom(struct grep_pat **list)\n>>  {\n>> diff --git a/grep.h b/grep.h\n>> index 9912c11..41821f3 100644\n>> --- a/grep.h\n>> +++ b/grep.h\n>> @@ -10,7 +10,8 @@ enum grep_pat_token {\n>>        GREP_OPEN_PAREN,\n>>        GREP_CLOSE_PAREN,\n>>        GREP_NOT,\n>> -       GREP_OR\n>> +       GREP_OR,\n>> +       GREP_LNO\n>\n> Maybe GREP_LINE_NR?\n>\n>\n>>  };\n>>\n>>  enum grep_context {\n\nYeah, I must admit that these symbols should probably get a more\ndescriptive name.\n\nBert\n"},{"id":"166876","messageId":"7vfwoxoy5c.fsf@alter.siamese.dyndns.org","threadId":"27236","inReplyTo":"cover.1304318972.git.bert.wesarg@googlemail.com","subject":"Re: [PATCH/RFC 0/4] grep: support to match by line number","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-02T16:38:55Z","receivedAt":"2011-05-02T16:38:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"I am personally not thrilled by what this series attempts to do, but first\na few questions:\n\n - Are there existing non-git \"grep\" implementations that do this?\n\n - If yes:\n - what command option letter do they use to specify line number?\n - do they not support a range notation (e.g. -@ 25-30,32-40)?\n - what do they do when given more than one file?\n\n - If no:\n - why not? Is it a sign that this is ill-thought out misfeature?\n - perhaps people use something like \"sed -n -e 25,30p file\" and be\n happy?\n"},{"id":"166877","messageId":"7vbozloy29.fsf@alter.siamese.dyndns.org","threadId":"27236","inReplyTo":"BANLkTimr7A64OYB51kSWe7pkjYaiOZy-ZQ@mail.gmail.com","subject":"Re: [PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-02T16:40:46Z","receivedAt":"2011-05-02T16:40:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bert Wesarg writes:\n\n> lno is already in use in this file, so I followed here the precept.\n> The position was chosen, because bol/eol is the line content to match,\n> so the line number should probably be near the content parameter.\n\nThe choice of \"lno\" is fine (and probably better than line_nr) as variable\nand enum names in the context of this file.\n\nIf we were to do this change, that is.\n"},{"id":"166878","messageId":"7v7ha9oxss.fsf@alter.siamese.dyndns.org","threadId":"27236","inReplyTo":"BANLkTikvYLKKj_Wd0EOxZh+vANxju4CV6Q@mail.gmail.com","subject":"Re: [PATCH/RFC 0/4] grep: support to match by line number","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-02T16:46:27Z","receivedAt":"2011-05-02T16:46:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bert Wesarg writes:\n\n>> Can you give a concrete example of how you'd use this? I'm not sure I\n>> understand the described use case.\n>\n> I parse the output of compiling a file, there may be warnings or even\n> errors, I collect the given line numbers, and then call 'git grep -p\n> -C -n (-@ )+ -- ' to show me what these warnings are about.\n\nWhat would you do next after doing that? Open the file in your editor and\njump to the line found by the grep? Oh, wait, that's the same line as\nyour compiler already found for you.\n\nAnd when you open the file in your editor, doesn't it show offending line\nin the middle of the screen? You can even fix it up right there. Isn't\nyour editor wonderful?\n\nThe extra \"grep -@\" step in-between looks like a totally made-up use case\nthat is not interesting nor convincing.\n"},{"id":"166879","messageId":"m31v0hvxi3.fsf@localhost.localdomain","threadId":"27236","inReplyTo":"7vfwoxoy5c.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC 0/4] grep: support to match by line number","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2011-05-02T17:11:59Z","receivedAt":"2011-05-02T17:11:59Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Junio C Hamano writes:\n\n> I am personally not thrilled by what this series attempts to do, but first\n> a few questions:\n> \n> - Are there existing non-git \"grep\" implementations that do this?\n> \n> - If yes:\n> - what command option letter do they use to specify line number?\n> - do they not support a range notation (e.g. -@ 25-30,32-40)?\n> - what do they do when given more than one file?\n\nI'm not sure if it is exactly the same as '-@' in Bert proposal, but\n\"ack\", grep-like text finder in Perl (http://betterthangrep.com or\nhttp://p3rl.org/ack), includes the following command line option:\n\n --line=NUM\n\n Only print line NUM of each file. Multiple lines can be given with\n multiple --line options or as a comma separated list\n (--line=3,5,7). --line=4-7 also works. The lines are always output in\n ascending order, no matter the order given on the command line.\n\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"166892","messageId":"BANLkTinVvJgE0+gxm8SdDi30-a11P2HnNA@mail.gmail.com","threadId":"27236","inReplyTo":"7vfwoxoy5c.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC 0/4] grep: support to match by line number","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2011-05-02T18:54:16Z","receivedAt":"2011-05-02T18:54:16Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Mon, May 2, 2011 at 18:38, Junio C Hamano wrote:\n> I am personally not thrilled by what this series attempts to do, but first\n> a few questions:\n>\n>  - Are there existing non-git \"grep\" implementations that do this?\n\nI didn't know of any, until Jakub mentioned the ack tool. And I didn't\nlook for one. I will answer you 'yes'-questions only in context of my\nproposal.\n\n>\n>  - If yes:\n>   - what command option letter do they use to specify line number?\n\nI have no strong argument for -@, it was short and -l and -L are\ntaken, but I wont use this that often on the command line, so a long\noption like '--line=' is ok for me too.\n\n>   - do they not support a range notation (e.g. -@ 25-30,32-40)?\n\nThat was already on my todo list and also mentioned in the last paragraph.\n\n>   - what do they do when given more than one file?\n\nLike the content patterns, they try to match.\n\n>\n>  - If no:\n>   - why not?  Is it a sign that this is ill-thought out misfeature?\n\nPrinting only some lines of a file isn't that hard, and there are\nobviously some standard tools which do this fine, but combining this\nwith the -C and --show-function feature isn't easy. So extending grep\nwith this feature sounds like the best bet.\n\n>   - perhaps people use something like \"sed -n -e 25,30p file\" and be\n>     happy?\n\nHow would you combine this with git grep HEAD or with multiple files?\n\nBert\n"},{"id":"166893","messageId":"7vy62oly2h.fsf@alter.siamese.dyndns.org","threadId":"27236","inReplyTo":"BANLkTinVvJgE0+gxm8SdDi30-a11P2HnNA@mail.gmail.com","subject":"Re: [PATCH/RFC 0/4] grep: support to match by line number","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-02T19:08:54Z","receivedAt":"2011-05-02T19:08:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bert Wesarg writes:\n\n>> - Are there existing non-git \"grep\" implementations that do this?\n>\n> I didn't know of any, until Jakub mentioned the ack tool. And I didn't\n> look for one. I will answer you 'yes'-questions only in context of my\n> proposal.\n\nThanks, but these questions are simple sanity checks done by contrasting\nyour design with _existing practices_, if any. It is not useful to answer\nwhat you did here, only to contrast your design with itself.\n\nAlso we can read them from your implementation ;-).\n\n>> - perhaps people use something like \"sed -n -e 25,30p file\" and be\n>> happy?\n>\n> How would you combine this with git grep HEAD or with multiple files?\n\nNow how did you get the compiler error messages from your example from the\nfiles in HEAD commit without checking them out?\n\nBesides, that question is still part of figuring out prior art done in the\ngit-unaware grep, so it is irrelevant that sed or other people's grep\ncannot peek into HEAD.\n"},{"id":"166895","messageId":"BANLkTi=jABd5+_R-5SjhR163WV=TF1wbmQ@mail.gmail.com","threadId":"27236","inReplyTo":"7v7ha9oxss.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC 0/4] grep: support to match by line number","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2011-05-02T19:14:22Z","receivedAt":"2011-05-02T19:14:22Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Mon, May 2, 2011 at 18:46, Junio C Hamano wrote:\n> Bert Wesarg writes:\n>\n>>> Can you give a concrete example of how you'd use this? I'm not sure I\n>>> understand the described use case.\n>>\n>> I parse the output of compiling a file, there may be warnings or even\n>> errors, I collect the given line numbers, and then call 'git grep -p\n>> -C -n (-@ )+ -- ' to show me what these warnings are about.\n>\n> What would you do next after doing that?  Open the file in your editor and\n> jump to the line found by the grep?  Oh, wait, that's the same line as\n> your compiler already found for you.\n\nA build run will probably produce many warnings or errors (think of\n-j) for many files, so you need to handle more than one file. After\nthe build run, you get a list of files which have warnings/errors,\nselecting one of the files will give you the original messages\nincluding the grep -@ output, from there you can select which message\nyou want to consider opening in the editor, so grep -@ helps you in\nyour decision what to open in your editor. If this would all be in a\nterminal your original output would be far away anyhow.\n\nI haven't implemented this 'workflow' for the terminal, but for a GUI,\nso I can jump to original output easily and open the file in the\neditor easily too. Prior to this I did this all on the command line,\nbut with recursive make this become too cumbersome.\n\n>\n> And when you open the file in your editor, doesn't it show offending line\n> in the middle of the screen?  You can even fix it up right there.  Isn't\n> your editor wonderful?\n>\n> The extra \"grep -@\" step in-between looks like a totally made-up use case\n> that is not interesting nor convincing.\n>\n>\n\nVisually parsing many warnings/errors of one file by successively\njumping to the offending line in the editor doesn't sound very\nproductive to me, but having all offending lines with context and\n--show-functions in one output does sound.\n\nBert\n"},{"id":"166898","messageId":"7vmxj4lx2e.fsf@alter.siamese.dyndns.org","threadId":"27236","inReplyTo":"BANLkTi=jABd5+_R-5SjhR163WV=TF1wbmQ@mail.gmail.com","subject":"Re: [PATCH/RFC 0/4] grep: support to match by line number","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-02T19:30:33Z","receivedAt":"2011-05-02T19:30:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Bert Wesarg writes:\n\n> A build run will probably produce many warnings or errors (think of\n> -j) for many files, so you need to handle more than one file. After\n> the build run, you get a list of files which have warnings/errors,\n> selecting one of the files will give you the original messages\n> including the grep -@ output, from there you can select which message\n> you want to consider opening in the editor, so grep -@ helps you in\n> your decision what to open in your editor.\n\nNow I got you talking ;-).\n\nWouldn't that workflow suggest that each invocation of -@ must be strongly\npaired with each path? After all, your hello.c having an error at line 100\ndoes not have much to do with which lines in your goodbye.c are buggy.\n\nWhich has larger implications by opening a big can of worms. Just a few\nexamples:\n\n - Should \"git grep -@1 -@3 foo bar\" show line 1 of foo and line 3 of bar,\n or should both lines 1 and 3 from both files be shown? Why would\n anybody want to see line 3 of foo or line 1 of bar in such a case?\n\n - Later part of command line arguments on standard git command set are\n pathspecs, not necessarily individual paths. You could for example say\n \"git grep -@1 -@3 directory/\". What should happen? Should the first\n line of the first file in directory and the third line of the second\n file in directory be shown? What would happen to the remaining files\n in the directory/? You could replace \"directory/\" with \"'*.c'\".\n\n Perhaps use of -@ need to tighten the command line parsing to make sure\n that the user specified exact number of concrete filenames, not globs\n nor leading path prefixes, and otherwise error out?\n\nI like \"this range from this file, that range from that file\" as a\nconcept. It has far wider application than just in the context of \"grep\",\nand I want to see us do it right from the beginning if we _were_ to design\nit. That is why I am poking this discussion to make sure I can solicit\ndeep enough thinking from people in the design phase.\n"},{"id":"166901","messageId":"BANLkTikOcZ9U2BJAPAvCwTFpYGcb13pDXg@mail.gmail.com","threadId":"27236","inReplyTo":"7vy62oly2h.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC 0/4] grep: support to match by line number","fromName":"Bert Wesarg","fromEmail":"bert.wesarg@googlemail.com","sentAt":"2011-05-02T19:33:48Z","receivedAt":"2011-05-02T19:33:48Z","isPatch":true,"sender":{"key":"bert.wesarg@googlemail.com","avatar":"https://avatars.githubusercontent.com/u/111934?v=4"},"body":"On Mon, May 2, 2011 at 21:08, Junio C Hamano wrote:\n> Bert Wesarg writes:\n>\n>>>  - Are there existing non-git \"grep\" implementations that do this?\n>>\n>> I didn't know of any, until Jakub mentioned the ack tool. And I didn't\n>> look for one. I will answer you 'yes'-questions only in context of my\n>> proposal.\n>\n> Thanks, but these questions are simple sanity checks done by contrasting\n> your design with _existing practices_, if any.  It is not useful to answer\n> what you did here, only to contrast your design with itself.\n\nI should probably have written 'I *can only* answer...', but I\nappreciate your questions anyhow, even if I can't answer them all.\n\n>\n> Also we can read them from your implementation ;-).\n>\n>>>   - perhaps people use something like \"sed -n -e 25,30p file\" and be\n>>>     happy?\n>>\n>> How would you combine this with git grep HEAD or with multiple files?\n>\n> Now how did you get the compiler error messages from your example from the\n> files in HEAD commit without checking them out?\n\nHaven't you heard of the git-gcc extension yet. Just kidding and point\ntaken. But I think/hope there are some more use cases than my own for\nthis feature, else I wouldn't have send the patches out.\n\nBert\n\n>\n> Besides, that question is still part of figuring out prior art done in the\n> git-unaware grep, so it is irrelevant that sed or other people's grep\n> cannot peek into HEAD.\n>\n"}]}