Re: [PATCH] revision: allow selection of commits that do not match a pattern
From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:20
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
quoted hunk ↗ jump to hunk
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.
quoted hunk ↗ jump to hunk
@@ -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 "}".
quoted hunk ↗ jump to hunk
@@ -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