{"thread":{"id":"28091","subject":"[RFC] grep should detect binary files like diff","startedAt":"2011-08-13T23:51:32Z","lastAt":"2011-08-13T23:51:32Z","messageCount":1,"participants":["Conrad Irwin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"173469","messageId":"1313279492-7241-1-git-send-email-conrad.irwin@gmail.com","threadId":"28091","inReplyTo":null,"subject":"[RFC] grep should detect binary files like diff","fromName":"Conrad Irwin","fromEmail":"conrad.irwin@gmail.com","sentAt":"2011-08-13T23:51:32Z","receivedAt":"2011-08-13T23:51:32Z","isPatch":false,"sender":{"key":"conrad.irwin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94272?v=4"},"body":"Hi all,\n\nThe problem I have is a large number of fixture files, in text-based\nformats, that pollute the output of various git commands. I can mitigate\nthis in git-diff using gitattributes mechanism, but there's no similar\nconfiguration for git-grep.\n\nThere are two issues with the naive approach to implementing this that\nI've attached below. Firstly the configuration variable is namespaced\nunder \"diff\", and secondly the attributes mechanism is not thread-safe.\n\nI'm inclined to think that the namespacing issue is not too significant;\nit would just require some documentation. If it is an issue then maybe a\nnew \"grep\" attribute could be created. Are there any places where you\nmight want git-grep and git-diff to treat different sets of files as\nbinary?\n\nThe thread-safety of the attributes mechanism is a much bigger problem,\nand is the only reason I made the behaviour depend on a configuration\noption below. You can either have a multi-threaded grep, or a grep that\ndetects binary files properly :(. I'm not sure how to even start\nresolving an issue like that, though I'm happy to accept pointers. Does\nanyone, Junio?, know what it would take to fix?\n\nConrad\n\n---\n Documentation/config.txt        |    5 ++++\n Documentation/git-grep.txt      |    5 ++++\n Documentation/gitattributes.txt |    3 ++\n builtin/grep.c                  |    5 ++++\n grep.c                          |   48 +++++++++++++++++++++-----------------\n grep.h                          |    1 +\n t/t7008-grep-binary.sh          |   18 ++++++++++++++\n 7 files changed, 63 insertions(+), 22 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 0658ffb..b7d65b3 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1070,6 +1070,11 @@ grep.lineNumber::\n grep.extendedRegexp::\n \tIf set to true, enable '--extended-regexp' option by default.\n \n+grep.binaryFiles::\n+\tIf set to true, linkgit:git-grep[1] will treat files as binary under\n+\tthe same circumstances as linkgit:git-diff[1]. See the\n+\t\"Marking files as binary\" section of linkgit:gitattributes[5].\n+\n gui.commitmsgwidth::\n \tDefines how wide the commit message window is in the\n \tlinkgit:git-gui[1]. \"75\" is the default.\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex e44a498..fd9ebc4 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -41,6 +41,11 @@ grep.lineNumber::\n grep.extendedRegexp::\n \tIf set to true, enable '--extended-regexp' option by default.\n \n+grep.binaryFiles::\n+\tIf set to true, linkgit:git-grep[1] will treat files as binary under\n+\tthe same circumstances as linkgit:git-diff[1]. See the\n+\t\"Marking files as binary\" section of linkgit:gitattributes[5].\n+\n \n OPTIONS\n -------\ndiff --git a/Documentation/gitattributes.txt b/Documentation/gitattributes.txt\nindex 2bbe76b..180aa2f 100644\n--- a/Documentation/gitattributes.txt\n+++ b/Documentation/gitattributes.txt\n@@ -644,6 +644,9 @@ attribute in the `.gitattributes` file:\n \n This will cause git to generate `Binary files differ` (or a binary\n patch, if binary patches are enabled) instead of a regular diff.\n+If the grep.binaryFiles configuration variable is set, linkgit:git-grep[1]\n+will also treat such files as binary, by default printing\n+`Binary file matches` instead of the matching line.\n \n However, one may also want to specify other diff driver attributes. For\n example, you might want to use `textconv` to convert postscript files to\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 1851797..e9d9003 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -324,6 +324,11 @@ static int grep_config(const char *var, const char *value, void *cb)\n \t\treturn 0;\n \t}\n \n+\tif (!strcmp(var, \"grep.binaryfiles\")) {\n+\t\topt->userdiff_binary = git_config_bool(var, value);\n+\t\treturn 0;\n+\t}\n+\n \tif (!strcmp(var, \"color.grep\"))\n \t\topt->color = git_config_colorbool(var, value, -1);\n \telse if (!strcmp(var, \"color.grep.context\"))\ndiff --git a/grep.c b/grep.c\nindex 26e8d8e..84063eb 100644\n--- a/grep.c\n+++ b/grep.c\n@@ -924,8 +924,8 @@ int grep_threads_ok(const struct grep_opt *opt)\n \t * machinery in grep_buffer_1. The attribute code is not\n \t * thread safe, so we disable the use of threads.\n \t */\n-\tif (opt->funcname && !opt->unmatch_name_only && !opt->status_only &&\n-\t    !opt->name_only)\n+\tif ((opt->funcname && !opt->unmatch_name_only && !opt->status_only &&\n+\t    !opt->name_only) || opt->userdiff_binary)\n \t\treturn 0;\n \n \treturn 1;\n@@ -947,6 +947,7 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,\n \tunsigned count = 0;\n \tint try_lookahead = 0;\n \tint show_function = 0;\n+\tint load_userdiff_func;\n \tenum grep_context ctx = GREP_CONTEXT_HEAD;\n \txdemitconf_t xecfg;\n \n@@ -968,31 +969,34 @@ static int grep_buffer_1(struct grep_opt *opt, const char *name,\n \t}\n \topt->last_shown = 0;\n \n-\tswitch (opt->binary) {\n-\tcase GREP_BINARY_DEFAULT:\n-\t\tif (buffer_is_binary(buf, size))\n-\t\t\tbinary_match_only = 1;\n-\t\tbreak;\n-\tcase GREP_BINARY_NOMATCH:\n-\t\tif (buffer_is_binary(buf, size))\n-\t\t\treturn 0; /* Assume unmatch */\n-\t\tbreak;\n-\tcase GREP_BINARY_TEXT:\n-\t\tbreak;\n-\tdefault:\n-\t\tdie(\"bug: unknown binary handling mode\");\n-\t}\n+\tload_userdiff_func = opt->funcname && !opt->unmatch_name_only && !opt->status_only &&\n+\t    !opt->name_only && !collect_hits;\n \n \tmemset(&xecfg, 0, sizeof(xecfg));\n-\tif (opt->funcname && !opt->unmatch_name_only && !opt->status_only &&\n-\t    !opt->name_only && !binary_match_only && !collect_hits) {\n+\tif (opt->userdiff_binary || load_userdiff_func) {\n+\t\t/* we have to be careful not to call this if we're using threads */\n \t\tstruct userdiff_driver *drv = userdiff_find_by_path(name);\n-\t\tif (drv && drv->funcname.pattern) {\n-\t\t\tconst struct userdiff_funcname *pe = &drv->funcname;\n-\t\t\txdiff_set_find_func(&xecfg, pe->pattern, pe->cflags);\n-\t\t\topt->priv = &xecfg;\n+\n+\t\tif (opt->userdiff_binary && drv && drv->binary != -1)\n+\t\t\tbinary_match_only = drv->binary;\n+\t\telse if (opt->binary != GREP_BINARY_TEXT)\n+\t\t\tbinary_match_only = buffer_is_binary(buf, size);\n+\n+\t\tif (load_userdiff_func && !binary_match_only) {\n+\t\t\tif (drv && drv->funcname.pattern) {\n+\t\t\t\tconst struct userdiff_funcname *pe = &drv->funcname;\n+\t\t\t\txdiff_set_find_func(&xecfg, pe->pattern, pe->cflags);\n+\t\t\t\topt->priv = &xecfg;\n+\t\t\t}\n \t\t}\n+\t} else if (opt->binary != GREP_BINARY_TEXT) {\n+\t\tbinary_match_only = buffer_is_binary(buf, size);\n+\t}\n+\n+\tif (binary_match_only && opt->binary == GREP_BINARY_NOMATCH) {\n+\t\treturn 0;\n \t}\n+\n \ttry_lookahead = should_lookahead(opt);\n \n \twhile (left) {\ndiff --git a/grep.h b/grep.h\nindex ae50c45..303cb78 100644\n--- a/grep.h\n+++ b/grep.h\n@@ -90,6 +90,7 @@ struct grep_opt {\n #define GREP_BINARY_NOMATCH\t1\n #define GREP_BINARY_TEXT\t2\n \tint binary;\n+\tint userdiff_binary;\n \tint extended;\n \tint pcre;\n \tint relative;\ndiff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh\nindex e058d18..a88503b 100755\n--- a/t/t7008-grep-binary.sh\n+++ b/t/t7008-grep-binary.sh\n@@ -99,4 +99,22 @@ test_expect_failure 'git grep y<NUL>x a' \"\n \ttest_must_fail git grep -f f a\n \"\n \n+test_expect_success 'git -c grep.binaryFiles=1 grep ina a' \"\n+\techo 'a diff' > .gitattributes &&\n+\tprintf 'binaryQfile' | q_to_nul >a &&\n+\techo 'a:binaryQfile' | q_to_nul >expect &&\n+\tgit -c grep.binaryFiles=1 grep ina a > actual &&\n+\trm .gitattributes &&\n+\ttest_cmp expect actual\n+\"\n+test_expect_success 'git -c grep.binaryFiles=1 grep tex t' \"\n+\techo 'text' > t &&\n+\tgit add t &&\n+\techo 't -diff' > .gitattributes &&\n+\techo Binary file t matches >expect &&\n+\tgit -c grep.binaryFiles=1 grep tex t >actual &&\n+\trm .gitattributes &&\n+\ttest_cmp expect actual\n+\"\n+\n test_done\n-- \n1.7.6.409.ge7a85\n"}]}