[PATCH] revision: allow selection of commits that do not match a pattern

[PATCH] revision: allow selection of commits that do not match a pattern

[Sven Verdoolaege]

We do this by maintaining two lists of patterns, one for
those that should match and one for those that should not match.

Signed-off-by: Sven Verdoolaege <skimo@kotnet.org>
---
 Documentation/git-rev-list.txt |    7 ++++-
 revision.c                     |   57 ++++++++++++++++++++++++++++-----------
 revision.h                     |    1 +
 3 files changed, 48 insertions(+), 17 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 20dcac6..22bfd60 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -214,11 +214,15 @@ limiting may be applied.
 
 	Limit the commits output to ones with author/committer
 	header lines that match the specified pattern (regular expression).
+	If this option is preceded by an odd number of --not options,
+	then only commits that do not match will be shown.
 
 --grep='pattern'::
 
 	Limit the commits output to ones with log message that
 	matches the specified pattern (regular expression).
+	If this option is preceded by an odd number of --not options,
+	then only commits that do not match will be shown.
 
 --regexp-ignore-case::
 
@@ -248,7 +252,8 @@ limiting may be applied.
 --not::
 
 	Reverses the meaning of the '{caret}' prefix (or lack thereof)
-	for all following revision specifiers, up to the next '--not'.
+	for all following revision specifiers as well as the result
+	of matching a pattern, up to the next '--not'.
 
 --all::
 
diff --git a/revision.c b/revision.c
index 5184716..4bf9a0b 100644
--- a/revision.c
+++ b/revision.c
@@ -821,20 +821,30 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 	return 0;
 }
 
-static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)
+static void add_grep_to_filter(struct grep_opt **filter, const char *ptn,
+				enum grep_pat_token what)
 {
-	if (!revs->grep_filter) {
+	if (!*filter) {
 		struct grep_opt *opt = xcalloc(1, sizeof(*opt));
 		opt->status_only = 1;
 		opt->pattern_tail = &(opt->pattern_list);
 		opt->regflags = REG_NEWLINE;
-		revs->grep_filter = opt;
+		*filter = opt;
 	}
-	append_grep_pattern(revs->grep_filter, ptn,
-			    "command line", 0, what);
+	append_grep_pattern(*filter, ptn, "command line", 0, what);
 }
 
-static void add_header_grep(struct rev_info *revs, const char *field, const char *pattern)
+static void add_grep(struct rev_info *revs, const char *ptn,
+		    enum grep_pat_token what, int not)
+{
+	if (not)
+		add_grep_to_filter(&revs->grep_neg_filter, ptn, what);
+	else
+		add_grep_to_filter(&revs->grep_filter, ptn, what);
+}
+
+static void add_header_grep(struct rev_info *revs, const char *field,
+			    const char *pattern, int not)
 {
 	char *pat;
 	const char *prefix;
@@ -849,12 +859,14 @@ static void add_header_grep(struct rev_info *revs, const char *field, const char
 		pattern++;
 	}
 	sprintf(pat, "^%s %s%s", field, prefix, pattern);
-	add_grep(revs, pat, GREP_PATTERN_HEAD);
+	fprintf(stderr, "not: %d\n", not);
+	add_grep(revs, pat, GREP_PATTERN_HEAD, not);
 }
 
-static void add_message_grep(struct rev_info *revs, const char *pattern)
+static void add_message_grep(struct rev_info *revs, const char *pattern,
+				int not)
 {
-	add_grep(revs, pattern, GREP_PATTERN_BODY);
+	add_grep(revs, pattern, GREP_PATTERN_BODY, not);
 }
 
 static void add_ignore_packed(struct rev_info *revs, const char *name)
@@ -1141,15 +1153,18 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 			 * Grepping the commit log
 			 */
 			if (!prefixcmp(arg, "--author=")) {
-				add_header_grep(revs, "author", arg+9);
+				add_header_grep(revs, "author", arg+9,
+						flags & UNINTERESTING);
 				continue;
 			}
 			if (!prefixcmp(arg, "--committer=")) {
-				add_header_grep(revs, "committer", arg+12);
+				add_header_grep(revs, "committer", arg+12,
+						flags & UNINTERESTING);
 				continue;
 			}
 			if (!prefixcmp(arg, "--grep=")) {
-				add_message_grep(revs, arg+7);
+				add_message_grep(revs, arg+7,
+						flags & UNINTERESTING);
 				continue;
 			}
 			if (!prefixcmp(arg, "--extended-regexp")) {
@@ -1212,6 +1227,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 	if (revs->grep_filter)
 		revs->grep_filter->regflags |= regflags;
 
+	if (revs->grep_neg_filter)
+		revs->grep_neg_filter->regflags |= regflags;
+
 	if (show_merge)
 		prepare_show_merge(revs);
 	if (def && !revs->pending.nr) {
@@ -1249,6 +1267,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		compile_grep_patterns(revs->grep_filter);
 	}
 
+	if (revs->grep_neg_filter) {
+		compile_grep_patterns(revs->grep_neg_filter);
+	}
+
 	return left;
 }
 
@@ -1329,11 +1351,14 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
 
 static int commit_match(struct commit *commit, struct rev_info *opt)
 {
-	if (!opt->grep_filter)
-		return 1;
-	return grep_buffer(opt->grep_filter,
+	return (!opt->grep_filter ||
+		grep_buffer(opt->grep_filter,
+			   NULL, /* we say nothing, not even filename */
+			   commit->buffer, strlen(commit->buffer))) &&
+	       (!opt->grep_neg_filter ||
+		!grep_buffer(opt->grep_neg_filter,
 			   NULL, /* we say nothing, not even filename */
-			   commit->buffer, strlen(commit->buffer));
+			   commit->buffer, strlen(commit->buffer)));
 }
 
 static struct commit *get_revision_1(struct rev_info *revs)
diff --git a/revision.h b/revision.h
index f46b4d5..9728d4c 100644
--- a/revision.h
+++ b/revision.h
@@ -84,6 +84,7 @@ struct rev_info {
 
 	/* Filter by commit log message */
 	struct grep_opt	*grep_filter;
+	struct grep_opt	*grep_neg_filter;
 
 	/* special limits */
 	int skip_count;
-- 
1.5.3.rc0.40.g46ad-dirty

Re: [PATCH] revision: allow selection of commits that do not match a pattern

[Johannes Schindelin]

Hi,

On Sat, 7 Jul 2007, Sven Verdoolaege wrote:

> We do this by maintaining two lists of patterns, one for
> those that should match and one for those that should not match.

I suspect that with this patch,

	git rev-list --not --grep bugfix HEAD

does not work as expected. Why? Because the --not is heeded when 
interpreting "HEAD". And that is confusing, because you use --not for two 
completely unrelated things.

Why not make "git rev-list --grep '!bugfix' HEAD" work?

Yes, you would have to have a special exception that the prefix "!!" 
actually matches an exclamation mark, but I'd be willing to live with 
that.

Ciao,
Dscho

Re: [PATCH] revision: allow selection of commits that do not match a pattern

[Sven Verdoolaege]

On Sat, Jul 07, 2007 at 05:27:23PM +0100, Johannes Schindelin wrote:
> I suspect that with this patch,
> 
> 	git rev-list --not --grep bugfix HEAD
> 
> does not work as expected. Why?

Well... I guess that depends on what you expect...

> Why not make "git rev-list --grep '!bugfix' HEAD" work?
> 
> Yes, you would have to have a special exception that the prefix "!!" 
> actually matches an exclamation mark, but I'd be willing to live with 
> that.

Hmm... what if you want to (not) match anything starting with
one or more '!' ?
How about I add a '--invert-match' option that would
apply to all following match options?
Or we could escape the '!' with backslash.

skimo

Re: [PATCH] revision: allow selection of commits that do not match a pattern

[Johannes Schindelin]

Hi,

On Sat, 7 Jul 2007, Sven Verdoolaege wrote:

> On Sat, Jul 07, 2007 at 05:27:23PM +0100, Johannes Schindelin wrote:
> > I suspect that with this patch,
> > 
> > 	git rev-list --not --grep bugfix HEAD
> > 
> > does not work as expected. Why?
> 
> Well... I guess that depends on what you expect...

Well, at least you hopefully that it is confusing. To use --not for grep 
patterns _as well_ as for revision arguments.

> > Why not make "git rev-list --grep '!bugfix' HEAD" work?
> > 
> > Yes, you would have to have a special exception that the prefix "!!" 
> > actually matches an exclamation mark, but I'd be willing to live with 
> > that.
> 
> Hmm... what if you want to (not) match anything starting with
> one or more '!' ?

Yeah, that would not work.

> How about I add a '--invert-match' option that would
> apply to all following match options?
> Or we could escape the '!' with backslash.

If we want to match it at the beginning. Yes, that sounds more reasonable 
to me.

Ciao,
Dscho

[PATCH] revision: allow selection of commits that do not match a pattern

[Sven Verdoolaege]

We do this by maintaining two lists of patterns, one for
those that should match and one for those that should not match.

Signed-off-by: Sven Verdoolaege <skimo@kotnet.org>
---
 Documentation/git-rev-list.txt |    8 +++++
 revision.c                     |   59 +++++++++++++++++++++++++++++++--------
 revision.h                     |    1 +
 3 files changed, 56 insertions(+), 12 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 20dcac6..a27e4dd 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -214,11 +214,19 @@ limiting may be applied.
 
 	Limit the commits output to ones with author/committer
 	header lines that match the specified pattern (regular expression).
+	A pattern starting with a '!' will show only commits that do
+	not match the remainder of the pattern.
+	To match lines starting with '!', escape the initial '!'
+	with a backslash.
 
 --grep='pattern'::
 
 	Limit the commits output to ones with log message that
 	matches the specified pattern (regular expression).
+	A pattern starting with a '!' will show only commits that do
+	not match the remainder of the pattern.
+	To match lines starting with '!', escape the initial '!'
+	with a backslash.
 
 --regexp-ignore-case::
 
diff --git a/revision.c b/revision.c
index 5184716..4b00ada 100644
--- a/revision.c
+++ b/revision.c
@@ -821,40 +821,65 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 	return 0;
 }
 
-static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)
+static void add_grep_to_filter(struct grep_opt **filter, const char *ptn,
+				enum grep_pat_token what)
 {
-	if (!revs->grep_filter) {
+	if (!*filter) {
 		struct grep_opt *opt = xcalloc(1, sizeof(*opt));
 		opt->status_only = 1;
 		opt->pattern_tail = &(opt->pattern_list);
 		opt->regflags = REG_NEWLINE;
-		revs->grep_filter = opt;
+		*filter = opt;
 	}
-	append_grep_pattern(revs->grep_filter, ptn,
-			    "command line", 0, what);
+	append_grep_pattern(*filter, ptn, "command line", 0, what);
 }
 
-static void add_header_grep(struct rev_info *revs, const char *field, const char *pattern)
+static void add_grep(struct rev_info *revs, const char *ptn,
+		    enum grep_pat_token what, int negated)
+{
+	if (negated)
+		add_grep_to_filter(&revs->grep_neg_filter, ptn, what);
+	else
+		add_grep_to_filter(&revs->grep_filter, ptn, what);
+}
+
+static void add_header_grep(struct rev_info *revs, const char *field,
+			    const char *pattern)
 {
 	char *pat;
 	const char *prefix;
 	int patlen, fldlen;
+	int negated = 0;
 
 	fldlen = strlen(field);
 	patlen = strlen(pattern);
 	pat = xmalloc(patlen + fldlen + 10);
 	prefix = ".*";
+	if (*pattern == '!') {
+		negated = 1;
+		pattern++;
+	}
+	if (pattern[0] == '\\' && pattern[1] == '!')
+		pattern++;
 	if (*pattern == '^') {
 		prefix = "";
 		pattern++;
 	}
 	sprintf(pat, "^%s %s%s", field, prefix, pattern);
-	add_grep(revs, pat, GREP_PATTERN_HEAD);
+	add_grep(revs, pat, GREP_PATTERN_HEAD, negated);
 }
 
 static void add_message_grep(struct rev_info *revs, const char *pattern)
 {
-	add_grep(revs, pattern, GREP_PATTERN_BODY);
+	int negated = 0;
+
+	if (*pattern == '!') {
+		negated = 1;
+		pattern++;
+	}
+	if (pattern[0] == '\\' && pattern[1] == '!')
+		pattern++;
+	add_grep(revs, pattern, GREP_PATTERN_BODY, negated);
 }
 
 static void add_ignore_packed(struct rev_info *revs, const char *name)
@@ -1212,6 +1237,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 	if (revs->grep_filter)
 		revs->grep_filter->regflags |= regflags;
 
+	if (revs->grep_neg_filter)
+		revs->grep_neg_filter->regflags |= regflags;
+
 	if (show_merge)
 		prepare_show_merge(revs);
 	if (def && !revs->pending.nr) {
@@ -1249,6 +1277,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		compile_grep_patterns(revs->grep_filter);
 	}
 
+	if (revs->grep_neg_filter) {
+		compile_grep_patterns(revs->grep_neg_filter);
+	}
+
 	return left;
 }
 
@@ -1329,11 +1361,14 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
 
 static int commit_match(struct commit *commit, struct rev_info *opt)
 {
-	if (!opt->grep_filter)
-		return 1;
-	return grep_buffer(opt->grep_filter,
+	return (!opt->grep_filter ||
+		grep_buffer(opt->grep_filter,
+			   NULL, /* we say nothing, not even filename */
+			   commit->buffer, strlen(commit->buffer))) &&
+	       (!opt->grep_neg_filter ||
+		!grep_buffer(opt->grep_neg_filter,
 			   NULL, /* we say nothing, not even filename */
-			   commit->buffer, strlen(commit->buffer));
+			   commit->buffer, strlen(commit->buffer)));
 }
 
 static struct commit *get_revision_1(struct rev_info *revs)
diff --git a/revision.h b/revision.h
index f46b4d5..9728d4c 100644
--- a/revision.h
+++ b/revision.h
@@ -84,6 +84,7 @@ struct rev_info {
 
 	/* Filter by commit log message */
 	struct grep_opt	*grep_filter;
+	struct grep_opt	*grep_neg_filter;
 
 	/* special limits */
 	int skip_count;
-- 
1.5.3.rc0.41.ge7772

Re: [PATCH] revision: allow selection of commits that do not match a pattern

[Johannes Schindelin]

Hi,

On Sat, 7 Jul 2007, Sven Verdoolaege wrote:

> We do this by maintaining two lists of patterns, one for
> those that should match and one for those that should not match.

I would at least give one example in the commit message

> diff --git a/revision.c b/revision.c
> index 5184716..4b00ada 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -821,40 +821,65 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
>  	return 0;
>  }
>  
> -static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)
> +static void add_grep_to_filter(struct grep_opt **filter, const char *ptn,
> +				enum grep_pat_token what)
>  {
> -	if (!revs->grep_filter) {
> +	if (!*filter) {

Why not keep it "add_grep", and do a

	struct grep_opt **filter = negated ? 
		&revs->grep_neg_filter : &revs->grep_filter;

Hm? You avoid an extra function that way.

> +static void add_header_grep(struct rev_info *revs, const char *field,
> +			    const char *pattern)
>  {
>  	char *pat;
>  	const char *prefix;
>  	int patlen, fldlen;
> +	int negated = 0;
>  
>  	fldlen = strlen(field);
>  	patlen = strlen(pattern);
>  	pat = xmalloc(patlen + fldlen + 10);
>  	prefix = ".*";
> +	if (*pattern == '!') {
> +		negated = 1;
> +		pattern++;
> +	}
> +	if (pattern[0] == '\\' && pattern[1] == '!')
> +		pattern++;
>  	if (*pattern == '^') {
>  		prefix = "";
>  		pattern++;
>  	}
>  	sprintf(pat, "^%s %s%s", field, prefix, pattern);
> -	add_grep(revs, pat, GREP_PATTERN_HEAD);
> +	add_grep(revs, pat, GREP_PATTERN_HEAD, negated);
>  }

The parsing for "!" is again duplicated in add_message_grep(). Why not put 
it into add_grep(), and do

	negated = *pattern == '!';
	sprintf(pat, "%s^%s %s%s", negated ? "!" : "", field, prefix, 
		pattern + negated);

instead? No need to change the signature of add_grep(), and all callers 
get the '!' feature for free.

> @@ -1249,6 +1277,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
>  		compile_grep_patterns(revs->grep_filter);
>  	}
>  
> +	if (revs->grep_neg_filter) {
> +		compile_grep_patterns(revs->grep_neg_filter);
> +	}
> +

Please lose the "{" and "}".

> @@ -1329,11 +1361,14 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
>  
>  static int commit_match(struct commit *commit, struct rev_info *opt)
>  {
> -	if (!opt->grep_filter)
> -		return 1;
> -	return grep_buffer(opt->grep_filter,
> +	return (!opt->grep_filter ||
> +		grep_buffer(opt->grep_filter,
> +			   NULL, /* we say nothing, not even filename */
> +			   commit->buffer, strlen(commit->buffer))) &&
> +	       (!opt->grep_neg_filter ||
> +		!grep_buffer(opt->grep_neg_filter,
>  			   NULL, /* we say nothing, not even filename */
> -			   commit->buffer, strlen(commit->buffer));
> +			   commit->buffer, strlen(commit->buffer)));
>  }

Urgh! That's not nice on my eyes.

Also, I suspect that the semantics are not yet clear, what should happen 
if all_match is unset.

BTW I suspect that a better way than having two filter lists is 
demonstrated in builtin-grep.c.

Ciao,
Dscho

Re: [PATCH] revision: allow selection of commits that do not match a pattern

[Sven Verdoolaege]

On Sat, Jul 07, 2007 at 08:35:35PM +0100, Johannes Schindelin wrote:
> Why not keep it "add_grep", and do a
> 
> 	struct grep_opt **filter = negated ? 
> 		&revs->grep_neg_filter : &revs->grep_filter;
> 
> Hm? You avoid an extra function that way.

[..]
> 
> The parsing for "!" is again duplicated in add_message_grep(). Why not put 
> it into add_grep(), and do
> 
> 	negated = *pattern == '!';
> 	sprintf(pat, "%s^%s %s%s", negated ? "!" : "", field, prefix, 
> 		pattern + negated);
> 
> instead? No need to change the signature of add_grep(), and all callers 
> get the '!' feature for free.

I can do these things, but they don't exactly improve readability, IMHO.

> > @@ -1249,6 +1277,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
> >  		compile_grep_patterns(revs->grep_filter);
> >  	}
> >  
> > +	if (revs->grep_neg_filter) {
> > +		compile_grep_patterns(revs->grep_neg_filter);
> > +	}
> > +
> 
> Please lose the "{" and "}".

I may still need them for doing something with all_match...

> > @@ -1329,11 +1361,14 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
> >  
> >  static int commit_match(struct commit *commit, struct rev_info *opt)
> >  {
> > -	if (!opt->grep_filter)
> > -		return 1;
> > -	return grep_buffer(opt->grep_filter,
> > +	return (!opt->grep_filter ||
> > +		grep_buffer(opt->grep_filter,
> > +			   NULL, /* we say nothing, not even filename */
> > +			   commit->buffer, strlen(commit->buffer))) &&
> > +	       (!opt->grep_neg_filter ||
> > +		!grep_buffer(opt->grep_neg_filter,
> >  			   NULL, /* we say nothing, not even filename */
> > -			   commit->buffer, strlen(commit->buffer));
> > +			   commit->buffer, strlen(commit->buffer)));
> >  }
> 
> Urgh! That's not nice on my eyes.

You prefer

	if (opt->grep_filter && !grep_buffer(opt->grep_filter,
			   NULL, /* we say nothing, not even filename */
			   commit->buffer, strlen(commit->buffer)))
	       return 0;
	if (opt->grep_neg_filter && grep_buffer(opt->grep_neg_filter,
			   NULL, /* we say nothing, not even filename */
			   commit->buffer, strlen(commit->buffer)));
	       return 0;
       return 1;

?

> Also, I suspect that the semantics are not yet clear, what should happen 
> if all_match is unset.

So what are the semantics of all_match without negated matches?
It doesn't seem to be documented in git-rev-list.txt.

> BTW I suspect that a better way than having two filter lists is 
> demonstrated in builtin-grep.c.

Could you be a bit more specific?
If you're talking about the GREP_NOT thing, then AFAICS that is line based
and I want these things to be commit based.  That is I want to select
commits with either a or no lines that match a given pattern and not
commits that have a line that matches some patterns and not some others.

skimo

[PATCH v3] revision: allow selection of commits that do not match a pattern

[Sven Verdoolaege]

We do this by maintaining two lists of patterns, one for
those that should match and one for those that should not match.

A negative pattern is specified by putting a '!' in front.
For example, to show the commits of Jakub Narebski that
are not about gitweb, you'd do a

	git log --author='Narebski' --grep='!gitweb' --all-match

As an added bonus, this patch also documents --all-match.

Signed-off-by: Sven Verdoolaege <skimo@kotnet.org>
---
 Documentation/git-rev-list.txt |   17 ++++++++++
 revision.c                     |   64 +++++++++++++++++++++++++++++++++------
 revision.h                     |    1 +
 3 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 20dcac6..c462f5d 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -214,11 +214,19 @@ limiting may be applied.
 
 	Limit the commits output to ones with author/committer
 	header lines that match the specified pattern (regular expression).
+	A pattern starting with a '!' will show only commits that do
+	not match the remainder of the pattern.
+	To match lines starting with '!', escape the initial '!'
+	with a backslash.
 
 --grep='pattern'::
 
 	Limit the commits output to ones with log message that
 	matches the specified pattern (regular expression).
+	A pattern starting with a '!' will show only commits that do
+	not match the remainder of the pattern.
+	To match lines starting with '!', escape the initial '!'
+	with a backslash.
 
 --regexp-ignore-case::
 
@@ -229,6 +237,15 @@ limiting may be applied.
 	Consider the limiting patterns to be extended regular expressions
 	instead of the default basic regular expressions.
 
+--all-match::
+
+	Without this option, a commit is shown if any of the
+	(positive or negative) patterns matches, i.e., there
+	is at least one positive match or not all of the negative
+	patterns match.  With this options, a commit is only
+	shown if all of the patterns match, i.e., all positive
+	patterns match and no negative pattern matches.
+
 --remove-empty::
 
 	Stop when a given path disappears from the tree.
diff --git a/revision.c b/revision.c
index 5184716..0035d40 100644
--- a/revision.c
+++ b/revision.c
@@ -821,34 +821,50 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 	return 0;
 }
 
-static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)
+static void add_grep(struct rev_info *revs, const char *ptn,
+		    enum grep_pat_token what)
 {
-	if (!revs->grep_filter) {
+	int negated = 0;
+	struct grep_opt **filter;
+
+	if (ptn[0] == '\\' && ptn[1] == '!')
+		ptn++;
+	if (*ptn == '!') {
+		negated = 1;
+		ptn++;
+	}
+	filter = negated ? &revs->grep_neg_filter : &revs->grep_filter;
+	if (!*filter) {
 		struct grep_opt *opt = xcalloc(1, sizeof(*opt));
 		opt->status_only = 1;
 		opt->pattern_tail = &(opt->pattern_list);
 		opt->regflags = REG_NEWLINE;
-		revs->grep_filter = opt;
+		*filter = opt;
 	}
-	append_grep_pattern(revs->grep_filter, ptn,
-			    "command line", 0, what);
+	append_grep_pattern(*filter, ptn, "command line", 0, what);
 }
 
-static void add_header_grep(struct rev_info *revs, const char *field, const char *pattern)
+static void add_header_grep(struct rev_info *revs, const char *field,
+			    const char *pattern)
 {
 	char *pat;
-	const char *prefix;
+	const char *prefix, *negated;
 	int patlen, fldlen;
 
 	fldlen = strlen(field);
 	patlen = strlen(pattern);
 	pat = xmalloc(patlen + fldlen + 10);
+	negated = "";
+	if (*pattern == '!') {
+		negated = "!";
+		pattern++;
+	}
 	prefix = ".*";
 	if (*pattern == '^') {
 		prefix = "";
 		pattern++;
 	}
-	sprintf(pat, "^%s %s%s", field, prefix, pattern);
+	sprintf(pat, "%s^%s %s%s", negated, field, prefix, pattern);
 	add_grep(revs, pat, GREP_PATTERN_HEAD);
 }
 
@@ -1212,6 +1228,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 	if (revs->grep_filter)
 		revs->grep_filter->regflags |= regflags;
 
+	if (revs->grep_neg_filter)
+		revs->grep_neg_filter->regflags |= regflags;
+
 	if (show_merge)
 		prepare_show_merge(revs);
 	if (def && !revs->pending.nr) {
@@ -1249,6 +1268,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		compile_grep_patterns(revs->grep_filter);
 	}
 
+	if (revs->grep_neg_filter) {
+		revs->grep_neg_filter->all_match = !all_match;
+		compile_grep_patterns(revs->grep_neg_filter);
+	}
+
 	return left;
 }
 
@@ -1327,11 +1351,31 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
 	return 0;
 }
 
+/*
+ * If all_match is set, then a commit matches if all the positive
+ * patterns match and not one of the negative patterns matches.
+ * If all_match is not set, then a commit matches if at least one
+ * of the positive patterns matches or not all of the negative
+ * patterns match.
+ */
 static int commit_match(struct commit *commit, struct rev_info *opt)
 {
-	if (!opt->grep_filter)
+	int pos_match, all_match;
+
+	pos_match = !opt->grep_filter ||
+		    grep_buffer(opt->grep_filter,
+			   NULL, /* we say nothing, not even filename */
+			   commit->buffer, strlen(commit->buffer));
+	if (!opt->grep_neg_filter)
+		return pos_match;
+
+	all_match = !opt->grep_neg_filter->all_match;
+	if (!all_match && opt->grep_filter && pos_match)
 		return 1;
-	return grep_buffer(opt->grep_filter,
+	if (all_match && !pos_match)
+		return 0;
+
+	return !grep_buffer(opt->grep_neg_filter,
 			   NULL, /* we say nothing, not even filename */
 			   commit->buffer, strlen(commit->buffer));
 }
diff --git a/revision.h b/revision.h
index f46b4d5..9728d4c 100644
--- a/revision.h
+++ b/revision.h
@@ -84,6 +84,7 @@ struct rev_info {
 
 	/* Filter by commit log message */
 	struct grep_opt	*grep_filter;
+	struct grep_opt	*grep_neg_filter;
 
 	/* special limits */
 	int skip_count;
-- 
1.5.3.rc0.65.ge75d-dirty

Re: [PATCH v3] revision: allow selection of commits that do not match a pattern

[Johannes Schindelin]

Hi,

just to give you an impression of what I had in mind, here is a WIP.  It 
is not completely thought through, for example I did not make up my mind 
how to handle something like "--not --not-at-all <pattern>".  Oh, and the 
code for non-status_only is not there.  And builtin-grep does not see any 
of this, yet. But you'll get the idea:

---

 grep.c     |   19 +++++++++++++++----
 grep.h     |    3 +++
 revision.c |   13 ++++++++++++-
 3 files changed, 30 insertions(+), 5 deletions(-)

diff --git a/grep.c b/grep.c
index f67d671..40a2620 100644
--- a/grep.c
+++ b/grep.c
@@ -66,20 +66,24 @@ static struct grep_expr *compile_pattern_atom(struct grep_pat **list)
 
 static struct grep_expr *compile_pattern_not(struct grep_pat **list)
 {
+	const char *what;
 	struct grep_pat *p;
 	struct grep_expr *x;
 
 	p = *list;
 	switch (p->token) {
 	case GREP_NOT:
+	case GREP_NOT_AT_ALL:
+		what = p->token == GREP_NOT ? "--not" : "--not-at-all";
 		if (!p->next)
-			die("--not not followed by pattern expression");
+			die("%s not followed by pattern expression", what);
 		*list = p->next;
 		x = xcalloc(1, sizeof (struct grep_expr));
-		x->node = GREP_NODE_NOT;
+		x->node = p->token == GREP_NOT ?
+			GREP_NODE_NOT : GREP_NODE_NOT_AT_ALL;
 		x->u.unary = compile_pattern_not(list);
 		if (!x->u.unary)
-			die("--not followed by non pattern expression");
+			die("%s followed by non pattern expression", what);
 		return x;
 	default:
 		return compile_pattern_atom(list);
@@ -173,6 +177,7 @@ static void free_pattern_expr(struct grep_expr *x)
 	case GREP_NODE_ATOM:
 		break;
 	case GREP_NODE_NOT:
+	case GREP_NODE_NOT_AT_ALL:
 		free_pattern_expr(x->u.unary);
 		break;
 	case GREP_NODE_AND:
@@ -316,6 +321,10 @@ static int match_expr_eval(struct grep_opt *o,
 	case GREP_NODE_NOT:
 		h = !match_expr_eval(o, x->u.unary, bol, eol, ctx, 0);
 		break;
+	case GREP_NODE_NOT_AT_ALL:
+		if (match_expr_eval(o, x->u.unary, bol, eol, ctx, 0))
+			o->not_at_all = 1;
+		break;
 	case GREP_NODE_AND:
 		if (!collect_hits)
 			return (match_expr_eval(o, x->u.binary.left,
@@ -382,6 +391,8 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 	unsigned count = 0;
 	enum grep_context ctx = GREP_CONTEXT_HEAD;
 
+	opt->not_at_all = 0;
+
 	if (buffer_is_binary(buf, size)) {
 		switch (opt->binary) {
 		case GREP_BINARY_DEFAULT:
@@ -500,7 +511,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
 		return 0;
 
 	if (opt->status_only)
-		return 0;
+		return !opt->not_at_all;
 	if (opt->unmatch_name_only) {
 		/* We did not see any hit, so we want to show this */
 		printf("%s\n", name);
diff --git a/grep.h b/grep.h
index d252dd2..f80a1c2 100644
--- a/grep.h
+++ b/grep.h
@@ -10,6 +10,7 @@ enum grep_pat_token {
 	GREP_CLOSE_PAREN,
 	GREP_NOT,
 	GREP_OR,
+	GREP_NOT_AT_ALL,
 };
 
 enum grep_context {
@@ -31,6 +32,7 @@ enum grep_expr_node {
 	GREP_NODE_NOT,
 	GREP_NODE_AND,
 	GREP_NODE_OR,
+	GREP_NODE_NOT_AT_ALL,
 };
 
 struct grep_expr {
@@ -68,6 +70,7 @@ struct grep_opt {
 	unsigned extended:1;
 	unsigned relative:1;
 	unsigned pathname:1;
+	unsigned not_at_all:1; /* is set if the pattern was seen */
 	int regflags;
 	unsigned pre_context;
 	unsigned post_context;
diff --git a/revision.c b/revision.c
index 5184716..3df8a57 100644
--- a/revision.c
+++ b/revision.c
@@ -823,6 +823,7 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
 
 static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)
 {
+	int negate = *ptn == '!';
 	if (!revs->grep_filter) {
 		struct grep_opt *opt = xcalloc(1, sizeof(*opt));
 		opt->status_only = 1;
@@ -830,6 +831,13 @@ static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token
 		opt->regflags = REG_NEWLINE;
 		revs->grep_filter = opt;
 	}
+	if (negate) {
+		revs->grep_filter->extended = 1;
+		append_grep_pattern(revs->grep_filter, ptn,
+				"command line", 0, GREP_NOT_AT_ALL);
+	}
+	if (negate || ( *ptn == '\\' && ptn[1] == '!'))
+		ptn++;
 	append_grep_pattern(revs->grep_filter, ptn,
 			    "command line", 0, what);
 }
@@ -839,7 +847,10 @@ static void add_header_grep(struct rev_info *revs, const char *field, const char
 	char *pat;
 	const char *prefix;
 	int patlen, fldlen;
+	int negate = *pattern == '!';
 
+	if (negate || (*pattern == '\\' && pattern[1] == '!'))
+		pattern++;
 	fldlen = strlen(field);
 	patlen = strlen(pattern);
 	pat = xmalloc(patlen + fldlen + 10);
@@ -848,7 +859,7 @@ static void add_header_grep(struct rev_info *revs, const char *field, const char
 		prefix = "";
 		pattern++;
 	}
-	sprintf(pat, "^%s %s%s", field, prefix, pattern);
+	sprintf(pat, "%s^%s %s%s", negate ? "!" : "", field, prefix, pattern);
 	add_grep(revs, pat, GREP_PATTERN_HEAD);
 }
 

Re: [PATCH v3] revision: allow selection of commits that do not match a pattern

[Sven Verdoolaege]

On Sun, Jul 08, 2007 at 03:22:06PM +0100, Johannes Schindelin wrote:
> @@ -382,6 +391,8 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
>  	unsigned count = 0;
>  	enum grep_context ctx = GREP_CONTEXT_HEAD;
>  
> +	opt->not_at_all = 0;
> +
>  	if (buffer_is_binary(buf, size)) {
>  		switch (opt->binary) {
>  		case GREP_BINARY_DEFAULT:
> @@ -500,7 +511,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,
>  		return 0;
>  
>  	if (opt->status_only)
> -		return 0;
> +		return !opt->not_at_all;
>  	if (opt->unmatch_name_only) {
>  		/* We did not see any hit, so we want to show this */
>  		printf("%s\n", name);

I don't understand this part.
Aren't you changing the return value from 0 to 1 here if there is no NOT_AT_ALL node?

> @@ -68,6 +70,7 @@ struct grep_opt {
>  	unsigned extended:1;
>  	unsigned relative:1;
>  	unsigned pathname:1;
> +	unsigned not_at_all:1; /* is set if the pattern was seen */
>  	int regflags;
>  	unsigned pre_context;
>  	unsigned post_context;

The name for this field is also a bit confusing.
Wouldn't "matched_some_line" or some such by more appropriate?

skimo

Re: [PATCH v3] revision: allow selection of commits that do not match a pattern

[Jeff King]

On Sun, Jul 08, 2007 at 12:57:20PM +0200, Sven Verdoolaege wrote:

> We do this by maintaining two lists of patterns, one for
> those that should match and one for those that should not match.
> 
> A negative pattern is specified by putting a '!' in front.
> For example, to show the commits of Jakub Narebski that
> are not about gitweb, you'd do a
> 
> 	git log --author='Narebski' --grep='!gitweb' --all-match

I wondered if the usage might be more natural if we could grep in
separate processes, and then build filtering pipelines (like we would
with regular grep).  For example, something like:

git-rev-list HEAD |
  git-revgrep --author=Narebski |
  git-revgrep -v gitweb |
  git-log -

However, my concern was that things might get a lot slower. And they do:

$ time git-rev-list --grep=Jeff.King HEAD >/dev/null
real    0m0.553s
user    0m0.532s
sys     0m0.020s

$ time git-rev-list HEAD | git-revgrep Jeff.King >/dev/null
real    0m0.662s
user    0m1.072s
sys     0m0.036s

(note the user time -- it's a dual-CPU box, so the wall clock time is
deceptive).

So it really does double your time (or triple, if you have a second
revgrep, and so forth (although as you filter out commits the cost of
each successive revgrep goes down)).  So it's probably not worth
pursuing this approach, but I thought I would throw it out there to
document my dead-end.

Quick and dirty git-revgrep code is below.

-Peff

---
diff --git a/Makefile b/Makefile
index d7541b4..baaf4f1 100644
--- a/Makefile
+++ b/Makefile
@@ -384,7 +384,8 @@ BUILTIN_OBJS = \
 	builtin-verify-pack.o \
 	builtin-write-tree.o \
 	builtin-show-ref.o \
-	builtin-pack-refs.o
+	builtin-pack-refs.o \
+	builtin-revgrep.o
 
 GITLIBS = $(LIB_FILE) $(XDIFF_LIB)
 EXTLIBS = -lz
diff --git a/builtin-revgrep.c b/builtin-revgrep.c
new file mode 100644
index 0000000..35993c8
--- /dev/null
+++ b/builtin-revgrep.c
@@ -0,0 +1,68 @@
+#include "cache.h"
+#include "commit.h"
+#include "grep.h"
+
+static int revgrep_one(const struct commit *commit, struct grep_opt *opt)
+{
+	return grep_buffer(opt, NULL, commit->buffer, strlen(commit->buffer));
+}
+
+static void revgrep_filter(struct grep_opt *opt)
+{
+	char line[1000];
+	unsigned char sha1[20];
+	struct commit *commit;
+
+	while (fgets(line, sizeof(line), stdin) != NULL) {
+		int len = strlen(line);
+		if (line[len - 1] == '\n')
+			line[--len] = 0;
+		if (!len)
+			break;
+		get_sha1_hex(line, sha1);
+		commit = lookup_commit_reference(sha1);
+		if (!commit)
+			continue;
+		if (revgrep_one(commit, opt))
+			printf("%s\n", line);
+	}
+}
+
+int cmd_revgrep(int argc, const char **argv, const char *prefix)
+{
+	struct grep_opt opt;
+
+	memset(&opt, 0, sizeof(opt));
+	opt.pattern_tail = &opt.pattern_list;
+	opt.status_only = 1;
+
+	while (1 < argc) {
+		const char *arg = argv[1];
+		argc--; argv++;
+
+		if (!strcmp(arg, "-i")) {
+			opt.regflags |= REG_ICASE;
+			continue;
+		}
+		if (!strcmp(arg, "-v")) {
+			opt.invert = 1;
+			continue;
+		}
+		if (!strcmp(arg, "-E")) {
+			opt.regflags |= REG_EXTENDED;
+			continue;
+		}
+
+		append_grep_pattern(&opt, arg, "command line", 0,
+				GREP_PATTERN);
+	}
+
+	if (!opt.pattern_list)
+		die("no pattern given");
+
+	compile_grep_patterns(&opt);
+	revgrep_filter(&opt);
+	free_grep_patterns(&opt);
+
+	return 0;
+}
diff --git a/builtin.h b/builtin.h
index 661a92f..d185528 100644
--- a/builtin.h
+++ b/builtin.h
@@ -82,5 +82,6 @@ extern int cmd_write_tree(int argc, const char **argv, const char *prefix);
 extern int cmd_verify_pack(int argc, const char **argv, const char *prefix);
 extern int cmd_show_ref(int argc, const char **argv, const char *prefix);
 extern int cmd_pack_refs(int argc, const char **argv, const char *prefix);
+extern int cmd_revgrep(int argc, const char **argv, const char *prefix);
 
 #endif
diff --git a/git.c b/git.c
index a647f9c..d9ec129 100644
--- a/git.c
+++ b/git.c
@@ -374,6 +374,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "verify-pack", cmd_verify_pack },
 		{ "show-ref", cmd_show_ref, RUN_SETUP },
 		{ "pack-refs", cmd_pack_refs, RUN_SETUP },
+		{ "revgrep", cmd_revgrep, RUN_SETUP },
 	};
 	int i;
 






> 
> As an added bonus, this patch also documents --all-match.
> 
> Signed-off-by: Sven Verdoolaege <skimo@kotnet.org>
> ---
>  Documentation/git-rev-list.txt |   17 ++++++++++
>  revision.c                     |   64 +++++++++++++++++++++++++++++++++------
>  revision.h                     |    1 +
>  3 files changed, 72 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
> index 20dcac6..c462f5d 100644
> --- a/Documentation/git-rev-list.txt
> +++ b/Documentation/git-rev-list.txt
> @@ -214,11 +214,19 @@ limiting may be applied.
>  
>  	Limit the commits output to ones with author/committer
>  	header lines that match the specified pattern (regular expression).
> +	A pattern starting with a '!' will show only commits that do
> +	not match the remainder of the pattern.
> +	To match lines starting with '!', escape the initial '!'
> +	with a backslash.
>  
>  --grep='pattern'::
>  
>  	Limit the commits output to ones with log message that
>  	matches the specified pattern (regular expression).
> +	A pattern starting with a '!' will show only commits that do
> +	not match the remainder of the pattern.
> +	To match lines starting with '!', escape the initial '!'
> +	with a backslash.
>  
>  --regexp-ignore-case::
>  
> @@ -229,6 +237,15 @@ limiting may be applied.
>  	Consider the limiting patterns to be extended regular expressions
>  	instead of the default basic regular expressions.
>  
> +--all-match::
> +
> +	Without this option, a commit is shown if any of the
> +	(positive or negative) patterns matches, i.e., there
> +	is at least one positive match or not all of the negative
> +	patterns match.  With this options, a commit is only
> +	shown if all of the patterns match, i.e., all positive
> +	patterns match and no negative pattern matches.
> +
>  --remove-empty::
>  
>  	Stop when a given path disappears from the tree.
> diff --git a/revision.c b/revision.c
> index 5184716..0035d40 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -821,34 +821,50 @@ int handle_revision_arg(const char *arg, struct rev_info *revs,
>  	return 0;
>  }
>  
> -static void add_grep(struct rev_info *revs, const char *ptn, enum grep_pat_token what)
> +static void add_grep(struct rev_info *revs, const char *ptn,
> +		    enum grep_pat_token what)
>  {
> -	if (!revs->grep_filter) {
> +	int negated = 0;
> +	struct grep_opt **filter;
> +
> +	if (ptn[0] == '\\' && ptn[1] == '!')
> +		ptn++;
> +	if (*ptn == '!') {
> +		negated = 1;
> +		ptn++;
> +	}
> +	filter = negated ? &revs->grep_neg_filter : &revs->grep_filter;
> +	if (!*filter) {
>  		struct grep_opt *opt = xcalloc(1, sizeof(*opt));
>  		opt->status_only = 1;
>  		opt->pattern_tail = &(opt->pattern_list);
>  		opt->regflags = REG_NEWLINE;
> -		revs->grep_filter = opt;
> +		*filter = opt;
>  	}
> -	append_grep_pattern(revs->grep_filter, ptn,
> -			    "command line", 0, what);
> +	append_grep_pattern(*filter, ptn, "command line", 0, what);
>  }
>  
> -static void add_header_grep(struct rev_info *revs, const char *field, const char *pattern)
> +static void add_header_grep(struct rev_info *revs, const char *field,
> +			    const char *pattern)
>  {
>  	char *pat;
> -	const char *prefix;
> +	const char *prefix, *negated;
>  	int patlen, fldlen;
>  
>  	fldlen = strlen(field);
>  	patlen = strlen(pattern);
>  	pat = xmalloc(patlen + fldlen + 10);
> +	negated = "";
> +	if (*pattern == '!') {
> +		negated = "!";
> +		pattern++;
> +	}
>  	prefix = ".*";
>  	if (*pattern == '^') {
>  		prefix = "";
>  		pattern++;
>  	}
> -	sprintf(pat, "^%s %s%s", field, prefix, pattern);
> +	sprintf(pat, "%s^%s %s%s", negated, field, prefix, pattern);
>  	add_grep(revs, pat, GREP_PATTERN_HEAD);
>  }
>  
> @@ -1212,6 +1228,9 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
>  	if (revs->grep_filter)
>  		revs->grep_filter->regflags |= regflags;
>  
> +	if (revs->grep_neg_filter)
> +		revs->grep_neg_filter->regflags |= regflags;
> +
>  	if (show_merge)
>  		prepare_show_merge(revs);
>  	if (def && !revs->pending.nr) {
> @@ -1249,6 +1268,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
>  		compile_grep_patterns(revs->grep_filter);
>  	}
>  
> +	if (revs->grep_neg_filter) {
> +		revs->grep_neg_filter->all_match = !all_match;
> +		compile_grep_patterns(revs->grep_neg_filter);
> +	}
> +
>  	return left;
>  }
>  
> @@ -1327,11 +1351,31 @@ static int rewrite_parents(struct rev_info *revs, struct commit *commit)
>  	return 0;
>  }
>  
> +/*
> + * If all_match is set, then a commit matches if all the positive
> + * patterns match and not one of the negative patterns matches.
> + * If all_match is not set, then a commit matches if at least one
> + * of the positive patterns matches or not all of the negative
> + * patterns match.
> + */
>  static int commit_match(struct commit *commit, struct rev_info *opt)
>  {
> -	if (!opt->grep_filter)
> +	int pos_match, all_match;
> +
> +	pos_match = !opt->grep_filter ||
> +		    grep_buffer(opt->grep_filter,
> +			   NULL, /* we say nothing, not even filename */
> +			   commit->buffer, strlen(commit->buffer));
> +	if (!opt->grep_neg_filter)
> +		return pos_match;
> +
> +	all_match = !opt->grep_neg_filter->all_match;
> +	if (!all_match && opt->grep_filter && pos_match)
>  		return 1;
> -	return grep_buffer(opt->grep_filter,
> +	if (all_match && !pos_match)
> +		return 0;
> +
> +	return !grep_buffer(opt->grep_neg_filter,
>  			   NULL, /* we say nothing, not even filename */
>  			   commit->buffer, strlen(commit->buffer));
>  }
> diff --git a/revision.h b/revision.h
> index f46b4d5..9728d4c 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -84,6 +84,7 @@ struct rev_info {
>  
>  	/* Filter by commit log message */
>  	struct grep_opt	*grep_filter;
> +	struct grep_opt	*grep_neg_filter;
>  
>  	/* special limits */
>  	int skip_count;
> -- 
> 1.5.3.rc0.65.ge75d-dirty
> 
> -
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html