From: Junio C Hamano <gitster@pobox•com>
To: Zoltan Klinger <zoltan.klinger@gmail•com>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] grep: fix match highlighting for combined patterns with context lines
Date: Tue, 21 Oct 2014 12:23:27 -0700 [thread overview]
Message-ID: <xmqqh9yxz0pc.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <1413870963-66431-1-git-send-email-zoltan.klinger@gmail.com> (Zoltan Klinger's message of "Tue, 21 Oct 2014 16:56:03 +1100")
Zoltan Klinger <zoltan.klinger@gmail•com> writes:
> 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'.
>
> Modify the show_line() function in grep.c to highlight matches only on
> lines that match the combined pattern. Do not highlight matches on lines
> that provide only context or contain only the function name of the match.
>
> The output of the same command after the change:
> $ 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
If your goal is to stop colouring words on context and other kinds
of lines, do you still need the "while (next_match(...))" loop for
them? Can't you make the resulting code clearer by restructuring
the inside of the whole "if (opt->color)" block further, something
along the lines of...
if (sign != ':') {
regmatch_t match; ...
enum grep_context ctx = GREP_CONTEXT_BODY;
...
while (next_match(...)) {
... the "word-by-word" loop ...
}
} else {
switch (sign) {
case '-':
line_color = opt->color_context;
break;
case ':':
line_color = opt->color_function;
break;
}
output_color(opt, bol, ..., line_color);
}
Hmm?
next prev parent reply other threads:[~2014-10-21 19:23 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 [this message]
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
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=xmqqh9yxz0pc.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=zoltan.klinger@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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