{"thread":{"id":"8917","subject":"[PATCH] Make attributes \"-diff\" and \"diff\" work as advertized","startedAt":"2007-07-07T16:53:17Z","lastAt":"2007-07-08T00:59:20Z","messageCount":3,"participants":["Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"46697","messageId":"Pine.LNX.4.64.0707071749220.4093@racer.site","threadId":"8917","inReplyTo":null,"subject":"[PATCH] Make attributes \"-diff\" and \"diff\" work as advertized","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-07T16:53:17Z","receivedAt":"2007-07-07T16:53:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIn Documentation/gitattributes.txt, it says that you can suppress\ndiffs by setting the attribute \"-diff\", and you can override the\nbinary detection with the attribute \"diff\".\n\nMake it work.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tEvidently, this is on top of \"next\" + the funcname patches, since\n\tI realized it only when working on top of them.\n\n\tHowever, it should also apply cleanly to \"master\", and even \n\t\"maint\". (The only reason I'm not doing it right now, is that I am \n\tin the middle of preparing a patch pair, and doing this patch was \n\teasy enough with rebase -i.)\n\n diff.c                   |   15 +++++++--------\n t/t4020-diff-external.sh |   12 ++++++++++++\n 2 files changed, 19 insertions(+), 8 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 21e61af..e92db5c 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1170,13 +1170,19 @@ static void diff_filespec_check_attr(struct diff_filespec *one)\n \tone->is_binary = 0;\n \tone->funcname_pattern_ident = NULL;\n \n+\tif (!one->data && DIFF_FILE_VALID(one))\n+\t\tdiff_populate_filespec(one, 0);\n+\n+\tif (one->data)\n+\t\tone->is_binary = buffer_is_binary(one->data, one->size);\n+\n \tif (!git_checkattr(one->path, 1, &attr_diff_check)) {\n \t\tconst char *value;\n \n \t\t/* binaryness */\n \t\tvalue = attr_diff_check.value;\n \t\tif (ATTR_TRUE(value))\n-\t\t\t;\n+\t\t\tone->is_binary = 0;\n \t\telse if (ATTR_FALSE(value))\n \t\t\tone->is_binary = 1;\n \n@@ -1186,13 +1192,6 @@ static void diff_filespec_check_attr(struct diff_filespec *one)\n \t\telse\n \t\t\tone->funcname_pattern_ident = value;\n \t}\n-\n-\tif (!one->data && DIFF_FILE_VALID(one))\n-\t\tdiff_populate_filespec(one, 0);\n-\n-\tif (one->data)\n-\t\tone->is_binary = buffer_is_binary(one->data, one->size);\n-\n }\n \n int diff_filespec_is_binary(struct diff_filespec *one)\ndiff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh\nindex f0045cd..ed3bd5b 100755\n--- a/t/t4020-diff-external.sh\n+++ b/t/t4020-diff-external.sh\n@@ -94,4 +94,16 @@ test_expect_success 'diff attribute should apply only to diff' '\n \n '\n \n+test_expect_success 'no diff with -diff' '\n+\techo >.gitattributes \"file -diff\" &&\n+\tgit diff | grep Binary\n+'\n+\n+echo NULZbetweenZwords | tr Z '\\0' > file\n+\n+test_expect_success 'force diff with \"diff\"' '\n+\techo >.gitattributes \"file diff\" &&\n+\tgit diff | grep -a second\n+'\n+\n test_done\n-- \n1.5.3.rc0.2712.g125b7f\n"},{"id":"46720","messageId":"7vy7hr69ky.fsf@assigned-by-dhcp.cox.net","threadId":"8917","inReplyTo":"Pine.LNX.4.64.0707071749220.4093@racer.site","subject":"Re: [PATCH] Make attributes \"-diff\" and \"diff\" work as advertized","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-08T00:08:29Z","receivedAt":"2007-07-08T00:08:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:\n\n> diff --git a/diff.c b/diff.c\n> index 21e61af..e92db5c 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1170,13 +1170,19 @@ static void diff_filespec_check_attr(struct diff_filespec *one)\n>  \tone->is_binary = 0;\n>  \tone->funcname_pattern_ident = NULL;\n>  \n> +\tif (!one->data && DIFF_FILE_VALID(one))\n> +\t\tdiff_populate_filespec(one, 0);\n> +\n> +\tif (one->data)\n> +\t\tone->is_binary = buffer_is_binary(one->data, one->size);\n> +\n>  \tif (!git_checkattr(one->path, 1, &attr_diff_check)) {\n>  \t\tconst char *value;\n>  \n>  \t\t/* binaryness */\n>  \t\tvalue = attr_diff_check.value;\n>  \t\tif (ATTR_TRUE(value))\n> -\t\t\t;\n> +\t\t\tone->is_binary = 0;\n\nI wanted to make sure we do not have to read data and run\nbuffer_is_binary() when attribute says we do not have to.  I\nwonder why moving the code around makes the difference.\n\n>  \t\telse if (ATTR_FALSE(value))\n>  \t\t\tone->is_binary = 1;\n>  \n> @@ -1186,13 +1192,6 @@ static void diff_filespec_check_attr(struct diff_filespec *one)\n>  \t\telse\n>  \t\t\tone->funcname_pattern_ident = value;\n>  \t}\n> -\n> -\tif (!one->data && DIFF_FILE_VALID(one))\n> -\t\tdiff_populate_filespec(one, 0);\n> -\n> -\tif (one->data)\n> -\t\tone->is_binary = buffer_is_binary(one->data, one->size);\n> -\n>  }\n\nAh, I see, because these are done unconditionally.  That was\nsilly of me.\n"},{"id":"46723","messageId":"Pine.LNX.4.64.0707080153220.4093@racer.site","threadId":"8917","inReplyTo":"7vy7hr69ky.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Make attributes \"-diff\" and \"diff\" work as advertized","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-08T00:59:20Z","receivedAt":"2007-07-08T00:59:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nwell, it was a quick fix. I had more on my wishlist for today, and wanted \nto be done with it quickly. The proper fix would be something like this, I \nguess (which should not be applied without the changes to t4020, of \ncourse):\n\n diff.c |   18 +++++++++++-------\n 1 files changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex 04e7e91..e47921f 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1113,13 +1113,13 @@ static void setup_diff_attr_check(struct git_attr_check *check)\n \n static void diff_filespec_check_attr(struct diff_filespec *one)\n {\n+\tint is_binary = -1;\n \tstruct git_attr_check attr_diff_check[2];\n \n \tif (one->checked_attr)\n \t\treturn;\n \n \tsetup_diff_attr_check(attr_diff_check);\n-\tone->is_binary = 0;\n \tone->hunk_header_ident = NULL;\n \n \tif (!git_checkattr(one->path, ARRAY_SIZE(attr_diff_check), attr_diff_check)) {\n@@ -1128,9 +1128,9 @@ static void diff_filespec_check_attr(struct diff_filespec *one)\n \t\t/* binaryness */\n \t\tvalue = attr_diff_check[0].value;\n \t\tif (ATTR_TRUE(value))\n-\t\t\t;\n+\t\t\tis_binary = 0;\n \t\telse if (ATTR_FALSE(value))\n-\t\t\tone->is_binary = 1;\n+\t\t\tis_binary = 1;\n \n \t\t/* hunk header ident */\n \t\tvalue = attr_diff_check[1].value;\n@@ -1140,11 +1140,15 @@ static void diff_filespec_check_attr(struct diff_filespec *one)\n \t\t\tone->hunk_header_ident = value;\n \t}\n \n-\tif (!one->data && DIFF_FILE_VALID(one))\n-\t\tdiff_populate_filespec(one, 0);\n+\tif (is_binary < 0) {\n+\t\tif (!one->data && DIFF_FILE_VALID(one))\n+\t\t\tdiff_populate_filespec(one, 0);\n \n-\tif (one->data)\n-\t\tone->is_binary = buffer_is_binary(one->data, one->size);\n+\t\tif (one->data)\n+\t\t\tone->is_binary =\n+\t\t\t\tbuffer_is_binary(one->data, one->size);\n+\t} else\n+\t\tone->is_binary = is_binary;\n \n }\n \n"}]}