Merge pull request #166 from twaugh/backport-pr156-fixes

Backport pr156 fixes

Author: twaugh

Makefile.am

@@ -304,6 +304,8 @@ TESTS = tests/newline1/run-test \
 	tests/whitespace-regression/run-test \
 	tests/interdiff-whitespace-w/run-test \
 	tests/interdiff-color-context/run-test \
+	tests/interdiff-patch2-headers/run-test \
+	tests/interdiff-mode-only/run-test \
 	tests/patch-ignore-whitespace/run-test \
 	tests/whitespace-w/run-test \
 	tests/filterdiff-inplace1/run-test \

src/interdiff.c

@@ -1136,7 +1136,8 @@ trim_context (FILE *f /* positioned at start of @@ line */,
 				fwrite (line, (size_t) got, 1, out);
 				continue;
 			}
-			print_color (out, type, "%s", line);
+			print_color (out, type, "%.*s", (int) got - 1, line);
+			fputc ('\n', out);
 		}
 	}
 
@@ -1442,29 +1443,36 @@ index_patch_generic (FILE *patch_file, struct file_list **file_list, int need_sk
 
 		/* For patch2, we need to handle the @@ line and skip content */
 		if (need_skip_content) {
-			if (getline (&line, &linelen, patch_file) == -1) {
+			int found = 0;
+
+			while (!found &&
+			       getline (&line, &linelen, patch_file) > 0) {
+				if (strncmp (line, "@@ ", 3))
+					continue;
+
+				p = strchr (line + 3, '+');
+				if (!p)
+					continue;
+
+				p = strchr (p, ',');
+				if (p) {
+					/* Like '@@ -1,3 +1,3 @@' */
+					p++;
+					skip = strtoul (p, &end, 10);
+					if (p == end)
+						continue;
+				} else
+					/* Like '@@ -1 +1 @@' */
+					skip = 1;
+				found = 1;
+			}
+
+			if (!found) {
 				free (names[0]);
 				free (names[1]);
 				break;
 			}
 
-			if (strncmp (line, "@@ ", 3))
-				goto try_next;
-
-			p = strchr (line + 3, '+');
-			if (!p)
-				goto try_next;
-			p = strchr (p, ',');
-			if (p) {
-				/* Like '@@ -1,3 +1,3 @@' */
-				p++;
-				skip = strtoul (p, &end, 10);
-				if (p == end)
-					goto try_next;
-			} else
-				/* Like '@@ -1 +1 @@' */
-				skip = 1;
-
 			add_to_list (file_list, best_name (2, names), pos);
 
 			while (skip--) {
@@ -1480,18 +1488,33 @@ index_patch_generic (FILE *patch_file, struct file_list **file_list, int need_sk
 			add_to_list (file_list, best_name (2, names), pos);
 		}
 
-	try_next:
 		free (names[0]);
 		free (names[1]);
 	}
 
 	if (line)
 		free (line);
 
+	/* Return success if the file is empty, has indexed files, or
+	 * has no --- lines at all (merge commits, mode-only changes,
+	 * notes-only commits, binary-only patches, etc.). */
 	if (file_is_empty || *file_list)
 		return 0;
-	else
-		return 1;
+
+	/* Check if the patch has any file-header --- lines (as
+	 * opposed to commit-message separators or b4 tracking).
+	 * If not, it simply has no diffable content. */
+	line = NULL;
+	linelen = 0;
+	rewind (patch_file);
+	while (getline (&line, &linelen, patch_file) != -1)
+		if (!strncmp (line, "--- a/", 6) ||
+		    !strncmp (line, "--- /dev/null", 13)) {
+			free (line);
+			return 1;
+		}
+	free (line);
+	return 0;
 }
 
 static int
@@ -2113,11 +2136,15 @@ interdiff (FILE *p1, FILE *p2, const char *patch1, const char *patch2)
 
 		names[0] = filename_from_header (line + 4);
 
-		if (getline (&line, &linelen, p1) == -1)
+		if (getline (&line, &linelen, p1) == -1) {
+			free (names[0]);
 			break;
+		}
 
-		if (strncmp (line, "+++ ", 4))
+		if (strncmp (line, "+++ ", 4)) {
+			free (names[0]);
 			continue;
+		}
 
 		names[1] = filename_from_header (line + 4);
 
@@ -2150,8 +2177,21 @@ interdiff (FILE *p1, FILE *p2, const char *patch1, const char *patch2)
                 free (p);
 	}
 
-	if (!file_is_empty && !patch_found)
-		no_patch (patch1);
+	if (!file_is_empty && !patch_found) {
+		/* Don't warn for patches with no file-header --- lines
+		 * (merge commits, mode-only, binary-only, etc.). */
+		int has_diff_content = 0;
+
+		rewind (p1);
+		while (getline (&line, &linelen, p1) != -1)
+			if (!strncmp (line, "--- a/", 6) ||
+			    !strncmp (line, "--- /dev/null", 13)) {
+				has_diff_content = 1;
+				break;
+			}
+		if (has_diff_content)
+			no_patch (patch1);
+	}
 
 	copy_residue (p2, mode == mode_flip ? flip1 : stdout);
 

tests/interdiff-mode-only/run-test

@@ -0,0 +1,35 @@
+#!/bin/sh
+
+# Test interdiff with mode-only patches
+# This tests that interdiff doesn't incorrectly warn "doesn't contain a patch"
+# for patches that only contain mode changes, binary diffs, or merge metadata
+
+. ${top_srcdir-.}/tests/common.sh
+
+# Create first patch - mode-only change (no actual diff content)
+cat << EOF > patch1
+diff --git a/script.sh b/script.sh
+old mode 100644
+new mode 100755
+EOF
+
+# Create second patch - same mode-only change
+cat << EOF > patch2
+diff --git a/script.sh b/script.sh
+old mode 100644
+new mode 100755
+EOF
+
+# Run interdiff - should succeed without the false "doesn't contain a patch" error
+${INTERDIFF} patch1 patch2 2>errors >actual || exit 1
+
+# The key test: should NOT have "doesn't contain a patch" error
+if grep -q "doesn't contain a patch" errors; then
+    echo "FAIL: Got spurious 'doesn't contain a patch' error"
+    cat errors
+    exit 1
+fi
+
+# For mode-only patches, interdiff should produce minimal or no output
+# Just verify it ran without error
+exit 0

tests/interdiff-patch2-headers/run-test

@@ -0,0 +1,54 @@
+#!/bin/sh
+
+# Test interdiff with patch2 having headers between +++ and @@
+# This tests that interdiff can handle extra headers (like "index") between
+# the +++ line and @@ line in patch2, without incorrectly reporting it as empty
+
+. ${top_srcdir-.}/tests/common.sh
+
+# Create first patch (simple unified diff)
+cat << EOF > patch1
+--- a/file.txt
++++ b/file.txt
+@@ -1,3 +1,3 @@
+ line one
+-line two
++line TWO
+ line three
+EOF
+
+# Create second patch (git-style with index line between +++ and @@)
+cat << EOF > patch2
+--- a/file.txt
++++ b/file.txt
+index abc123..def456 100644
+@@ -1,3 +1,3 @@
+ line one
+-line two
++line 2
+ line three
+EOF
+
+# Expected interdiff output
+# This shows the difference between changing "two" to "TWO" vs "2"
+cat << EOF > expected
+diff -u b/file.txt b/file.txt
+--- b/file.txt
++++ b/file.txt
+@@ -1,3 +1,3 @@
+ line one
+-line TWO
++line 2
+ line three
+EOF
+
+# Run interdiff - should succeed without errors
+${INTERDIFF} patch1 patch2 2>errors >actual || exit 1
+
+# Should not have any errors (especially not "patch2 is empty" or similar)
+[ -s errors ] && exit 1
+
+# Compare the actual output with expected
+diff -u expected actual || exit 1
+
+exit 0