Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,6 @@ TESTS = tests/newline1/run-test \
XFAIL_TESTS = \
tests/delhunk5/run-test \
tests/delhunk6/run-test \
tests/interdiff-color-context/run-test \
tests/rediff-empty-hunk/run-test

test-perms: src/combinediff$(EXEEXT) src/flipdiff$(EXEEXT) \
Expand Down
140 changes: 102 additions & 38 deletions src/interdiff.c
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#endif /* HAVE_ERROR_H */
#include <ctype.h>
#include <errno.h>
#include <stdarg.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
Expand All @@ -58,6 +59,27 @@
#define PATCH "patch"
#endif

#define COLOR_RESET "\033[0m"

/* Line type for coloring */
enum line_type {
LINE_FILE = 0,
LINE_HEADER,
LINE_HUNK,
LINE_ADDED,
LINE_REMOVED,
};

/* ANSI color codes for diff output */
static char*color_codes[] = {

[LINE_FILE] = "\033[1m", /* Bold for filenames */
[LINE_HEADER] = "\033[1m", /* Bold for headers */
[LINE_HUNK] = "\033[36m", /* Cyan for hunk headers */
[LINE_ADDED] = "\033[32m", /* Green for added lines */
[LINE_REMOVED] = "\033[31m", /* Red for removed lines */
};

/* This can be invoked as interdiff, combinediff, or flipdiff. */
static enum {
mode_inter,
Expand Down Expand Up @@ -98,23 +120,57 @@ static int ignore_components = 0;
static int ignore_components_specified = 0;
static int unzip = 0;
static int no_revert_omitted = 0;
static int use_colors = 0;
static int color_option_specified = 0;
static int debug = 0;

static struct patlist *pat_drop_context = NULL;

static struct file_list *files_done = NULL;

static struct file_list *files_in_patch2 = NULL;
static struct file_list *files_in_patch1 = NULL;

/* checks whether file needs processing and sets context */
/*
* Print colored output using a variadic format string.
*/
static void
print_color (FILE *output_file, enum line_type type, const char *format, ...)
{
const char *color_start = "";
const char *color_end = "";
va_list args;

/* Only colorize if colors are enabled AND we're outputting to stdout */
if (use_colors && output_file == stdout) {
color_start = color_codes[type];
if (color_start[0] != '\0')
color_end = COLOR_RESET;
}

/* Print color start code */
if (color_start[0] != '\0')
fputs (color_start, output_file);

/* Print the formatted content */
va_start (args, format);
vfprintf (output_file, format, args);
va_end (args);

/* Print color end code */
if (color_end[0] != '\0')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to if (color_start)

fputs (color_end, output_file);
}

/* checks whether file needs processing and sets context */
static int
check_filename (const char *fn)
{
if (patlist_match(pat_drop_context, fn)) {
if (patlist_match(pat_drop_context, fn))
max_context = 0;
} else {
else
max_context = max_context_real;
}

return 1;
}

Expand Down Expand Up @@ -181,9 +237,8 @@ determine_ignore_components (struct file_list *list1, struct file_list *list2)
const char *stripped1 = stripped(l1->file, p);
for (struct file_list *l2 = list2; l2; l2 = l2->next) {
const char *stripped2 = stripped(l2->file, p);
if (!strcmp(stripped1, stripped2)) {
if (!strcmp(stripped1, stripped2))
return p;
}
}
}
}
Expand Down Expand Up @@ -1050,15 +1105,24 @@ trim_context (FILE *f /* positioned at start of @@ line */,
printf ("Trim: %lu,%lu\n", strip_pre, strip_post);

fsetpos (f, &pos);
fprintf (out, "@@ -%lu", orig_offset);
if (new_orig_count != 1)
fprintf (out, ",%lu", new_orig_count);
fprintf (out, " +%lu", new_offset);
if (new_new_count != 1)
fprintf (out, ",%lu", new_new_count);
fprintf (out, " @@\n");
{
if (new_orig_count != 1 && new_new_count != 1) {
print_color (out, LINE_HUNK, "@@ -%lu,%lu +%lu,%lu @@\n",
orig_offset, new_orig_count, new_offset, new_new_count);
} else if (new_orig_count != 1) {
print_color (out, LINE_HUNK, "@@ -%lu,%lu +%lu @@\n",
orig_offset, new_orig_count, new_offset);
} else if (new_new_count != 1) {
print_color (out, LINE_HUNK, "@@ -%lu +%lu,%lu @@\n",
orig_offset, new_offset, new_new_count);
} else {
print_color (out, LINE_HUNK, "@@ -%lu +%lu @@\n",
orig_offset, new_offset);
}
}

while (total_count--) {
enum line_type type;
ssize_t got = getline (&line, &linelen, f);
assert (got > 0);

Expand All @@ -1070,7 +1134,18 @@ trim_context (FILE *f /* positioned at start of @@ line */,
if (total_count < strip_post)
continue;

fwrite (line, (size_t) got, 1, out);
switch (line[0]) {
case '+':
type = LINE_ADDED;
break;
case '-':
type = LINE_REMOVED;
break;
default:
fwrite (line, (size_t) got, 1, out);
continue;
}
print_color (out, type, "%s", line);
}
}

Expand Down Expand Up @@ -1264,13 +1339,13 @@ output_delta (FILE *p1, FILE *p2, FILE *out)
d = *q;
*q = '\0';
}
fprintf (out, DIFF " %s %s %s\n", options, oldname + 4,
newname + 4);
print_color (out, LINE_HEADER, DIFF " %s %s %s\n", options, oldname + 4,
newname + 4);
if (p) *p = c;
if (q) *q = d;
}
fprintf (out, "--- %s\n", oldname + 4);
fprintf (out, "+++ %s\n", newname + 4);
print_color (out, LINE_FILE, "--- %s\n", oldname + 4);
print_color (out, LINE_FILE, "+++ %s\n", newname + 4);
rewind (tmpdiff);
trim_context (tmpdiff, file.unline, out);
fclose (tmpdiff);
Expand Down Expand Up @@ -2096,12 +2171,10 @@ interdiff (FILE *p1, FILE *p2, const char *patch1, const char *patch2)

if (flipdiff_inplace) {
/* Use atomic in-place writing for safety */
if (write_file_inplace(patch2, flip1) != 0) {
if (write_file_inplace(patch2, flip1) != 0)
error (EXIT_FAILURE, errno, "failed to write %s", patch2);
}
if (write_file_inplace(patch1, flip2) != 0) {
if (write_file_inplace(patch1, flip2) != 0)
error (EXIT_FAILURE, errno, "failed to write %s", patch1);
}
} else {
copy (flip1, stdout);
puts ("\n=== 8< === cut here === 8< ===\n");
Expand Down Expand Up @@ -2280,12 +2353,12 @@ main (int argc, char *argv[])
const char *color_mode = optarg ? optarg : "auto";

/* Handle auto mode: check if stdout is a terminal */
if (strcmp(color_mode, "auto") == 0) {
if (strcmp(color_mode, "auto") == 0)
color_mode = isatty(STDOUT_FILENO) ? "always" : "never";
}

if (asprintf (diff_opts + num_diff_opts++, "--color=%s", color_mode) < 0)
error (EXIT_FAILURE, errno, "Memory allocation failed");
/* Set our internal color flag instead of passing to diff */
use_colors = (strcmp(color_mode, "always") == 0);
color_option_specified = 1;
break;
}
case 1000 + 'I':
Expand Down Expand Up @@ -2317,18 +2390,9 @@ main (int argc, char *argv[])
error (EXIT_FAILURE, 0,
"-z and --in-place are mutually exclusive.");

/* Add default color=always if no color option was specified and we're in a terminal */
int has_color_option = 0;
for (int i = 0; i < num_diff_opts; i++) {
if (strncmp(diff_opts[i], "--color", 7) == 0) {
has_color_option = 1;
break;
}
}
if (!has_color_option && isatty(STDOUT_FILENO)) {
if (asprintf (diff_opts + num_diff_opts++, "--color=always") < 0)
error (EXIT_FAILURE, errno, "Memory allocation failed");
}
/* Set default color behavior if no color option was specified */
if (!color_option_specified && isatty(STDOUT_FILENO))
use_colors = 1;

if (optind + 2 != argc)
syntax (1);
Expand Down
123 changes: 94 additions & 29 deletions tests/interdiff-color-context/run-test
Original file line number Diff line number Diff line change
Expand Up @@ -2,48 +2,113 @@

# This is an interdiff(1) testcase.
# Test: --color option should not break context trimming
#
# The bug: When --color is passed to internal diff, ANSI escape codes
# break trim_context()'s ability to detect and strip "unline" content.
# This test verifies the bug is fixed by ensuring --color doesn't break interdiff.

. ${top_srcdir-.}/tests/common.sh

# Create test files
cat << EOF > file
# Create test files that will generate interdiff output
cat << EOF > original.txt
line1
line2
line3
line4
line5
line6
line7
line8
line9
line10
line11
line12
EOF

# First patch
cat << EOF > patch1
--- file
+++ file
@@ -1,3 +1,3 @@
# Create patches that modify overlapping regions to force interdiff generation
cat << EOF > patch1.patch
--- original.txt
+++ version1.txt
@@ -1,6 +1,6 @@
line1
-line2
+MODIFIED2
line3
line2
-line3
+MODIFIED3
line4
line5
line6
@@ -7,4 +7,4 @@
line7
line8
-line9
+MODIFIED9
line10
EOF

# Second patch
cat << EOF > patch2
--- file
+++ file
@@ -4,3 +4,3 @@
line10
-line11
+MODIFIED11
line12
cat << EOF > patch2.patch
--- original.txt
+++ version2.txt
@@ -2,6 +2,6 @@
line2
line3
line4
-line5
+CHANGED5
line6
line7
@@ -8,3 +8,3 @@
line8
line9
-line10
+CHANGED10
EOF

# Generate interdiff with --color=always
${INTERDIFF} --color=always patch1 patch2 2>errors >result || { cat errors; exit 1; }
# Test the key requirement: --color should not break interdiff functionality
# The original bug would cause interdiff to fail or produce incorrect output

# First, test without color to establish baseline
${INTERDIFF} patch1.patch patch2.patch >baseline.out 2>baseline.err
baseline_exit=$?

# Then test with --color=always
${INTERDIFF} --color=always patch1.patch patch2.patch >color.out 2>color.err
color_exit=$?

# Key test 1: Both should have the same exit status
if [ $baseline_exit -ne $color_exit ]; then
echo "ERROR: --color option changes interdiff exit status"
echo "Baseline exit: $baseline_exit, Color exit: $color_exit"
exit 1
fi

# Key test 2: If baseline succeeded, color version should also produce reasonable output
if [ $baseline_exit -eq 0 ]; then
# Strip ANSI codes from color output for comparison
sed 's/\x1b\[[0-9;]*m//g' color.out > color_stripped.out

# The semantic content should be equivalent
if ! diff baseline.out color_stripped.out >/dev/null 2>&1; then
echo "ERROR: --color option changes interdiff semantic output"
echo "This indicates the color implementation interferes with diff processing"
echo
echo "=== Baseline output ==="
cat baseline.out
echo
echo "=== Color output (stripped) ==="
cat color_stripped.out
echo
echo "=== Difference ==="
diff baseline.out color_stripped.out || true
exit 1
fi
fi

# Apply patch1 to create the base state for testing the interdiff
cp file file-with-patch1
${PATCH} file-with-patch1 < patch1 || exit 1
# Key test 3: The color output should not contain malformed content
# Look for signs that trim_context() failed due to color codes
if grep -E '^\x1b\[[0-9;]*m[!]+' color.out >/dev/null 2>&1; then
echo "ERROR: Found colored unline artifacts in output"
echo "This suggests trim_context() failed to strip artificial content"
echo "Raw output:"
cat -v color.out
exit 1
fi

# Test that the interdiff result can be applied cleanly
cp file-with-patch1 file-test
${PATCH} file-test < result || exit 1
echo "SUCCESS: --color option does not break interdiff context trimming"