Re: [PATCH] git-log: added --none-match option
Junio C Hamano writes: > Christoph Junghans writes: > >> The only useful thing I could image is using it in conjunction with >> --files-with-matches, but that is what --files-without-match is for. > > Yes, "-l" was exactly what I had in mind and I was hoping that "git > grep -l --no-match -e WIP -e TODO -e FIXME -e NEEDSWORK" may be a > way to find perfect files without needing any work. > > You can say "git grep -L -e WIP -e TODO -e FIXME -e NEEDSWORK" > instead. I missed that "-L" option. Thanks for your patience. I should have realized that this not only can be "log-only" but _should_ be "log-only". As you pointed out, when we already have "-L", trying to extend it to "grep" does not make much sense. Continuing this line of thought, as we determined that it is pointless to have this at "grep" level and it is needed only in the "log" family of commands, I would very much prefer the approach taken by your original "log --invert-grep" patch. I would further say that I prefer not to touch grep_opt at all. The new "global-invert bit" is about "we'd run the usual grep thing on the log message, and instead of filtering to only show the commits with matching message, we only show the ones with messages that do not match". That logically belongs to the revision struct that is used to interpret what the underlying grep machinery figured out, and should not have to affect the way how underlying grep machinery works. We would want a few test to make sure that we do not break the feature in the future changes. Here is an attempt. The patch would apply any commit your original "--invert-grep" option would have applied. Thanks again. -- >8 -- Subject: log: teach --invert-grep option "git log --grep=" shows only commits with messages that match the given string, but sometimes it is useful to be able to show only commits that do *not* have certain messages (e.g. "show me ones that are not FIXUP commits"). Signed-off-by: Christoph Junghans Signed-off-by: Junio C Hamano --- jc: Christoph originally had the invert-grep flag in grep_opt, but because "git grep --invert-grep" does not make sense except in conjunction with "--files-with-matches", which is already covered by "--files-without-matches", I moved it to revisions structure. I think it expresses what the feature is about better to have the flag there. When the newly inserted two tests run, the history would have commits with messages "initial", "second", "third", "fourth", "fifth", "sixth" and Second", committed in this order. The first commit that does not match either "th" or "Sec" is "second", and "initial" is the one that does not match either "th" or "Sec" case insensitively. Documentation/rev-list-options.txt | 4 contrib/completion/git-completion.bash | 2 +- revision.c | 4 +++- revision.h | 2 ++ t/t4202-log.sh | 12 5 files changed, 22 insertions(+), 2 deletions(-) diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index deb8cca..05aa997 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -66,6 +66,10 @@ if it is part of the log message. Limit the commits output to ones that match all given `--grep`, instead of ones that match at least one. +--invert-grep:: + Limit the commits output to ones with log message that do not + match the pattern specified with `--grep=`. + -i:: --regexp-ignore-case:: Match the regular expression limiting patterns without regard to letter diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 06bf262..53857f0 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1428,7 +1428,7 @@ __git_log_gitk_options=" # Options that go well for log and shortlog (not gitk) __git_log_shortlog_options=" --author= --committer= --grep= - --all-match + --all-match --invert-grep " __git_log_pretty_formats="oneline short medium full fuller email raw format:" diff --git a/revision.c b/revision.c index 615535c..84b33a3 100644 --- a/revision.c +++ b/revision.c @@ -1952,6 +1952,8 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg grep_set_pattern_type_option(GREP_PATTERN_TYPE_PCRE, &revs->grep_filter); } else if (!strcmp(arg, "--all-match")) { revs->grep_filter.all_match = 1; + } else if (!strcmp(arg, "--invert-grep")) { + revs->invert_grep = 1; } else if ((argcount = parse_long_opt("encoding", argv, &optarg))) { if (strcmp(optarg, "none")) git_log_output_encoding = xstrdup(optarg); @@ -2848,7 +2850,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
Re: [PATCH] git-log: added --none-match option
Christoph Junghans writes: > The only useful thing I could image is using it in conjunction with > --files-with-matches, but that is what --files-without-match is for. Yes, "-l" was exactly what I had in mind and I was hoping that "git grep -l --no-match -e WIP -e TODO -e FIXME -e NEEDSWORK" may be a way to find perfect files without needing any work. You can say "git grep -L -e WIP -e TODO -e FIXME -e NEEDSWORK" instead. I missed that "-L" option. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] git-log: added --none-match option
2015-01-06 16:02 GMT-07:00 Junio C Hamano : > Christoph Junghans writes: > >> Implements a inverted match for "git log", like in the case of >> "git grep -v", which is useful from time to time to e.g. filter >> FIXUP message out of "git log". >> >> Internally, a new bol 'none_match' has been introduces as >> revs->grep_filter.invert inverts the match line-wise, which cannot >> work as i.e. empty line always not match the pattern given. >> >> Signed-off-by: Christoph Junghans >> --- > > The patch itself looks like a good start, except that the above > description no longer matches the implementation. > > I further suspect it would be better to rename all_match to > all_or_none and then you can lose the "these two are mutually > incompatible" check that is placed together with a wrong existing > comment. I also notice that you forgot to update the "git grep" > where the original "--all-match" came from. That was on purpose. I am not quite sure what would be the point of "showing only matches from files that match no patterns" (option description from your patch below). If a file matches none of the patterns, what matches are there to show? The only useful thing I could image is using it in conjunction with --files-with-matches, but that is what --files-without-match is for. > > A partial fix-up may start like this on top of your version. By > renaming the variable used in the existing code, the compiler will > remind you that there are a few more places that your patch did not > touch that does something special for --all-match, which are a good > candidates you need to think if doing something similarly special > for the --none-match case is necessary. > > Thanks. > > diff --git a/builtin/grep.c b/builtin/grep.c > index 4063882..9ba4254 100644 > --- a/builtin/grep.c > +++ b/builtin/grep.c > @@ -727,8 +727,12 @@ int cmd_grep(int argc, const char **argv, const char > *prefix) > close_callback }, > OPT__QUIET(&opt.status_only, >N_("indicate hit with exit status without > output")), > - OPT_BOOL(0, "all-match", &opt.all_match, > - N_("show only matches from files that match all > patterns")), > + OPT_SET_INT(0, "all-match", &opt.all_or_none, > + N_("show only matches from files that match all > patterns"), > + GREP_ALL_MATCH), > + OPT_SET_INT(0, "none-match", &opt.all_or_none, > + N_("show only matches from files that match no > patterns"), > + GREP_NONE_MATCH), > { OPTION_SET_INT, 0, "debug", &opt.debug, NULL, > N_("show parse tree for grep expression"), > PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1 }, > diff --git a/grep.c b/grep.c > index f486ee5..1ff5dea 100644 > --- a/grep.c > +++ b/grep.c > @@ -1622,14 +1622,18 @@ static int chk_hit_marker(struct grep_expr *x) > > int grep_source(struct grep_opt *opt, struct grep_source *gs) > { > - if(opt->none_match) > - return !grep_source_1(opt, gs, 0); > /* > * we do not have to do the two-pass grep when we do not check > -* buffer-wide "all-match". > +* buffer-wide "all-match" or "none-match". > */ > - if (!opt->all_match) > + switch (opt->all_or_none) { > + case GREP_ALL_MATCH: > return grep_source_1(opt, gs, 0); > + case GREP_NONE_MATCH: > + return !grep_source_1(opt, gs, 0); > + default: > + break; > + } > > /* Otherwise the toplevel "or" terms hit a bit differently. > * We first clear hit markers from them. > diff --git a/grep.h b/grep.h > index 8e50c95..2cdabf2 100644 > --- a/grep.h > +++ b/grep.h > @@ -101,8 +101,9 @@ struct grep_opt { > int count; > int word_regexp; > int fixed; > - int all_match; > - int none_match; > +#define GREP_ALL_MATCH 1 > +#define GREP_NONE_MATCH 2 > + int all_or_none; > int debug; > #define GREP_BINARY_DEFAULT0 > #define GREP_BINARY_NOMATCH1 > diff --git a/revision.c b/revision.c > index d43779e..b955848 100644 > --- a/revision.c > +++ b/revision.c > @@ -2010,9 +2010,9 @@ static int handle_revision_opt(struct rev_info *revs, > int argc, const char **arg > } else if (!strcmp(arg, "--perl-regexp")) { > grep_set_pattern_type_option(GREP_PATTERN_TYPE_PCRE, > &revs->grep_filter); > } else if (!strcmp(arg, "--all-match")) { > - revs->grep_filter.all_match = 1; > + revs->grep_filter.all_or_none = GREP_ALL_MATCH; > } else if (!strcmp(arg, "--none-match")) { > - revs->grep_filter.none_match = 1; > + revs->grep_filter.all_or_none = GREP_NONE_MATCH; > } else if ((argcount = parse_long_opt("encoding", argv, &optarg))) { >
Re: [PATCH] git-log: added --none-match option
Christoph Junghans writes: > Implements a inverted match for "git log", like in the case of > "git grep -v", which is useful from time to time to e.g. filter > FIXUP message out of "git log". > > Internally, a new bol 'none_match' has been introduces as > revs->grep_filter.invert inverts the match line-wise, which cannot > work as i.e. empty line always not match the pattern given. > > Signed-off-by: Christoph Junghans > --- The patch itself looks like a good start, except that the above description no longer matches the implementation. I further suspect it would be better to rename all_match to all_or_none and then you can lose the "these two are mutually incompatible" check that is placed together with a wrong existing comment. I also notice that you forgot to update the "git grep" where the original "--all-match" came from. A partial fix-up may start like this on top of your version. By renaming the variable used in the existing code, the compiler will remind you that there are a few more places that your patch did not touch that does something special for --all-match, which are a good candidates you need to think if doing something similarly special for the --none-match case is necessary. Thanks. diff --git a/builtin/grep.c b/builtin/grep.c index 4063882..9ba4254 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -727,8 +727,12 @@ int cmd_grep(int argc, const char **argv, const char *prefix) close_callback }, OPT__QUIET(&opt.status_only, N_("indicate hit with exit status without output")), - OPT_BOOL(0, "all-match", &opt.all_match, - N_("show only matches from files that match all patterns")), + OPT_SET_INT(0, "all-match", &opt.all_or_none, + N_("show only matches from files that match all patterns"), + GREP_ALL_MATCH), + OPT_SET_INT(0, "none-match", &opt.all_or_none, + N_("show only matches from files that match no patterns"), + GREP_NONE_MATCH), { OPTION_SET_INT, 0, "debug", &opt.debug, NULL, N_("show parse tree for grep expression"), PARSE_OPT_NOARG | PARSE_OPT_HIDDEN, NULL, 1 }, diff --git a/grep.c b/grep.c index f486ee5..1ff5dea 100644 --- a/grep.c +++ b/grep.c @@ -1622,14 +1622,18 @@ static int chk_hit_marker(struct grep_expr *x) int grep_source(struct grep_opt *opt, struct grep_source *gs) { - if(opt->none_match) - return !grep_source_1(opt, gs, 0); /* * we do not have to do the two-pass grep when we do not check -* buffer-wide "all-match". +* buffer-wide "all-match" or "none-match". */ - if (!opt->all_match) + switch (opt->all_or_none) { + case GREP_ALL_MATCH: return grep_source_1(opt, gs, 0); + case GREP_NONE_MATCH: + return !grep_source_1(opt, gs, 0); + default: + break; + } /* Otherwise the toplevel "or" terms hit a bit differently. * We first clear hit markers from them. diff --git a/grep.h b/grep.h index 8e50c95..2cdabf2 100644 --- a/grep.h +++ b/grep.h @@ -101,8 +101,9 @@ struct grep_opt { int count; int word_regexp; int fixed; - int all_match; - int none_match; +#define GREP_ALL_MATCH 1 +#define GREP_NONE_MATCH 2 + int all_or_none; int debug; #define GREP_BINARY_DEFAULT0 #define GREP_BINARY_NOMATCH1 diff --git a/revision.c b/revision.c index d43779e..b955848 100644 --- a/revision.c +++ b/revision.c @@ -2010,9 +2010,9 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg } else if (!strcmp(arg, "--perl-regexp")) { grep_set_pattern_type_option(GREP_PATTERN_TYPE_PCRE, &revs->grep_filter); } else if (!strcmp(arg, "--all-match")) { - revs->grep_filter.all_match = 1; + revs->grep_filter.all_or_none = GREP_ALL_MATCH; } else if (!strcmp(arg, "--none-match")) { - revs->grep_filter.none_match = 1; + revs->grep_filter.all_or_none = GREP_NONE_MATCH; } else if ((argcount = parse_long_opt("encoding", argv, &optarg))) { if (strcmp(optarg, "none")) git_log_output_encoding = xstrdup(optarg); @@ -2335,8 +2335,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s die("cannot combine --walk-reflogs with --graph"); if (!revs->reflog_info && revs->grep_filter.use_reflog_filter) die("cannot use --grep-reflog without --walk-reflogs"); - if (revs->grep_filter.all_match && revs->grep_filter.none_match) - die("cannot combine --all-match with --none-match"); return left; } -- To unsubscribe from this list: send the line "unsub