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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Apr 23, 2018, 07:27 UTC
Message-ID: <87604ie61h.fsf@evledraar.gmail.com>
In-Reply-To: <CAPig+cR0dP2Hrb4JhuqmCNFzDPoy7KdqYX3irso6mKdmh9Y4VQ@mail.gmail.com>

On Mon, Apr 23 2018, Eric Sunshine wrote:

> On Sun, Apr 22, 2018 at 9:17 PM, Taylor Blau <me@ttaylorr.com> 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.

> Looking at the tests again, are you gaining anything by placing them
> inside that for-loop? (Genuine question.)

The tests in that loop are just to stress-test grep with/without a working tree. Even though we can see from the implementation that it's the same in both cases here, I think it makes sense to add new stuff to that loop by default to stress that case.