git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Use a *real* built-in diff generator

From
Junio C Hamano <junkio@cox.net>
Date
Mar 25, 2006, 19:52 UTC
Message-ID
<7v7j6iwbdk.fsf@assigned-by-dhcp.cox.net>
In-Reply-To
<Pine.LNX.4.64.0603250742340.15714@g5.osdl.org>
Linus Torvalds <torvalds@osdl.org> writes:
Show 11 quoted lines
> On Sat, 25 Mar 2006, Linus Torvalds wrote:
>> 
>> I'll be taking a look at trying to fix it. 
>
> Actually, it ended up being easier than I expected it to be.
>
> This (on top of the previous patch) should fix it.
>...
> +	if (!size || rec[size-1] != '\n') {
> +		mb[2].ptr = "\n\\ No newline at end of file\n";
>...

Thanks. Here is what I cooked last night, which I rebased on top of your two patches, to:

 - Punt "binary" diff like GNU does;
 - Minimally support GIT_DIFF_OPTS to allow passing -u0;
 - Omit ---/+++ lines when we do not have any diff output;
 - Adjust the tests to "@@ -k,l +m,n @@" form.
 - Adjust the tests to lack of "diff -p".

The first two are hacks but I think they are good enough hacks for real life. The third one is a real fix. I agree with you that libxdiff form is more rational, and that is the justification for the fourth point.

-- >8 --
diff --git a/diff.c b/diff.c
index f6a1f5d..cd2ce0f 100644
--- a/diff.c
+++ b/diff.c
@@ -212,13 +212,34 @@
 	return 0;
 }
 
+struct emit_callback {
+	const char **label_path;
+};
+
 static int fn_out(void *priv, mmbuffer_t *mb, int nbuf)
 {
 	int i;
+	struct emit_callback *ecbdata = priv;
 
+	if (ecbdata->label_path[0]) {
+		printf("--- %s\n", ecbdata->label_path[0]);
+		printf("+++ %s\n", ecbdata->label_path[1]);
+		ecbdata->label_path[0] = ecbdata->label_path[1] = NULL;
+	}
 	for (i = 0; i < nbuf; i++)
 		if (!fwrite(mb[i].ptr, mb[i].size, 1, stdout))
 			return -1;
+	return 0;
+}
+
+#define FIRST_FEW_BYTES 8000
+static int mmfile_is_binary(mmfile_t *mf)
+{
+	long sz = mf->size;
+	if (FIRST_FEW_BYTES < sz)
+		sz = FIRST_FEW_BYTES;
+	if (memchr(mf->ptr, 0, sz))
+		return 1;
 	return 0;
 }
 
@@ -306,22 +327,32 @@
 	if (label_path[1][0] != '/')
 		label_path[1] = quote_two("b/", name_b);
 
-	printf("--- %s\n", label_path[0]);
-	printf("+++ %s\n", label_path[1]);
-
 	if (fill_mmfile(&mf1, temp[0].name) < 0 ||
 	    fill_mmfile(&mf2, temp[1].name) < 0)
 		die("unable to read files to diff");
 
-	/* Crazy xdl interfaces.. */
-	{
+	if (mmfile_is_binary(&mf1) || mmfile_is_binary(&mf2))
+		printf("Binary files %s and %s differ\n",
+		       label_path[0], label_path[1]);
+	else {
+		/* Crazy xdl interfaces.. */
+		const char *diffopts = getenv("GIT_DIFF_OPTS");
 		xpparam_t xpp;
 		xdemitconf_t xecfg;
 		xdemitcb_t ecb;
+		struct emit_callback ecbdata;
 
+		ecbdata.label_path = label_path;
 		xpp.flags = XDF_NEED_MINIMAL;
 		xecfg.ctxlen = 3;
+		if (!diffopts)
+			;
+		else if (!strncmp(diffopts, "--unified=", 10))
+			xecfg.ctxlen = strtoul(diffopts + 10, NULL, 10);
+		else if (!strncmp(diffopts, "-u", 2))
+			xecfg.ctxlen = strtoul(diffopts + 2, NULL, 10);
 		ecb.outf = fn_out;
+		ecb.priv = &ecbdata;
 		xdl_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
 	}
 
diff --git a/t/t1200-tutorial.sh b/t/t1200-tutorial.sh
index 1002413..5ac373f 100755
--- a/t/t1200-tutorial.sh
+++ b/t/t1200-tutorial.sh
@@ -22,7 +22,7 @@
 index 557db03..263414f 100644
 --- a/hello
 +++ b/hello
-@@ -1 +1,2 @@
+@@ -1,1 +1,2 @@
  Hello World
 +It's a new day for git
 EOF
@@ -60,14 +60,14 @@
 index 0000000..f24c74a
 --- /dev/null
 +++ b/example
-@@ -0,0 +1 @@
+@@ -0,0 +1,1 @@
 +Silly example
 diff --git a/hello b/hello
 new file mode 100644
 index 0000000..557db03
 --- /dev/null
 +++ b/hello
-@@ -0,0 +1 @@
+@@ -0,0 +1,1 @@
 +Hello World
 EOF
 
diff --git a/t/t4001-diff-rename.sh b/t/t4001-diff-rename.sh
index 2e3c20d..08c1131 100755
--- a/t/t4001-diff-rename.sh
+++ b/t/t4001-diff-rename.sh
@@ -49,7 +49,7 @@
 rename to path1
 --- a/path0
 +++ b/path1
-@@ -8,7 +8,7 @@ Line 7
+@@ -8,7 +8,7 @@
  Line 8
  Line 9
  Line 10
diff --git a/t/t4003-diff-rename-1.sh b/t/t4003-diff-rename-1.sh
index 2751970..af744fd 100755
--- a/t/t4003-diff-rename-1.sh
+++ b/t/t4003-diff-rename-1.sh
@@ -36,7 +36,7 @@
 copy to COPYING.1
 --- a/COPYING
 +++ b/COPYING.1
-@@ -6 +6 @@
+@@ -6,1 +6,1 @@
 - HOWEVER, in order to allow a migration to GPLv3 if that seems like
 + However, in order to allow a migration to GPLv3 if that seems like
 diff --git a/COPYING b/COPYING.2
@@ -44,13 +44,13 @@
 rename to COPYING.2
 --- a/COPYING
 +++ b/COPYING.2
-@@ -2 +2 @@
+@@ -2,1 +2,1 @@
 - Note that the only valid version of the GPL as far as this project
 + Note that the only valid version of the G.P.L as far as this project
-@@ -6 +6 @@
+@@ -6,1 +6,1 @@
 - HOWEVER, in order to allow a migration to GPLv3 if that seems like
 + HOWEVER, in order to allow a migration to G.P.Lv3 if that seems like
-@@ -12 +12 @@
+@@ -12,1 +12,1 @@
 -	This file is licensed under the GPL v2, or a later version
 +	This file is licensed under the G.P.L v2, or a later version
 EOF
@@ -74,13 +74,13 @@
 diff --git a/COPYING b/COPYING
 --- a/COPYING
 +++ b/COPYING
-@@ -2 +2 @@
+@@ -2,1 +2,1 @@
 - Note that the only valid version of the GPL as far as this project
 + Note that the only valid version of the G.P.L as far as this project
-@@ -6 +6 @@
+@@ -6,1 +6,1 @@
 - HOWEVER, in order to allow a migration to GPLv3 if that seems like
 + HOWEVER, in order to allow a migration to G.P.Lv3 if that seems like
-@@ -12 +12 @@
+@@ -12,1 +12,1 @@
 -	This file is licensed under the GPL v2, or a later version
 +	This file is licensed under the G.P.L v2, or a later version
 diff --git a/COPYING b/COPYING.1
@@ -88,7 +88,7 @@
 copy to COPYING.1
 --- a/COPYING
 +++ b/COPYING.1
-@@ -6 +6 @@
+@@ -6,1 +6,1 @@
 - HOWEVER, in order to allow a migration to GPLv3 if that seems like
 + However, in order to allow a migration to GPLv3 if that seems like
 EOF
@@ -116,7 +116,7 @@
 copy to COPYING.1
 --- a/COPYING
 +++ b/COPYING.1
-@@ -6 +6 @@
+@@ -6,1 +6,1 @@
 - HOWEVER, in order to allow a migration to GPLv3 if that seems like
 + However, in order to allow a migration to GPLv3 if that seems like
 EOF
diff --git a/t/t4004-diff-rename-symlink.sh b/t/t4004-diff-rename-symlink.sh
index a23aaa0..04f0147 100755
--- a/t/t4004-diff-rename-symlink.sh
+++ b/t/t4004-diff-rename-symlink.sh
@@ -40,7 +40,7 @@
 new file mode 120000
 --- /dev/null
 +++ b/bozbar
-@@ -0,0 +1 @@
+@@ -0,0 +1,1 @@
 +xzzzy
 \ No newline at end of file
 diff --git a/frotz b/nitfol
@@ -55,7 +55,7 @@
 deleted file mode 100644
 --- a/yomin
 +++ /dev/null
-@@ -1 +0,0 @@
+@@ -1,1 +0,0 @@
 -xyzzy
 \ No newline at end of file
 EOF
diff --git a/t/t4011-diff-symlink.sh b/t/t4011-diff-symlink.sh
index 379a831..36ac16d 100755
--- a/t/t4011-diff-symlink.sh
+++ b/t/t4011-diff-symlink.sh
@@ -15,7 +15,7 @@
 index 0000000..7c465af
 --- /dev/null
 +++ b/frotz
-@@ -0,0 +1 @@
+@@ -0,0 +1,1 @@
 +xyzzy
 \ No newline at end of file
 EOF
@@ -41,7 +41,7 @@
 index 7c465af..0000000
 --- a/frotz
 +++ /dev/null
-@@ -1 +0,0 @@
+@@ -1,1 +0,0 @@
 -xyzzy
 \ No newline at end of file
 EOF
@@ -68,7 +68,7 @@
 index 7c465af..df1db54 120000
 --- a/frotz
 +++ b/frotz
-@@ -1 +1 @@
+@@ -1,1 +1,1 @@
 -xyzzy
 \ No newline at end of file
 +yxyyz
Previous: Davide LibenziNext: Junio C Hamano
Message 19 of 22 in “Use a *real* built-in diff generator”
  1. Linus TorvaldsMar 25, 2006
  2. Junio C HamanoMar 25, 2006
  3. Junio C HamanoMar 25, 2006
  4. Linus TorvaldsMar 25, 2006
  5. Davide LibenziMar 25, 2006
  6. Marco CostalbaMar 25, 2006
  7. Alex RiesenMar 25, 2006
  8. Linus TorvaldsMar 25, 2006
  9. Morten WelinderMar 25, 2006
  10. Linus TorvaldsMar 25, 2006
  11. Linus TorvaldsMar 25, 2006
  12. Davide LibenziMar 25, 2006
  13. Linus TorvaldsMar 25, 2006
  14. Davide LibenziMar 26, 2006
  15. Ralf BaechleMar 26, 2006
  16. Petr BaudisMar 26, 2006
  17. Linus TorvaldsMar 25, 2006
  18. Davide LibenziMar 26, 2006
  19. Junio C HamanoMar 25, 2006
  20. Junio C HamanoMar 25, 2006
  21. Linus TorvaldsMar 25, 2006
  22. Linus TorvaldsMar 25, 2006

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.