threads / patch / 8917

patchMake attributes "-diff" and "diff" work as advertized

Subject: [PATCH] Make attributes "-diff" and "diff" work as advertized

## tl;dr

3 messages between Jul 7, 2007 and Jul 8, 2007. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Johannes Schindelin· Jul 7, 2007, 16:53 UTC · lore

In Documentation/gitattributes.txt, it says that you can suppress diffs by setting the attribute "-diff", and you can override the binary detection with the attribute "diff".

Make it work.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Evidently, this is on top of "next" + the funcname patches, since
	I realized it only when working on top of them.

	However, it should also apply cleanly to "master", and even 
	"maint". (The only reason I'm not doing it right now, is that I am 
	in the middle of preparing a patch pair, and doing this patch was 
	easy enough with rebase -i.)

 diff.c                   |   15 +++++++--------
 t/t4020-diff-external.sh |   12 ++++++++++++
 2 files changed, 19 insertions(+), 8 deletions(-)

Show changes to 2 files +19 −8

diff.c, t/t4020-diff-external.sh

diff --git a/diff.c b/diff.c
index 21e61af..e92db5c 100644
--- a/diff.c
+++ b/diff.c
@@ -1170,13 +1170,19 @@ static void diff_filespec_check_attr(struct diff_filespec *one)
 	one->is_binary = 0;
 	one->funcname_pattern_ident = NULL;
 
+	if (!one->data && DIFF_FILE_VALID(one))
+		diff_populate_filespec(one, 0);
+
+	if (one->data)
+		one->is_binary = buffer_is_binary(one->data, one->size);
+
 	if (!git_checkattr(one->path, 1, &attr_diff_check)) {
 		const char *value;
 
 		/* binaryness */
 		value = attr_diff_check.value;
 		if (ATTR_TRUE(value))
-			;
+			one->is_binary = 0;
 		else if (ATTR_FALSE(value))
 			one->is_binary = 1;
 
@@ -1186,13 +1192,6 @@ static void diff_filespec_check_attr(struct diff_filespec *one)
 		else
 			one->funcname_pattern_ident = value;
 	}
-
-	if (!one->data && DIFF_FILE_VALID(one))
-		diff_populate_filespec(one, 0);
-
-	if (one->data)
-		one->is_binary = buffer_is_binary(one->data, one->size);
-
 }
 
 int diff_filespec_is_binary(struct diff_filespec *one)
diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh
index f0045cd..ed3bd5b 100755
--- a/t/t4020-diff-external.sh
+++ b/t/t4020-diff-external.sh
@@ -94,4 +94,16 @@ test_expect_success 'diff attribute should apply only to diff' '
 
 '
 
+test_expect_success 'no diff with -diff' '
+	echo >.gitattributes "file -diff" &&
+	git diff | grep Binary
+'
+
+echo NULZbetweenZwords | tr Z '\0' > file
+
+test_expect_success 'force diff with "diff"' '
+	echo >.gitattributes "file diff" &&
+	git diff | grep -a second
+'
+
 test_done

-- 
1.5.3.rc0.2712.g125b7f

Junio C Hamano· Jul 8, 2007, 00:08 UTC · re: Johannes Schindelin · lore

Re: [PATCH] Make attributes "-diff" and "diff" work as advertized

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 22 quoted lines

> diff --git a/diff.c b/diff.c
> index 21e61af..e92db5c 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -1170,13 +1170,19 @@ static void diff_filespec_check_attr(struct diff_filespec *one)
>  	one->is_binary = 0;
>  	one->funcname_pattern_ident = NULL;
>  
> +	if (!one->data && DIFF_FILE_VALID(one))
> +		diff_populate_filespec(one, 0);
> +
> +	if (one->data)
> +		one->is_binary = buffer_is_binary(one->data, one->size);
> +
>  	if (!git_checkattr(one->path, 1, &attr_diff_check)) {
>  		const char *value;
>  
>  		/* binaryness */
>  		value = attr_diff_check.value;
>  		if (ATTR_TRUE(value))
> -			;
> +			one->is_binary = 0;

I wanted to make sure we do not have to read data and run buffer_is_binary() when attribute says we do not have to. I wonder why moving the code around makes the difference.

Show 15 quoted lines

>  		else if (ATTR_FALSE(value))
>  			one->is_binary = 1;
>  
> @@ -1186,13 +1192,6 @@ static void diff_filespec_check_attr(struct diff_filespec *one)
>  		else
>  			one->funcname_pattern_ident = value;
>  	}
> -
> -	if (!one->data && DIFF_FILE_VALID(one))
> -		diff_populate_filespec(one, 0);
> -
> -	if (one->data)
> -		one->is_binary = buffer_is_binary(one->data, one->size);
> -
>  }

Ah, I see, because these are done unconditionally. That was silly of me.

Johannes Schindelin· Jul 8, 2007, 00:59 UTC · re: Junio C Hamano · lore

Re: [PATCH] Make attributes "-diff" and "diff" work as advertized

Hi,

well, it was a quick fix. I had more on my wishlist for today, and wanted to be done with it quickly. The proper fix would be something like this, I guess (which should not be applied without the changes to t4020, of course):

 diff.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

Show changes to diff.c +11 −7

diff --git a/diff.c b/diff.c
index 04e7e91..e47921f 100644
--- a/diff.c
+++ b/diff.c
@@ -1113,13 +1113,13 @@ static void setup_diff_attr_check(struct git_attr_check *check)
 
 static void diff_filespec_check_attr(struct diff_filespec *one)
 {
+	int is_binary = -1;
 	struct git_attr_check attr_diff_check[2];
 
 	if (one->checked_attr)
 		return;
 
 	setup_diff_attr_check(attr_diff_check);
-	one->is_binary = 0;
 	one->hunk_header_ident = NULL;
 
 	if (!git_checkattr(one->path, ARRAY_SIZE(attr_diff_check), attr_diff_check)) {
@@ -1128,9 +1128,9 @@ static void diff_filespec_check_attr(struct diff_filespec *one)
 		/* binaryness */
 		value = attr_diff_check[0].value;
 		if (ATTR_TRUE(value))
-			;
+			is_binary = 0;
 		else if (ATTR_FALSE(value))
-			one->is_binary = 1;
+			is_binary = 1;
 
 		/* hunk header ident */
 		value = attr_diff_check[1].value;
@@ -1140,11 +1140,15 @@ static void diff_filespec_check_attr(struct diff_filespec *one)
 			one->hunk_header_ident = value;
 	}
 
-	if (!one->data && DIFF_FILE_VALID(one))
-		diff_populate_filespec(one, 0);
+	if (is_binary < 0) {
+		if (!one->data && DIFF_FILE_VALID(one))
+			diff_populate_filespec(one, 0);
 
-	if (one->data)
-		one->is_binary = buffer_is_binary(one->data, one->size);
+		if (one->data)
+			one->is_binary =
+				buffer_is_binary(one->data, one->size);
+	} else
+		one->is_binary = is_binary;
 
 }