* [PATCH/RFC 0/4] grep: support to match by line number @ 2011-05-02 11:39 Bert Wesarg [not found] ` <cover.1304321122.git.bert.wesarg@googlemail.com> ` (2 more replies) 0 siblings, 3 replies; 22+ messages in thread From: Bert Wesarg @ 2011-05-02 11:39 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
[parent not found: <cover.1304321122.git.bert.wesarg@googlemail.com>]
* [PATCH/RFC 1/4] grep: prepare for re-using the space of the regexp member in struct grep_pat [not found] ` <cover.1304321122.git.bert.wesarg@googlemail.com> @ 2011-05-02 11:39 ` Bert Wesarg 2011-05-02 13:27 ` Thiago Farina 2011-05-02 11:39 ` [PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern Bert Wesarg ` (2 subsequent siblings) 3 siblings, 1 reply; 22+ messages in thread From: Bert Wesarg @ 2011-05-02 11:39 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 1/4] grep: prepare for re-using the space of the regexp member in struct grep_pat 2011-05-02 11:39 ` [PATCH/RFC 1/4] grep: prepare for re-using the space of the regexp member in struct grep_pat Bert Wesarg @ 2011-05-02 13:27 ` Thiago Farina 2011-05-02 14:25 ` Bert Wesarg 0 siblings, 1 reply; 22+ messages in thread From: Thiago Farina @ 2011-05-02 13:27 UTC (permalink / raw) To: Bert Wesarg; +Cc: git 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 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 1/4] grep: prepare for re-using the space of the regexp member in struct grep_pat 2011-05-02 13:27 ` Thiago Farina @ 2011-05-02 14:25 ` Bert Wesarg 0 siblings, 0 replies; 22+ messages in thread From: Bert Wesarg @ 2011-05-02 14:25 UTC (permalink / raw) To: Thiago Farina; +Cc: git 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern [not found] ` <cover.1304321122.git.bert.wesarg@googlemail.com> 2011-05-02 11:39 ` [PATCH/RFC 1/4] grep: prepare for re-using the space of the regexp member in struct grep_pat Bert Wesarg @ 2011-05-02 11:39 ` Bert Wesarg 2011-05-02 13:30 ` Thiago Farina 2011-05-02 11:39 ` [PATCH/RFC 3/4] grep: introduce pattern which matches at line number Bert Wesarg 2011-05-02 11:39 ` [PATCH/RFC 4/4] grep: provide option to match " Bert Wesarg 3 siblings, 1 reply; 22+ messages in thread From: Bert Wesarg @ 2011-05-02 11:39 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern 2011-05-02 11:39 ` [PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern Bert Wesarg @ 2011-05-02 13:30 ` Thiago Farina 2011-05-02 14:29 ` Bert Wesarg 0 siblings, 1 reply; 22+ messages in thread From: Thiago Farina @ 2011-05-02 13:30 UTC (permalink / raw) To: Bert Wesarg; +Cc: git 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 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern 2011-05-02 13:30 ` Thiago Farina @ 2011-05-02 14:29 ` Bert Wesarg 2011-05-02 16:40 ` Junio C Hamano 0 siblings, 1 reply; 22+ messages in thread From: Bert Wesarg @ 2011-05-02 14:29 UTC (permalink / raw) To: Thiago Farina; +Cc: git 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; ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern 2011-05-02 14:29 ` Bert Wesarg @ 2011-05-02 16:40 ` Junio C Hamano 0 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2011-05-02 16:40 UTC (permalink / raw) To: Bert Wesarg; +Cc: Thiago Farina, git 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH/RFC 3/4] grep: introduce pattern which matches at line number [not found] ` <cover.1304321122.git.bert.wesarg@googlemail.com> 2011-05-02 11:39 ` [PATCH/RFC 1/4] grep: prepare for re-using the space of the regexp member in struct grep_pat Bert Wesarg 2011-05-02 11:39 ` [PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern Bert Wesarg @ 2011-05-02 11:39 ` Bert Wesarg 2011-05-02 13:33 ` Thiago Farina 2011-05-02 11:39 ` [PATCH/RFC 4/4] grep: provide option to match " Bert Wesarg 3 siblings, 1 reply; 22+ messages in thread From: Bert Wesarg @ 2011-05-02 11:39 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 3/4] grep: introduce pattern which matches at line number 2011-05-02 11:39 ` [PATCH/RFC 3/4] grep: introduce pattern which matches at line number Bert Wesarg @ 2011-05-02 13:33 ` Thiago Farina 2011-05-02 14:32 ` Bert Wesarg 0 siblings, 1 reply; 22+ messages in thread From: Thiago Farina @ 2011-05-02 13:33 UTC (permalink / raw) To: Bert Wesarg; +Cc: git 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 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 3/4] grep: introduce pattern which matches at line number 2011-05-02 13:33 ` Thiago Farina @ 2011-05-02 14:32 ` Bert Wesarg 0 siblings, 0 replies; 22+ messages in thread From: Bert Wesarg @ 2011-05-02 14:32 UTC (permalink / raw) To: Thiago Farina; +Cc: git 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH/RFC 4/4] grep: provide option to match line number [not found] ` <cover.1304321122.git.bert.wesarg@googlemail.com> ` (2 preceding siblings ...) 2011-05-02 11:39 ` [PATCH/RFC 3/4] grep: introduce pattern which matches at line number Bert Wesarg @ 2011-05-02 11:39 ` Bert Wesarg 3 siblings, 0 replies; 22+ messages in thread From: Bert Wesarg @ 2011-05-02 11:39 UTC (permalink / raw) To: git; +Cc: 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 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 0/4] grep: support to match by line number 2011-05-02 11:39 [PATCH/RFC 0/4] grep: support to match by line number Bert Wesarg [not found] ` <cover.1304321122.git.bert.wesarg@googlemail.com> @ 2011-05-02 11:54 ` Sverre Rabbelier 2011-05-02 12:20 ` Bert Wesarg 2011-05-02 16:38 ` Junio C Hamano 2 siblings, 1 reply; 22+ messages in thread From: Sverre Rabbelier @ 2011-05-02 11:54 UTC (permalink / raw) To: Bert Wesarg; +Cc: git 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 0/4] grep: support to match by line number 2011-05-02 11:54 ` [PATCH/RFC 0/4] grep: support to match by " Sverre Rabbelier @ 2011-05-02 12:20 ` Bert Wesarg 2011-05-02 16:46 ` Junio C Hamano 0 siblings, 1 reply; 22+ messages in thread From: Bert Wesarg @ 2011-05-02 12:20 UTC (permalink / raw) To: Sverre Rabbelier; +Cc: git 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 > ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 0/4] grep: support to match by line number 2011-05-02 12:20 ` Bert Wesarg @ 2011-05-02 16:46 ` Junio C Hamano 2011-05-02 19:14 ` Bert Wesarg 0 siblings, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2011-05-02 16:46 UTC (permalink / raw) To: Bert Wesarg; +Cc: Sverre Rabbelier, git 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 0/4] grep: support to match by line number 2011-05-02 16:46 ` Junio C Hamano @ 2011-05-02 19:14 ` Bert Wesarg 2011-05-02 19:30 ` Junio C Hamano 0 siblings, 1 reply; 22+ messages in thread From: Bert Wesarg @ 2011-05-02 19:14 UTC (permalink / raw) To: Junio C Hamano; +Cc: Sverre Rabbelier, git 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 0/4] grep: support to match by line number 2011-05-02 19:14 ` Bert Wesarg @ 2011-05-02 19:30 ` Junio C Hamano 0 siblings, 0 replies; 22+ messages in thread From: Junio C Hamano @ 2011-05-02 19:30 UTC (permalink / raw) To: Bert Wesarg; +Cc: Sverre Rabbelier, git 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 0/4] grep: support to match by line number 2011-05-02 11:39 [PATCH/RFC 0/4] grep: support to match by line number Bert Wesarg [not found] ` <cover.1304321122.git.bert.wesarg@googlemail.com> 2011-05-02 11:54 ` [PATCH/RFC 0/4] grep: support to match by " Sverre Rabbelier @ 2011-05-02 16:38 ` Junio C Hamano 2011-05-02 17:11 ` Jakub Narebski 2011-05-02 18:54 ` Bert Wesarg 2 siblings, 2 replies; 22+ messages in thread From: Junio C Hamano @ 2011-05-02 16:38 UTC (permalink / raw) To: Bert Wesarg; +Cc: git 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? ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 0/4] grep: support to match by line number 2011-05-02 16:38 ` Junio C Hamano @ 2011-05-02 17:11 ` Jakub Narebski 2011-05-02 18:54 ` Bert Wesarg 1 sibling, 0 replies; 22+ messages in thread From: Jakub Narebski @ 2011-05-02 17:11 UTC (permalink / raw) To: Junio C Hamano; +Cc: Bert Wesarg, git 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 0/4] grep: support to match by line number 2011-05-02 16:38 ` Junio C Hamano 2011-05-02 17:11 ` Jakub Narebski @ 2011-05-02 18:54 ` Bert Wesarg 2011-05-02 19:08 ` Junio C Hamano 1 sibling, 1 reply; 22+ messages in thread From: Bert Wesarg @ 2011-05-02 18:54 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Jakub Narebski 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 ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 0/4] grep: support to match by line number 2011-05-02 18:54 ` Bert Wesarg @ 2011-05-02 19:08 ` Junio C Hamano 2011-05-02 19:33 ` Bert Wesarg 0 siblings, 1 reply; 22+ messages in thread From: Junio C Hamano @ 2011-05-02 19:08 UTC (permalink / raw) To: Bert Wesarg; +Cc: git, Jakub Narebski 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. ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH/RFC 0/4] grep: support to match by line number 2011-05-02 19:08 ` Junio C Hamano @ 2011-05-02 19:33 ` Bert Wesarg 0 siblings, 0 replies; 22+ messages in thread From: Bert Wesarg @ 2011-05-02 19:33 UTC (permalink / raw) To: Junio C Hamano; +Cc: git, Jakub Narebski 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. > ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2011-05-02 19:33 UTC | newest] Thread overview: 22+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-05-02 11:39 [PATCH/RFC 0/4] grep: support to match by line number Bert Wesarg [not found] ` <cover.1304321122.git.bert.wesarg@googlemail.com> 2011-05-02 11:39 ` [PATCH/RFC 1/4] grep: prepare for re-using the space of the regexp member in struct grep_pat Bert Wesarg 2011-05-02 13:27 ` Thiago Farina 2011-05-02 14:25 ` Bert Wesarg 2011-05-02 11:39 ` [PATCH/RFC 2/4] grep: pass current line number down to match_one_pattern Bert Wesarg 2011-05-02 13:30 ` Thiago Farina 2011-05-02 14:29 ` Bert Wesarg 2011-05-02 16:40 ` Junio C Hamano 2011-05-02 11:39 ` [PATCH/RFC 3/4] grep: introduce pattern which matches at line number Bert Wesarg 2011-05-02 13:33 ` Thiago Farina 2011-05-02 14:32 ` Bert Wesarg 2011-05-02 11:39 ` [PATCH/RFC 4/4] grep: provide option to match " Bert Wesarg 2011-05-02 11:54 ` [PATCH/RFC 0/4] grep: support to match by " Sverre Rabbelier 2011-05-02 12:20 ` Bert Wesarg 2011-05-02 16:46 ` Junio C Hamano 2011-05-02 19:14 ` Bert Wesarg 2011-05-02 19:30 ` Junio C Hamano 2011-05-02 16:38 ` Junio C Hamano 2011-05-02 17:11 ` Jakub Narebski 2011-05-02 18:54 ` Bert Wesarg 2011-05-02 19:08 ` Junio C Hamano 2011-05-02 19:33 ` Bert Wesarg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).