Re: [PATCH 2/2] grep.c: teach 'git grep --only-matching'
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 27, 2018, 16:40 UTC
- Message-ID
- <xmqqsh58gp3p.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <f156bd7c897359926ec407e805ecb0630a8b12da.1529961706.git.me@ttaylorr.com>
Taylor Blau <me@ttaylorr.com> writes:
Show 22 quoted lines
> - if (sign == ':')
> - match_color = opt->color_match_selected;
> - else
> - match_color = opt->color_match_context;
> - if (sign == ':')
> - line_color = opt->color_selected;
> - else if (sign == '-')
> - line_color = opt->color_context;
> - else if (sign == '=')
> - line_color = opt->color_function;
> + if (opt->color) {
> + if (sign == ':')
> + match_color = opt->color_match_selected;
> + else
> + match_color = opt->color_match_context;
> + if (sign == ':')
> + line_color = opt->color_selected;
> + else if (sign == '-')
> + line_color = opt->color_context;
> + else if (sign == '=')
> + line_color = opt->color_function;
> + }The above change (specifically, the fact that this now is enclosed in "if (opt->color) { ... }") unfortunately leaves match_color undefined at this point in the control flow. The next loop then calls output_color() with an undefined match_color and tricks stupid compiler to issue a warning and makes -Werror build to fail.
Show 12 quoted lines
> *eol = '\0';
> while (next_match(opt, bol, eol, ctx, &match, eflags)) {
> if (match.rm_so == match.rm_eo)
> break;
>
> - output_color(opt, bol, match.rm_so, line_color);
> + if (opt->only_matching)
> + show_line_header(opt, name, lno, cno, sign);
> + else
> + output_color(opt, bol, match.rm_so, line_color);
> output_color(opt, bol + match.rm_so,
> match.rm_eo - match.rm_so, match_color);output_color() does check want_color(opt->color) before using its last parameter, and want_color() gives false for opt->color that is 0 (i.e. leaves match_color to be undefined), so in this case, the compiler is worried too much, but still, we should work it around if it is easy to do so.
Just initializing match_color where it is defined at the beginning of show_line() should be sufficient, I think.