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
[PATCH] git-log: added --none-match option
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
---
Documentation/rev-list-options.txt | 4
contrib/completion/git-completion.bash | 2 +-
grep.c | 2 ++
grep.h | 1 +
revision.c | 4
5 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/Documentation/rev-list-options.txt
b/Documentation/rev-list-options.txt
index afccfdc..08e4ed8 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.
+--none-match::
+ Limit the commits output to ones that do not match any of the
+ given `--grep`, instead of ones that match at least one.
+
-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 23988ec..b0720e9 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1425,7 +1425,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 --none-match
"
__git_log_pretty_formats="oneline short medium full fuller email raw format:"
diff --git a/grep.c b/grep.c
index 6e085f8..eadf8d9 100644
--- a/grep.c
+++ b/grep.c
@@ -1622,6 +1622,8 @@ 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".
diff --git a/grep.h b/grep.h
index 95f197a..8e50c95 100644
--- a/grep.h
+++ b/grep.h
@@ -102,6 +102,7 @@ struct grep_opt {
int word_regexp;
int fixed;
int all_match;
+ int none_match;
int debug;
#define GREP_BINARY_DEFAULT0
#define GREP_BINARY_NOMATCH1
diff --git a/revision.c b/revision.c
index 75dda92..d43779e 100644
--- a/revision.c
+++ b/revision.c
@@ -2011,6 +2011,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, "--none-match")) {
+ revs->grep_filter.none_match = 1;
} else if ((argcount = parse_long_opt("encoding", argv, &optarg))) {
if (strcmp(optarg, "none"))
git_log_output_encoding = xstrdup(optarg);
@@ -2333,6 +2335,8 @@ 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;
}
--
2.0.5
--
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
