Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'
Jeff King writes: > FWIW, I like the GNU phrasing. I thought the "non-empty" part was not > all that interesting, but after hearing that BSD behaves differently, it > is probably worth mentioning. > > I think the actual behavior of your patch matches GNU grep, and does not > need changing. Great ;-)
Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'
On Fri, Jul 06, 2018 at 03:15:22PM -0500, Taylor Blau wrote: > On Fri, Jul 06, 2018 at 11:21:06AM -0700, Junio C Hamano wrote: > > Taylor Blau writes: > > > > > I think that this might be clear enough on its own, especially since > > > this is the same as BSD grep on my machine. I think that part_s_ of a > > > line indicates that behavior, but perhaps not. On GNU grep, this is: > > > > > > Print only the matched (non-empty) parts of a matching line, with each > > > such part on a separate output line. > > > > Interesting. I wonder what "git grep -o '^'" would do ;-) > > That invocation prints nothing, but on BSD grep it prints quite a few > blank lines :-). > > I'm hesitant on sending a patch per the hunk of your reply below because > of this. Should we mirror BSD grep's behavior exactly here? I suppose > that we could somehow, but it seems like we might be doing too much to > support what appears to me to be an odd use-case. IMHO the GNU behavior (omitting non-empty matches) makes more sense. And it's also what your patch already does. ;) Although amusingly "git grep -o ^" will still print a ton of "Binary file ... matches". That _also_ matches what GNU grep does. I'm not sure if there's a saner behavior (it really has nothing to do with the funny empty match; any binary file with -o cannot show the normal text line). > > In any case, I find that the GNU phrasing is the most clear among > > the ones I've seen in this thread so far. > > OK. I'm happy to re-send that patch with the GNU phrasing depending on > what others think (and the above). I'll let this cook and collect some > thoughts over the weekend. FWIW, I like the GNU phrasing. I thought the "non-empty" part was not all that interesting, but after hearing that BSD behaves differently, it is probably worth mentioning. I think the actual behavior of your patch matches GNU grep, and does not need changing. -Peff
Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'
On Fri, Jul 06, 2018 at 11:21:06AM -0700, Junio C Hamano wrote: > Taylor Blau writes: > > > I think that this might be clear enough on its own, especially since > > this is the same as BSD grep on my machine. I think that part_s_ of a > > line indicates that behavior, but perhaps not. On GNU grep, this is: > > > > Print only the matched (non-empty) parts of a matching line, with each > > such part on a separate output line. > > Interesting. I wonder what "git grep -o '^'" would do ;-) That invocation prints nothing, but on BSD grep it prints quite a few blank lines :-). I'm hesitant on sending a patch per the hunk of your reply below because of this. Should we mirror BSD grep's behavior exactly here? I suppose that we could somehow, but it seems like we might be doing too much to support what appears to me to be an odd use-case. > > I'm happy to pick either and re-send this patch (2/2) again, if it > > wouldn't be too much to juggle. Otherwise, I can re-roll to v4. > > Please do not re-send a different version of a patch with the same > v$n value. Either re-send, otherwise re-roll, will give us v4, not > v3. > > In any case, I find that the GNU phrasing is the most clear among > the ones I've seen in this thread so far. OK. I'm happy to re-send that patch with the GNU phrasing depending on what others think (and the above). I'll let this cook and collect some thoughts over the weekend. Thanks, Taylor
Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'
Taylor Blau writes: > I think that this might be clear enough on its own, especially since > this is the same as BSD grep on my machine. I think that part_s_ of a > line indicates that behavior, but perhaps not. On GNU grep, this is: > > Print only the matched (non-empty) parts of a matching line, with each > such part on a separate output line. Interesting. I wonder what "git grep -o '^'" would do ;-) > I'm happy to pick either and re-send this patch (2/2) again, if it > wouldn't be too much to juggle. Otherwise, I can re-roll to v4. Please do not re-send a different version of a patch with the same v$n value. Either re-send, otherwise re-roll, will give us v4, not v3. In any case, I find that the GNU phrasing is the most clear among the ones I've seen in this thread so far.
Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'
On Thu, Jul 05, 2018 at 10:21:11AM -0400, Jeff King wrote: > On Tue, Jul 03, 2018 at 04:51:52PM -0500, Taylor Blau wrote: > > > diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt > > index 0de3493b80..be13fc3253 100644 > > --- a/Documentation/git-grep.txt > > +++ b/Documentation/git-grep.txt > > @@ -17,7 +17,7 @@ SYNOPSIS > >[-l | --files-with-matches] [-L | --files-without-match] > >[(-O | --open-files-in-pager) []] > >[-z | --null] > > - [-c | --count] [--all-match] [-q | --quiet] > > + [ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet] > >[--max-depth ] > >[--color[=] | --no-color] > >[--break] [--heading] [-p | --show-function] > > @@ -201,6 +201,10 @@ providing this option will cause it to die. > > Output \0 instead of the character that normally follows a > > file name. > > > > +-o:: > > +--only-matching:: > > + Output only the matching part of the lines. > > + > > Putting myself into the shoes of a naive reader, I have to wonder what > happens when there are multiple matching parts on the same line. I know > the answer from your commit message, but maybe that should be covered > here? Maybe: > > Output only the matching part of the lines. If there are multiple > matching parts, each is output on a separate line. I think that this might be clear enough on its own, especially since this is the same as BSD grep on my machine. I think that part_s_ of a line indicates that behavior, but perhaps not. On GNU grep, this is: Print only the matched (non-empty) parts of a matching line, with each such part on a separate output line. I'm happy to pick either and re-send this patch (2/2) again, if it wouldn't be too much to juggle. Otherwise, I can re-roll to v4. Thanks, Taylor
Re: [PATCH v3 0/2] grep.c: teach --only-matching to 'git-grep(1)'
On Tue, Jul 03, 2018 at 04:51:52PM -0500, Taylor Blau wrote: > diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt > index 0de3493b80..be13fc3253 100644 > --- a/Documentation/git-grep.txt > +++ b/Documentation/git-grep.txt > @@ -17,7 +17,7 @@ SYNOPSIS > [-l | --files-with-matches] [-L | --files-without-match] > [(-O | --open-files-in-pager) []] > [-z | --null] > -[-c | --count] [--all-match] [-q | --quiet] > +[ -o | --only-matching ] [-c | --count] [--all-match] [-q | --quiet] > [--max-depth ] > [--color[=] | --no-color] > [--break] [--heading] [-p | --show-function] > @@ -201,6 +201,10 @@ providing this option will cause it to die. > Output \0 instead of the character that normally follows a > file name. > > +-o:: > +--only-matching:: > + Output only the matching part of the lines. > + Putting myself into the shoes of a naive reader, I have to wonder what happens when there are multiple matching parts on the same line. I know the answer from your commit message, but maybe that should be covered here? Maybe: Output only the matching part of the lines. If there are multiple matching parts, each is output on a separate line. -Peff
