From: René Scharfe <l.s.r@web.de>
Date: Thu, 10 May 2018 05:58:47 GMT
Subject: Re: [PATCH v5 5/7] builtin/grep.c: add '--column' option to 'git-grep(1)'
Message-ID: <4640b041-f66c-3c1f-1329-baa4580df5cc@web.de>
In-Reply-To: <xmqqefikl6jx.fsf@gitster-ct.c.googlers.com>

Am 10.05.2018 um 02:04 schrieb Junio C Hamano:
> Taylor Blau <me@ttaylorr.com> writes:
> 
>> This check we should retain and change the wording to mention '--and',
>> '--or', and '--not' specifically.
> 
> Why are these problematic in the first place?  If I said
> 
>      $ git grep -e first --and -e these
>      $ git grep -e first --and --not -e those
>      $ git grep -e first --or -e those
> 
> I'd expect that the first line of this paragraph will hit, and the
> first hit for these three are "these", "first" and "first",
> respectively.  Most importantly, in the last one, "--or" can be
> omitted and the whole thing stops being "extended", so rejecting
> extended as a whole does not make much sense.
> 
>      $ git grep -v second
>      $ git grep --not -e second
> 
> may hit all lines in this message (except for the obvious two
> lines), but we cannot say which column we found a hit.  I am
> wondering if it is too grave a sin to report "the whole line is what
> satisfied the criteria given" and say the match lies at column #1.
> 
> By doing so, obviously we can sidestep the whole "this mode is
> sometimes incompatible" and "I need to compute a lot to see if the
> given expression is compatible or not" issues.

FWIW, Silver Searcher 2.1.0 does just that:

	$ echo a | ag --column -v b
	1:a

ripgrep 0.8.1 as well:

	$ echo a | rg --column -v b
	1:1:a

Side note: This example also shows that --column implies --line-number
for ripgrep because column numbers are mostly useless without line
numbers (https://github.com/BurntSushi/ripgrep/issues/243).  I'm not
sure I'm buying that reasoning.

ack-grep 2.22 seems to have problems with that combination:

	$ echo a | ack --column -v b
	a

	$ echo a | ack -H --column -v b
	-
	Use of uninitialized value $line_parts[1] in join or string at /usr/bin/ack line 653, <STDIN> line 1.
	1::a

René