From: Michael J Gruber <git@drmicha.warpmail.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCHv2 3/6] t7810-grep: test multiple --author with --all-match
Date: Fri, 14 Sep 2012 10:14:14 +0200 [thread overview]
Message-ID: <5052E756.2040605@drmicha.warpmail.net> (raw)
In-Reply-To: <7vwqzx4i9i.fsf@alter.siamese.dyndns.org>
Junio C Hamano venit, vidit, dixit 14.09.2012 01:26:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> One possible improvement we can make is to parse the command line in
>> the last example with "--all-match" to
>>
>> [all-match]
>> (or
>> pattern_body<body>commit
>> (or
>> pattern_body<body>tag
>> (or
>> pattern_head<head 1>Linus
>> (or
>> pattern_head<head 0>Junio
>> true
>> )
>> )
>> )
>> )
>>
>> so that the backbone becomes
>>
>> - Mentions commit,
>> - Mentions tag,
>> - Committed by Linus,
>> - Authored by Junio
>>
>> to require that both commit and tag are mentioned in the message.
>
> And this is an attempt to do exactly that. Earlier, when we have
> both header expression (which by the way has to be an OR node by
> construction) and pattern expression (which could be anything), we
> created a top-level OR node (again, look at this as if you are
> reading LISP),
>
> OR
> / \
> / \
> pattern OR
> / \ / \
> ..... committer OR
> / \
> author TRUE
>
> in other words, the three elements on the top-level backbone are
> "pattern", "committer" and "author"; when there are more than one
> elements in the "pattern", the top-level node of it is OR, so that
> node is inspected by "all-match", hence the result ends up ignoring
> the "--all-match" given from the command line.
>
> This patch turns it into
>
> OR
> / \
> / \
> / OR
> committer / \
> author \
> pattern
>
> when "--all-match" was given from the command line, so that the
> "committer", "author" and elements on the top-level backbone in
> "pattern" form the top-level backbone of the resulting expression to
> be inspected by the "all-match" logic.
>
> Does this pass the expect-failure test in your [PATCH 5/6]?
Just a quick heads up:
I merged 38ed8ef (log --grep/--author: honor --all-match honored for
multiple --grep patterns, 2012-09-13) from pu into my test branch,
and this fixes what I had marked as known failure there. Thanks!
[I still have to figure out the logic, but begin to understand that
"(OR...) and "(AND...)" are linewise, and all-match is a bufferwise AND
or something. Now, what is "*or*" ...]
> grep.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git c/grep.c w/grep.c
> index be15c47..925aa92 100644
> --- c/grep.c
> +++ w/grep.c
> @@ -476,6 +476,22 @@ static struct grep_expr *prep_header_patterns(struct grep_opt *opt)
> return header_expr;
> }
>
> +static struct grep_expr *grep_splice_or(struct grep_expr *x, struct grep_expr *y)
> +{
> + struct grep_expr *z = x;
> +
> + while (x) {
> + assert(x->node == GREP_NODE_OR);
> + if (x->u.binary.right &&
> + x->u.binary.right->node == GREP_NODE_TRUE) {
> + x->u.binary.right = y;
> + break;
> + }
> + x = x->u.binary.right;
> + }
> + return z;
> +}
> +
> static void compile_grep_patterns_real(struct grep_opt *opt)
> {
> struct grep_pat *p;
> @@ -510,6 +526,9 @@ static void compile_grep_patterns_real(struct grep_opt *opt)
>
> if (!opt->pattern_expression)
> opt->pattern_expression = header_expr;
> + else if (opt->all_match)
> + opt->pattern_expression = grep_splice_or(header_expr,
> + opt->pattern_expression);
> else
> opt->pattern_expression = grep_or_expr(opt->pattern_expression,
> header_expr);
>
next prev parent reply other threads:[~2012-09-14 8:14 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-11 14:45 [PATCH] rev-list/log: document logic with several limiting options Michael J Gruber
2012-09-11 16:22 ` Junio C Hamano
2012-09-12 13:43 ` Michael J Gruber
2012-09-12 17:25 ` Junio C Hamano
2012-09-12 17:26 ` Junio C Hamano
2012-09-13 14:04 ` [PATCHv2 0/6] " Michael J Gruber
2012-09-13 14:04 ` [PATCHv2 1/6] t7810-grep: bring log --grep tests in common form Michael J Gruber
2012-09-13 14:04 ` [PATCHv2 2/6] t7810-grep: test multiple --grep with and without --all-match Michael J Gruber
2012-09-13 14:04 ` [PATCHv2 3/6] t7810-grep: test multiple --author with --all-match Michael J Gruber
2012-09-13 20:47 ` Junio C Hamano
2012-09-13 23:26 ` Junio C Hamano
2012-09-14 8:14 ` Michael J Gruber [this message]
2012-09-14 15:55 ` Junio C Hamano
2012-09-13 14:04 ` [PATCHv2 4/6] t7810-grep: test interaction of multiple --grep and --author options Michael J Gruber
2012-09-13 14:04 ` [PATCHv2 5/6] t7810-grep: test --all-match with " Michael J Gruber
2012-09-13 14:04 ` [PATCHv2 6/6] rev-list/log: document logic with several limiting options Michael J Gruber
2012-09-13 22:29 ` Junio C Hamano
2012-09-14 1:19 ` Junio C Hamano
2012-09-14 2:04 ` Junio C Hamano
2012-09-14 9:46 ` [PATCHv3 00/11] " Michael J Gruber
2012-09-14 9:46 ` [PATCHv3 01/11] grep: teach --debug option to dump the parse tree Michael J Gruber
2012-09-14 17:09 ` Junio C Hamano
2012-09-14 9:46 ` [PATCHv3 02/11] log: name --debug-grep option like in the commit message Michael J Gruber
2012-09-14 9:46 ` [PATCHv3 03/11] grep: show --debug output only once Michael J Gruber
2012-09-14 17:11 ` Junio C Hamano
2012-09-14 9:46 ` [PATCHv3 04/11] log --grep/--author: honor --all-match honored for multiple --grep patterns Michael J Gruber
2012-09-14 9:46 ` [PATCHv3 05/11] log: document use of multiple commit limiting options Michael J Gruber
2012-09-14 9:46 ` [PATCHv3 06/11] fixup! " Michael J Gruber
2012-09-14 17:23 ` Junio C Hamano
2012-09-14 9:46 ` [PATCHv3 07/11] t7810-grep: bring log --grep tests in common form Michael J Gruber
2012-09-14 9:46 ` [PATCHv3 08/11] t7810-grep: test multiple --grep with and without --all-match Michael J Gruber
2012-09-14 9:46 ` [PATCHv3 09/11] t7810-grep: test multiple --author with --all-match Michael J Gruber
2012-09-14 17:58 ` Junio C Hamano
2012-09-14 9:46 ` [PATCHv3 10/11] t7810-grep: test interaction of multiple --grep and --author options Michael J Gruber
2012-09-14 9:46 ` [PATCHv3 11/11] t7810-grep: test --all-match with " Michael J Gruber
2012-09-14 18:01 ` Junio C Hamano
2012-09-14 17:25 ` [PATCHv3 00/11] rev-list/log: document logic with several limiting options Junio C Hamano
2012-09-13 20:18 ` [PATCHv2 0/6] " Junio C Hamano
2012-09-13 7:28 ` [PATCH] " Michael J Gruber
2012-09-13 21:21 ` Junio C Hamano
2012-09-14 7:46 ` Michael J Gruber
2012-09-14 7:50 ` Junio C Hamano
2012-09-14 8:21 ` Michael J Gruber
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=5052E756.2040605@drmicha.warpmail.net \
--to=git@drmicha.warpmail.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.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.