Re: [PATCH v5 5/7] builtin/grep.c: add '--column' option to 'git-grep(1)'

[Taylor Blau <me@ttaylorr.com>]

On Thu, May 10, 2018 at 09:04:34AM +0900, Junio C Hamano wrote:
> 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.

Agreed that this is what I would expect, too. The trouble is that we
never do a compilation step from containing --and, --or, --not to a
POSIX regexp. So, if we're extended, we have to assume that there might
not be an answer.

Given this, I don't think we should categorically die() when we
encounter this (more below in the next hunk of this mail). I think this
thread has established that there are certainly cases where we cannot
provide a meaningful answer, (--not at the top-level, for instance) but
there are cases where we can (as you indicate above).

One day, I would like to support --column with --and, --or, or --not in
cases where there _is_ a definite answer. That said, omitting this
information for now and at least not die()-ing I think is worth taking
this patch earlier, and leaving some #leftoverbits.

>     $ 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.

I think that is sensible. I previously was opposed to this because I
thought that it would be too difficult to script around the 'sometimes
we have columns but other times not' and 'I gave --column' but have to
check whether or not they are really there.

I no longer believe that my above argument is sound. It simplifies the
matter greatly to simply not share columns when we don't have a good
answer, and do when we do.

In other terms:

  * not giving '--column' will _never_ give a column,
  * '--column --invert' will _always_ die(), and
  * '--column --[and | or | not]' will _never_ give a column.

Thanks,
Taylor