Re: textconv not invoked when viewing merge commit

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> We just dump the binary goo all over the terminal. So I think the whole
>> combined-diff code path needs to learn how to handle binaries properly.
>
> How would you show multi-way diffs for binary files?
>
> It would probably be sufficient to say "binary files differ" at the
> beginning of the patch-combining codepath of the combined diff, which
> would at least keep the --raw -c/--cc output working.

In other words, I suspect that the only places you need to touch in the
existing codepath would be these places.

diff --git a/combine-diff.c b/combine-diff.c
index 655fa89..9c96f1f 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -201,7 +201,7 @@ static void consume_line(void *state_, char *line, unsigned long len)
 	}
 }
 
-static void combine_diff(const unsigned char *parent, unsigned int mode,
+static int combine_diff(const unsigned char *parent, unsigned int mode,
 			 mmfile_t *result_file,
 			 struct sline *sline, unsigned int cnt, int n,
 			 int num_parent, int result_deleted)
@@ -215,10 +215,17 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,
 	unsigned long sz;
 
 	if (result_deleted)
-		return; /* result deleted */
+		return 0; /* result deleted */
 
 	parent_file.ptr = grab_blob(parent, mode, &sz);
 	parent_file.size = sz;
+
+	if (path has textconv) {
+		parent_file.{ptr,size} = textconv of parent_file;
+	} else if (path is binary) {
+		return -1;
+	}
+
 	memset(&xpp, 0, sizeof(xpp));
 	xpp.flags = 0;
 	memset(&xecfg, 0, sizeof(xecfg));
@@ -255,6 +262,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,
 			p_lno++; /* no '+' means parent had it */
 	}
 	sline[lno].p_lno[n] = p_lno; /* trailer */
+	return 0;
 }
 
 static unsigned long context = 3;
@@ -777,6 +785,12 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 			close(fd);
 	}
 
+	if (path has textconv) {
+		result, result_size = textconv of result;
+	} else if (path is binary) {
+		goto exit_binary;
+	}
+
 	for (cnt = 0, cp = result; cp < result + result_size; cp++) {
 		if (*cp == '\n')
 			cnt++;
@@ -820,11 +834,13 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 				break;
 			}
 		}
-		if (i <= j)
-			combine_diff(elem->parent[i].sha1,
-				     elem->parent[i].mode,
-				     &result_file, sline,
-				     cnt, i, num_parent, result_deleted);
+		if (i <= j) {
+			if (combine_diff(elem->parent[i].sha1,
+					 elem->parent[i].mode,
+					 &result_file, sline,
+					 cnt, i, num_parent, result_deleted))
+				goto exit_binary;
+		}
 		if (elem->parent[i].mode != elem->mode)
 			mode_differs = 1;
 	}
@@ -892,6 +908,8 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 		dump_sline(sline, cnt, num_parent,
 			   DIFF_OPT_TST(opt, COLOR_DIFF), result_deleted);
 	}
+
+free_exit:
 	free(result);
 
 	for (lno = 0; lno < cnt; lno++) {
@@ -906,6 +924,11 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 	}
 	free(sline[0].p_lno);
 	free(sline);
+	return;
+
+exit_binary:
+	printf("path '%s' is binary\n", elem->path);
+	goto free_exit;
 }
 
 #define COLONS "::::::::::::::::::::::::::::::::"