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

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Mon, 23 Apr 2018 10:01:17 +0200

On Mon, Apr 23 2018, Taylor Blau wrote:

> For your consideration: https://github.com/ttaylorr/git/compare/tb/grep-colno

Looks good to me aside from two minor issues I noticed:

 * In "grep.c: display column number of first match" you use a comment
   style we don't normally use, i.e. /**\n not /*\n. See "Multi-line
   comments" in Documentation/CodingGuidelines.

 * You're not updating contrib/git-jump/README to document the new
   output format. It just refers to the old format, but now depending on
   if you use "grep" or not it'll use this new thing. It also makes
   sense to update the example added in 007d06aa57 ("contrib/git-jump:
   allow to configure the grep command", 2017-11-20) which seems to have
   added jump.grepCmd as a workaround for not having this.

But also, after just looking at this the second time around; Is there a
reason we shouldn't just call this --column, not --column-number? I
realize the former works because of the lazyness of our getopt parsing
(also --colu though..).

I think when we add features to git-grep we should be as close to GNU
grep as possible (e.g. not add this -m alias meaning something different
as in your v1), but if GNU grep doesn't have something go with the trend
of other grep tools, as noted at
https://beyondgrep.com/feature-comparison/ (and I found another one that
has this: https://github.com/beyondgrep/website/pull/83), so there's
already 3 prominent grep tools that call this just --column.

I think we should just go with that.

Also, as a bonus question, since you're poking at this column code
anyway, interested in implementing -o (--only-matching)? That would be
super-useful (see
https://public-inbox.org/git/[email-masked]/) :)