On Sun, Apr 22, 2018 at 4:47 PM, Taylor Blau <me@xxxxxxxxxxxx> wrote:
> In a subsequent patch, we teach show_line() to optionally include the
> column number of the first match on each matched line.
>
> The regmatch_t involved in match_line() and match_one_pattern() both
> contain this information (via regmatch_t->rm_so), but their current
> implementation throws this stack variable away at the end of the call.
>
> Instead, let's teach match_line() to take in a 'regmatch_t *' so that
> callers can inspect the result of their calls. This will prove useful in
> a subsequent commit when a caller will forward on information from the
> regmatch_t into show_line (as described above).
>
> The return condition remains unchanged, therefore the only change
> required of callers is the addition of a single argument.
Is 'rm_so' the only piece of information which callers will ever want
to extract from 'regmatch_t'? If so, this patch's approach might be
overly broad, unnecessarily exposing too much of match_lines()'s
internal implementation. An alternative would be to narrow the
interface and limit exposure by passing in an 'int *matched_col' or
some such.
(Not necessarily a big deal, but something to consider.)
> Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
> ---
> diff --git a/grep.c b/grep.c
> @@ -1299,17 +1299,17 @@ static int match_expr(struct grep_opt *opt, char *bol, char *eol,
> static int match_line(struct grep_opt *opt, char *bol, char *eol,
> - enum grep_context ctx, int collect_hits)
> + regmatch_t *match, 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);
The new 'match' argument has no impact in the 'opt->extended' case.
Perhaps this deserves calling out in the commit message.
> /* 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, ctx, match, 0))
> return 1;
> }
> return 0;
