Junio C Hamano <gitster <at> pobox.com> writes:
> Please reword this to describe the problem being solved first (why it
> needs to be solved, in what situation you cannot live without the
> feature), and then describe the approach you used to solve it.
>
Done — I also removed the extraneous braces from the patch.
>
> in order to avoid uselessly spewing garbage at you while reminding you
> that the command is not showing the whole truth and you _might_ want to
> look into the elided file if you wanted to with "grep -a world hello.o".
> Compared to this, it feels that the result your patch goes too far and
> hides the truth a bit too much to my taste. Maybe it is just me.
I used to use this approach, hooking into the "diff" attribute directly to mark
a file as binary, however that was clearly a hack. When developing this patch I
went through a few iterations, one in which -grep meant "treat this file as
binary", however explaining that in the man page is subtle and ugly: "HINT: you
might want to set a file as binary because you don't want to see results from it
when grepping". It's much more obvious to have -grep mean "don't show me
results".
A nicer alternative could be to allow "grep=binary" (and for completeness
"grep=text") in addition to "-grep". Then people who want to see matches but not
the contents of the matches can tell grep that their files are "binary". It
would also make sense to add "grep=binary" to the binary macro-attribute. We
could even extend the system arbitrarily to allow something like the textconv
attributes of git-diff... one step at a time is probably better though.
> Perhaps you, or all participants of your particular project, usually do
> not want to see any grep hits from minified.js, but you may still want to
> be able to say "I want to dig deeper and make sure I have copyright
> strings in all files", for example. It is unclear how you envision to
> support such a use case building on top of this patch.
I think that it would be very reasonable to add a flag to grep to tell it to
ignore the attribute temporarily (like --no-textconv on git-diff) and update the
"-a" shorthand to imply "--text --no-exclude-attribute".
Yours,
Conrad
---8<---
Git grep is used by developers to search the code stored in their repositories,
however it can give noisy results when the repository contains files that are
not of direct interest to development. Examples of such files include test
fixtures, dlls, or minified source code.
To help these developers search efficiently, git grep will now use the
gitattributes mechanism to ignore all files with an unset "grep" attribute.
Another approach considered was to allow an --exclude flag to grep at runtime,
however this is more clunky to use when the set of files to be excluded is
fixed.
Signed-off-by: Conrad Irwin <conrad.irwin@xxxxxxxxx>
---
Documentation/git-grep.txt | 7 +++++++
Documentation/gitattributes.txt | 9 +++++++++
builtin/grep.c | 6 ++++++
grep.c | 21 +++++++++++++++++++++
grep.h | 1 +
t/t7810-grep.sh | 30 ++++++++++++++++++++++++++++++
6 files changed, 74 insertions(+), 0 deletions(-)
diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index 6a8b1e3..7c74165 100644
--- a/Documentation/git-grep.txt
+++ b/Documentation/git-grep.txt
@@ -242,6 +242,13 @@ OPTIONS
If given, limit the search to paths matching at least one pattern.
Both leading paths match and glob(7) patterns are supported.
+ATTRIBUTES
+----------
+
+grep::
+ If the grep attribute is unset on a file using the linkgit:gitattributes[1]
+ mechanism, then that file will not be searched.
+
Examples
--------
diff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt
index a85b187..3ffcec7 100644
--- a/Documentation/gitattributes.txt
+++ b/Documentation/gitattributes.txt
@@ -869,6 +869,15 @@ If this attribute is not set or has an invalid value, the value of the
`gui.encoding` configuration variable is used instead
(See linkgit:git-config[1]).
+Configuring files to search
+~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+`grep`
+^^^^^^
+
+If the attribute `grep` is unset for a file then linkgit:git-grep[1]
+will ignore that file while searching for matches.
+
USING MACRO ATTRIBUTES
----------------------
diff --git a/builtin/grep.c b/builtin/grep.c
index 9ce064a..a7817fe 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -398,6 +398,9 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
struct strbuf pathbuf = STRBUF_INIT;
char *name;
+ if (!should_grep_path(opt, filename + tree_name_len))
+ return 0;
+
if (opt->relative && opt->prefix_length) {
quote_path_relative(filename + tree_name_len, -1, &pathbuf,
opt->prefix);
@@ -464,6 +467,9 @@ static int grep_file(struct grep_opt *opt, const char *filename)
struct strbuf buf = STRBUF_INIT;
char *name;
+ if (!should_grep_path(opt, filename))
+ return 0;
+
if (opt->relative && opt->prefix_length)
quote_path_relative(filename, -1, &buf, opt->prefix);
else
diff --git a/grep.c b/grep.c
index 486230b..e948576 100644
--- a/grep.c
+++ b/grep.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "grep.h"
+#include "attr.h"
#include "userdiff.h"
#include "xdiff-interface.h"
@@ -829,6 +830,26 @@ static inline void grep_attr_unlock(struct grep_opt *opt)
#define grep_attr_unlock(opt)
#endif
+static struct git_attr_check attr_check[1];
+static void setup_attr_check(void)
+{
+ if (attr_check[0].attr)
+ return; /* already done */
+ attr_check[0].attr = git_attr("grep");
+}
+int should_grep_path(struct grep_opt *opt, const char *name) {
+ int ret = 1;
+
+ grep_attr_lock(opt);
+ setup_attr_check();
+ git_check_attr(name, 1, attr_check);
+ if (ATTR_FALSE(attr_check[0].value))
+ ret = 0;
+ grep_attr_unlock(opt);
+
+ return ret;
+}
+
static int match_funcname(struct grep_opt *opt, const char *name, char *bol, char *eol)
{
xdemitconf_t *xecfg = opt->priv;
diff --git a/grep.h b/grep.h
index fb205f3..266002d 100644
--- a/grep.h
+++ b/grep.h
@@ -129,6 +129,7 @@ extern void append_header_grep_pattern(struct grep_opt *, enum grep_header_field
extern void compile_grep_patterns(struct grep_opt *opt);
extern void free_grep_patterns(struct grep_opt *opt);
extern int grep_buffer(struct grep_opt *opt, const char *name, char *buf, unsigned long size);
+extern int should_grep_path(struct grep_opt *opt, const char *name);
extern struct grep_opt *grep_opt_dup(const struct grep_opt *opt);
extern int grep_threads_ok(const struct grep_opt *opt);
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index 7ba5b16..c991518 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -871,4 +871,34 @@ test_expect_success 'mimic ack-grep --group' '
test_cmp expected actual
'
+test_expect_success 'with -grep attribute' '
+ echo "hello.c -grep" >.gitattributes &&
+ test_must_fail git grep printf &&
+ rm .gitattributes
+'
+
+test_expect_success 'with -grep attribute on specific file' '
+ echo "hello.c -grep" >.gitattributes &&
+ test_must_fail git grep printf hello.c &&
+ rm .gitattributes
+'
+
+test_expect_success 'with -grep attribute on specific revision' '
+ echo "hello.c -grep" >.gitattributes &&
+ test_must_fail git grep printf HEAD &&
+ rm .gitattributes
+'
+
+test_expect_success 'with -grep attribute on specific file/revision' '
+ echo "hello.c -grep" >.gitattributes &&
+ test_must_fail git grep printf HEAD hello.c &&
+ rm .gitattributes
+'
+
+test_expect_failure 'with -grep attribute on specific tree' '
+ echo "hello.c -grep" >.gitattributes &&
+ test_must_fail git grep printf HEAD:hello.c &&
+ rm .gitattributes
+'
+
test_done
--
1.7.9.rc2.1.g1fdd3
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html