Re: [PATCH v2 2/6] grep.c: take column number as argument to show_line()

[Taylor Blau <me@ttaylorr.com>]

On Mon, Apr 23, 2018 at 03:34:21AM -0400, Eric Sunshine wrote:
> On Mon, Apr 23, 2018 at 3:27 AM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
> > On Mon, Apr 23 2018, Eric Sunshine wrote:
> >> One important issue I noticed is that patch 3/7 neglects to update
> >> grep.c:init_grep_defaults() to initialize opt.color_columnno.
> >
> > I think this is fine for fields that are 0 by default, since the struct
> >  is already zero'd out. See my e62ba43244 ("grep: remove redundant
> >  double assignment to 0", 2017-06-29) for some prior art.
>
> Indeed, I wasn't worried about opt.columnnum, which is fine being
> zero'd out by the memset(). What I was concerned about was
> opt.color_columnno; the corresponding opt.color_lineno is handled
> explicitly in init_grep_defaults():
>
>     color_set(opt->color_lineno, "");
>
> I'd expect opt.color_columnno to be treated likewise.

I agree with Ævar and Eric, we should certainly zero-out
opt->color_lineno in grep.c's init_grep_defaults().

I recall doing this in v1, but I think that I must have dropped this
part of the patch on the floor. In either case, I have amended my local
copy to include this color_set() invocation and will include it in v3
(which I hope to send later this evening).


Thanks,
Taylor