git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "René Scharfe" <l.s.r@web.de>
To: Zoltan Klinger <zoltan.klinger@gmail.com>, git@vger.kernel.org
Cc: Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] grep: fix match highlighting for combined patterns with context lines
Date: Sun, 26 Oct 2014 19:15:24 +0100	[thread overview]
Message-ID: <544D3A3C.4080906@web.de> (raw)
In-Reply-To: <1413870963-66431-1-git-send-email-zoltan.klinger@gmail.com>

Am 21.10.2014 um 07:56 schrieb Zoltan Klinger:
> When git grep is run with combined patterns such as '-e p1 --and -e p2'
> and surrounding context lines are requested, the output contains
> incorrectly highlighted matches.
>
> Consider the following output (highlighted matches are surrounded by '*'
> characters):
>      $ cat testfile
>      foo a
>      foo b
>      foo bar
>      baz bar foo
>      bar x
>      bar y
>      $ git grep -n -C2 -e foo --and -e bar testfile
>      testfile-1-*foo* a
>      testfile-2-*foo* b
>      testfile:3:*foo* *bar*
>      testfile:4:baz *bar* *foo*
>      testfile-5-*bar* x
>      testfile-6-*bar* y
>
> Lines 1, 2, 5 and 6 do not match the combined patterns, they only
> contain incorrectly highlighted 'false positives'.

The old code highlights all search terms, anywhere. I wouldn't call the 
ones in the context lines false positives.  The user might be interested 
in those occurrences as well (I know I am ;).

GNU grep allows coloring to be configured in much greater detail with 
its GREP_COLORS variable.  I didn't think that level of tuning is 
desirable until now.  What your patch does is equivalent to change the 
default of "ms=01;31:mc=01;31" (color matching string in selected lines 
and context lines) to "ms=01;31:mc=" (color matching string in selected 
lines).

The difference is only visible with -v or git grep's --not and --and.

So, if you really don't want matching string in context lines to be 
colored, perhaps it's time to add a color.grep.contextmatch for matching 
text in context lines?

René

  parent reply	other threads:[~2014-10-26 18:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-21  5:56 [PATCH] grep: fix match highlighting for combined patterns with context lines Zoltan Klinger
2014-10-21 19:23 ` Junio C Hamano
2014-10-21 22:40   ` Junio C Hamano
2014-10-22  0:45     ` Zoltan Klinger
2014-10-22 19:14       ` Junio C Hamano
2014-10-26 18:15 ` René Scharfe [this message]
2014-10-27 18:23   ` [PATCH][RFC] grep: add color.grep.matchcontext and color.grep.matchselected René Scharfe
2014-10-27 19:29     ` Junio C Hamano
2014-10-27 19:47       ` Junio C Hamano
2014-10-27 23:32       ` Zoltan Klinger
2014-10-28 16:50         ` Junio C Hamano
2014-10-28 18:19         ` René Scharfe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=544D3A3C.4080906@web.de \
    --to=l.s.r@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=zoltan.klinger@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).