Re: [PATCH v2 0/7] grep.c: teach --column to 'git-grep(1)'

[Jeff King]

On Wed, Jun 20, 2018 at 03:05:30PM -0500, Taylor Blau wrote:

> Hi,
> 
> Here is a re-roll of my series to add --column to 'git-grep(1)'. Since
> last time, not much has changed other than the following:
> 
>   - Fix a typo where 'col', 'icol' were spelled as 'match', 'imatch'
>     [1].
> 
>   - Disable short-circuiting OR when --column is given [2].

If we're going to do this, should we be short-circuiting AND, too?

Handling just OR makes this work:

  $ ./git grep --column -e scalable --or -e fast -- README.md
  README.md:7:Git - fast, scalable, distributed revision control system
  README.md:10:Git is a fast, scalable, distributed revision control system 
with an

but not this:

  $ ./git grep --column -v --not -e scalable --and --not -e fast -- README.md
  README.md:13:Git - fast, scalable, distributed revision control system
  README.md:16:Git is a fast, scalable, distributed revision control system 
with an

I wasn't sure where we landed in the discussion on "how much crazy stuff
to support". But AFAIK, the code in this iteration handles every crazy
case already except this one. If we're going to care about OR, maybe we
should just cover all cases.

> @@ -1429,7 +1447,7 @@ static void show_line(struct grep_opt *opt, char *bol, 
> char *eol,
>        */
>       if (opt->columnnum && cno) {
>               char buf[32];
> -             xsnprintf(buf, sizeof(buf), "%d", cno);
> +             xsnprintf(buf, sizeof(buf), "%zu", cno);

Unfortunately %z isn't portable. You have to use:

  xsnprintf(buf, sizeof(buf), "%"PRIuMAX, (uintmax_t)cno);

-Peff