All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.