From d51e74a4c2f669732b945ff525839b1d6f221c39 Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Sun, 14 Sep 2025 11:27:18 +0100 Subject: [PATCH 1/9] Finally create a working test-case for #119 Assisted-by: Cursor --- tests/interdiff-color-context/run-test | 123 +++++++++++++++++++------ 1 file changed, 94 insertions(+), 29 deletions(-) diff --git a/tests/interdiff-color-context/run-test b/tests/interdiff-color-context/run-test index b03d2782..dbd3da1a 100755 --- a/tests/interdiff-color-context/run-test +++ b/tests/interdiff-color-context/run-test @@ -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" From 111cc97990889655886bfc64d11de4b8cc2599a7 Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Sun, 14 Sep 2025 11:31:17 +0100 Subject: [PATCH 2/9] Create our own ANSI codes for colours instead of passing through --color Fixed: #119 Assisted-by: Cursor --- Makefile.am | 1 - src/interdiff.c | 180 +++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 154 insertions(+), 27 deletions(-) diff --git a/Makefile.am b/Makefile.am index 6e7cb83e..f0905b53 100644 --- a/Makefile.am +++ b/Makefile.am @@ -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) \ diff --git a/src/interdiff.c b/src/interdiff.c index 54d6c0f4..0414806c 100644 --- a/src/interdiff.c +++ b/src/interdiff.c @@ -58,6 +58,25 @@ #define PATCH "patch" #endif +/* ANSI color codes for diff output */ +#define COLOR_RESET "\033[0m" +#define COLOR_HEADER "\033[1m" /* Bold for headers */ +#define COLOR_FILE "\033[1m" /* Bold for filenames */ +#define COLOR_HUNK "\033[36m" /* Cyan for hunk headers */ +#define COLOR_ADDED "\033[32m" /* Green for added lines */ +#define COLOR_REMOVED "\033[31m" /* Red for removed lines */ +#define COLOR_CONTEXT "" /* No color for context lines */ + +/* Line type for coloring */ +enum line_type { + LINE_CONTEXT, + LINE_ADDED, + LINE_REMOVED, + LINE_HEADER, + LINE_FILE, + LINE_HUNK +}; + /* This can be invoked as interdiff, combinediff, or flipdiff. */ static enum { mode_inter, @@ -98,11 +117,62 @@ 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; + +/* + * Colorize a line based on its type and whether colors are enabled. + * Returns a newly allocated string that must be freed by the caller. + * Only colorizes when outputting to a terminal (stdout is a TTY). + */ +static char * +colorize_line (const char *line, enum line_type type, FILE *output_file) +{ + const char *color_start = ""; + const char *color_end = ""; + char *result; + + /* Only colorize if colors are enabled AND we're outputting to stdout */ + if (!use_colors || output_file != stdout) { + return xstrdup (line); + } + + switch (type) { + case LINE_CONTEXT: + color_start = COLOR_CONTEXT; + break; + case LINE_ADDED: + color_start = COLOR_ADDED; + break; + case LINE_REMOVED: + color_start = COLOR_REMOVED; + break; + case LINE_HEADER: + color_start = COLOR_HEADER; + break; + case LINE_FILE: + color_start = COLOR_FILE; + break; + case LINE_HUNK: + color_start = COLOR_HUNK; + break; + } + + if (color_start[0] != '\0') { + color_end = COLOR_RESET; + } + + if (asprintf (&result, "%s%s%s", color_start, line, color_end) < 0) { + error (EXIT_FAILURE, errno, "Memory allocation failed"); + } + + return result; +} static struct file_list *files_in_patch2 = NULL; static struct file_list *files_in_patch1 = NULL; @@ -982,7 +1052,9 @@ trim_context (FILE *f /* positioned at start of @@ line */, if (line[0] == '\\') { /* Pass '\' lines through unaltered. */ - fputs (line, out); + char *colored_line = colorize_line (line, LINE_CONTEXT, out); + fputs (colored_line, out); + free (colored_line); continue; } @@ -1050,13 +1122,35 @@ 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"); + { + char *hunk_header; + char *colored_hunk; + if (new_orig_count != 1 && new_new_count != 1) { + if (asprintf (&hunk_header, "@@ -%lu,%lu +%lu,%lu @@\n", + orig_offset, new_orig_count, new_offset, new_new_count) < 0) { + error (EXIT_FAILURE, errno, "Memory allocation failed"); + } + } else if (new_orig_count != 1) { + if (asprintf (&hunk_header, "@@ -%lu,%lu +%lu @@\n", + orig_offset, new_orig_count, new_offset) < 0) { + error (EXIT_FAILURE, errno, "Memory allocation failed"); + } + } else if (new_new_count != 1) { + if (asprintf (&hunk_header, "@@ -%lu +%lu,%lu @@\n", + orig_offset, new_offset, new_new_count) < 0) { + error (EXIT_FAILURE, errno, "Memory allocation failed"); + } + } else { + if (asprintf (&hunk_header, "@@ -%lu +%lu @@\n", + orig_offset, new_offset) < 0) { + error (EXIT_FAILURE, errno, "Memory allocation failed"); + } + } + colored_hunk = colorize_line (hunk_header, LINE_HUNK, out); + fputs (colored_hunk, out); + free (hunk_header); + free (colored_hunk); + } while (total_count--) { ssize_t got = getline (&line, &linelen, f); @@ -1070,7 +1164,25 @@ trim_context (FILE *f /* positioned at start of @@ line */, if (total_count < strip_post) continue; - fwrite (line, (size_t) got, 1, out); + { + enum line_type type; + switch (line[0]) { + case '+': + type = LINE_ADDED; + break; + case '-': + type = LINE_REMOVED; + break; + case ' ': + case '\n': + default: + type = LINE_CONTEXT; + break; + } + char *colored_line = colorize_line (line, type, out); + fputs (colored_line, out); + free (colored_line); + } } } @@ -1253,6 +1365,7 @@ output_delta (FILE *p1, FILE *p2, FILE *out) /* First character */ if (human_readable) { char *p, *q, c, d; + char *header_line; c = d = '\0'; /* shut gcc up */ p = strchr (oldname + 4, '\t'); if (p) { @@ -1264,13 +1377,35 @@ output_delta (FILE *p1, FILE *p2, FILE *out) d = *q; *q = '\0'; } - fprintf (out, DIFF " %s %s %s\n", options, oldname + 4, - newname + 4); + if (asprintf (&header_line, DIFF " %s %s %s\n", options, oldname + 4, + newname + 4) < 0) { + error (EXIT_FAILURE, errno, "Memory allocation failed"); + } + char *colored_header = colorize_line (header_line, LINE_HEADER, out); + fputs (colored_header, out); + free (header_line); + free (colored_header); if (p) *p = c; if (q) *q = d; } - fprintf (out, "--- %s\n", oldname + 4); - fprintf (out, "+++ %s\n", newname + 4); + { + char *old_line, *new_line; + char *colored_old, *colored_new; + if (asprintf (&old_line, "--- %s\n", oldname + 4) < 0) { + error (EXIT_FAILURE, errno, "Memory allocation failed"); + } + if (asprintf (&new_line, "+++ %s\n", newname + 4) < 0) { + error (EXIT_FAILURE, errno, "Memory allocation failed"); + } + colored_old = colorize_line (old_line, LINE_FILE, out); + colored_new = colorize_line (new_line, LINE_FILE, out); + fputs (colored_old, out); + fputs (colored_new, out); + free (old_line); + free (new_line); + free (colored_old); + free (colored_new); + } rewind (tmpdiff); trim_context (tmpdiff, file.unline, out); fclose (tmpdiff); @@ -2284,8 +2419,9 @@ main (int argc, char *argv[]) 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': @@ -2317,17 +2453,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) From 13548efa7e95e204a49469e64ec2861df1fb86f2 Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Mon, 15 Sep 2025 16:57:40 +0100 Subject: [PATCH 3/9] Use variadic function and puts() line colour prefix/suffix Suggestion from Sultan Alsawaf Assisted-by: Cursor --- src/interdiff.c | 174 ++++++++++++++++++++---------------------------- 1 file changed, 72 insertions(+), 102 deletions(-) diff --git a/src/interdiff.c b/src/interdiff.c index 0414806c..7e41755d 100644 --- a/src/interdiff.c +++ b/src/interdiff.c @@ -33,6 +33,7 @@ #endif /* HAVE_ERROR_H */ #include #include +#include #include #include #include @@ -125,58 +126,66 @@ static struct patlist *pat_drop_context = NULL; static struct file_list *files_done = NULL; -/* - * Colorize a line based on its type and whether colors are enabled. - * Returns a newly allocated string that must be freed by the caller. - * Only colorizes when outputting to a terminal (stdout is a TTY). - */ -static char * -colorize_line (const char *line, enum line_type type, FILE *output_file) -{ - const char *color_start = ""; - const char *color_end = ""; - char *result; - - /* Only colorize if colors are enabled AND we're outputting to stdout */ - if (!use_colors || output_file != stdout) { - return xstrdup (line); - } - - switch (type) { - case LINE_CONTEXT: - color_start = COLOR_CONTEXT; - break; - case LINE_ADDED: - color_start = COLOR_ADDED; - break; - case LINE_REMOVED: - color_start = COLOR_REMOVED; - break; - case LINE_HEADER: - color_start = COLOR_HEADER; - break; - case LINE_FILE: - color_start = COLOR_FILE; - break; - case LINE_HUNK: - color_start = COLOR_HUNK; - break; - } - - if (color_start[0] != '\0') { - color_end = COLOR_RESET; - } - - if (asprintf (&result, "%s%s%s", color_start, line, color_end) < 0) { - error (EXIT_FAILURE, errno, "Memory allocation failed"); - } - - return result; -} 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. + * This avoids the need for memory allocation and deallocation + * that was required with the colorize_line approach. + */ + 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) { + switch (type) { + case LINE_CONTEXT: + color_start = COLOR_CONTEXT; + break; + case LINE_ADDED: + color_start = COLOR_ADDED; + break; + case LINE_REMOVED: + color_start = COLOR_REMOVED; + break; + case LINE_HEADER: + color_start = COLOR_HEADER; + break; + case LINE_FILE: + color_start = COLOR_FILE; + break; + case LINE_HUNK: + color_start = COLOR_HUNK; + break; + } + + 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') { + fputs (color_end, output_file); + } + } + + /* checks whether file needs processing and sets context */ static int check_filename (const char *fn) { @@ -1052,9 +1061,7 @@ trim_context (FILE *f /* positioned at start of @@ line */, if (line[0] == '\\') { /* Pass '\' lines through unaltered. */ - char *colored_line = colorize_line (line, LINE_CONTEXT, out); - fputs (colored_line, out); - free (colored_line); + print_color (out, LINE_CONTEXT, "%s", line); continue; } @@ -1123,33 +1130,19 @@ trim_context (FILE *f /* positioned at start of @@ line */, fsetpos (f, &pos); { - char *hunk_header; - char *colored_hunk; if (new_orig_count != 1 && new_new_count != 1) { - if (asprintf (&hunk_header, "@@ -%lu,%lu +%lu,%lu @@\n", - orig_offset, new_orig_count, new_offset, new_new_count) < 0) { - error (EXIT_FAILURE, errno, "Memory allocation failed"); - } + 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) { - if (asprintf (&hunk_header, "@@ -%lu,%lu +%lu @@\n", - orig_offset, new_orig_count, new_offset) < 0) { - error (EXIT_FAILURE, errno, "Memory allocation failed"); - } + print_color (out, LINE_HUNK, "@@ -%lu,%lu +%lu @@\n", + orig_offset, new_orig_count, new_offset); } else if (new_new_count != 1) { - if (asprintf (&hunk_header, "@@ -%lu +%lu,%lu @@\n", - orig_offset, new_offset, new_new_count) < 0) { - error (EXIT_FAILURE, errno, "Memory allocation failed"); - } + print_color (out, LINE_HUNK, "@@ -%lu +%lu,%lu @@\n", + orig_offset, new_offset, new_new_count); } else { - if (asprintf (&hunk_header, "@@ -%lu +%lu @@\n", - orig_offset, new_offset) < 0) { - error (EXIT_FAILURE, errno, "Memory allocation failed"); - } + print_color (out, LINE_HUNK, "@@ -%lu +%lu @@\n", + orig_offset, new_offset); } - colored_hunk = colorize_line (hunk_header, LINE_HUNK, out); - fputs (colored_hunk, out); - free (hunk_header); - free (colored_hunk); } while (total_count--) { @@ -1179,9 +1172,7 @@ trim_context (FILE *f /* positioned at start of @@ line */, type = LINE_CONTEXT; break; } - char *colored_line = colorize_line (line, type, out); - fputs (colored_line, out); - free (colored_line); + print_color (out, type, "%s", line); } } } @@ -1365,7 +1356,6 @@ output_delta (FILE *p1, FILE *p2, FILE *out) /* First character */ if (human_readable) { char *p, *q, c, d; - char *header_line; c = d = '\0'; /* shut gcc up */ p = strchr (oldname + 4, '\t'); if (p) { @@ -1377,34 +1367,14 @@ output_delta (FILE *p1, FILE *p2, FILE *out) d = *q; *q = '\0'; } - if (asprintf (&header_line, DIFF " %s %s %s\n", options, oldname + 4, - newname + 4) < 0) { - error (EXIT_FAILURE, errno, "Memory allocation failed"); - } - char *colored_header = colorize_line (header_line, LINE_HEADER, out); - fputs (colored_header, out); - free (header_line); - free (colored_header); + print_color (out, LINE_HEADER, DIFF " %s %s %s\n", options, oldname + 4, + newname + 4); if (p) *p = c; if (q) *q = d; } { - char *old_line, *new_line; - char *colored_old, *colored_new; - if (asprintf (&old_line, "--- %s\n", oldname + 4) < 0) { - error (EXIT_FAILURE, errno, "Memory allocation failed"); - } - if (asprintf (&new_line, "+++ %s\n", newname + 4) < 0) { - error (EXIT_FAILURE, errno, "Memory allocation failed"); - } - colored_old = colorize_line (old_line, LINE_FILE, out); - colored_new = colorize_line (new_line, LINE_FILE, out); - fputs (colored_old, out); - fputs (colored_new, out); - free (old_line); - free (new_line); - free (colored_old); - free (colored_new); + 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); From 5ec12f83c7e8dc5996b59f519d3defcbe0fcf10f Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Tue, 16 Sep 2025 12:41:26 +0100 Subject: [PATCH 4/9] Incorporate Sultan Alsawaf's suggestions from PR review --- src/interdiff.c | 131 +++++++++++++++++++----------------------------- 1 file changed, 51 insertions(+), 80 deletions(-) diff --git a/src/interdiff.c b/src/interdiff.c index 7e41755d..35aa4802 100644 --- a/src/interdiff.c +++ b/src/interdiff.c @@ -59,23 +59,25 @@ #define PATCH "patch" #endif -/* ANSI color codes for diff output */ #define COLOR_RESET "\033[0m" -#define COLOR_HEADER "\033[1m" /* Bold for headers */ -#define COLOR_FILE "\033[1m" /* Bold for filenames */ -#define COLOR_HUNK "\033[36m" /* Cyan for hunk headers */ -#define COLOR_ADDED "\033[32m" /* Green for added lines */ -#define COLOR_REMOVED "\033[31m" /* Red for removed lines */ -#define COLOR_CONTEXT "" /* No color for context lines */ /* Line type for coloring */ enum line_type { - LINE_CONTEXT, + LINE_FILE = 0, + LINE_HEADER, + LINE_HUNK, LINE_ADDED, LINE_REMOVED, - LINE_HEADER, - LINE_FILE, - LINE_HUNK +}; + +/* 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. */ @@ -131,58 +133,33 @@ static struct file_list *files_in_patch1 = NULL; /* * Print colored output using a variadic format string. - * This avoids the need for memory allocation and deallocation - * that was required with the colorize_line approach. */ 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) { - switch (type) { - case LINE_CONTEXT: - color_start = COLOR_CONTEXT; - break; - case LINE_ADDED: - color_start = COLOR_ADDED; - break; - case LINE_REMOVED: - color_start = COLOR_REMOVED; - break; - case LINE_HEADER: - color_start = COLOR_HEADER; - break; - case LINE_FILE: - color_start = COLOR_FILE; - break; - case LINE_HUNK: - color_start = COLOR_HUNK; - break; - } - - 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') { - fputs (color_end, output_file); - } + 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') + fputs (color_end, output_file); } /* checks whether file needs processing and sets context */ @@ -1061,7 +1038,7 @@ trim_context (FILE *f /* positioned at start of @@ line */, if (line[0] == '\\') { /* Pass '\' lines through unaltered. */ - print_color (out, LINE_CONTEXT, "%s", line); + fputs (line, out); continue; } @@ -1146,6 +1123,7 @@ trim_context (FILE *f /* positioned at start of @@ line */, } while (total_count--) { + enum line_type type; ssize_t got = getline (&line, &linelen, f); assert (got > 0); @@ -1157,23 +1135,18 @@ trim_context (FILE *f /* positioned at start of @@ line */, if (total_count < strip_post) continue; - { - enum line_type type; - switch (line[0]) { - case '+': - type = LINE_ADDED; - break; - case '-': - type = LINE_REMOVED; - break; - case ' ': - case '\n': - default: - type = LINE_CONTEXT; - break; - } - print_color (out, type, "%s", line); + 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); } } @@ -2385,9 +2358,8 @@ 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"; - } /* Set our internal color flag instead of passing to diff */ use_colors = (strcmp(color_mode, "always") == 0); @@ -2424,9 +2396,8 @@ main (int argc, char *argv[]) "-z and --in-place are mutually exclusive."); /* Set default color behavior if no color option was specified */ - if (!color_option_specified && isatty(STDOUT_FILENO)) { + if (!color_option_specified && isatty(STDOUT_FILENO)) use_colors = 1; - } if (optind + 2 != argc) syntax (1); From 126b622e1ed03b331c2d62dd8fb23fbd105e15de Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Tue, 16 Sep 2025 12:52:24 +0100 Subject: [PATCH 5/9] Remove some unnecessary braces --- src/interdiff.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/interdiff.c b/src/interdiff.c index 35aa4802..ded87394 100644 --- a/src/interdiff.c +++ b/src/interdiff.c @@ -166,11 +166,11 @@ static struct file_list *files_in_patch1 = NULL; 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; } @@ -237,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; - } } } } @@ -2174,12 +2173,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"); From 05c4b2d48ec5a42d37956d555df8379c2bfe619b Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Tue, 16 Sep 2025 14:18:48 +0100 Subject: [PATCH 6/9] Remove another unnecessary extra code block --- src/interdiff.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/interdiff.c b/src/interdiff.c index ded87394..46096ea6 100644 --- a/src/interdiff.c +++ b/src/interdiff.c @@ -1344,10 +1344,8 @@ output_delta (FILE *p1, FILE *p2, FILE *out) if (p) *p = c; if (q) *q = d; } - { - print_color (out, LINE_FILE, "--- %s\n", oldname + 4); - print_color (out, LINE_FILE, "+++ %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); From 151b978aae16058cf3b77b74f672324f21e7684e Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Wed, 17 Sep 2025 09:43:40 +0100 Subject: [PATCH 7/9] More improvements from PR review --- src/interdiff.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/interdiff.c b/src/interdiff.c index 46096ea6..0f8e757e 100644 --- a/src/interdiff.c +++ b/src/interdiff.c @@ -59,25 +59,24 @@ #define PATCH "patch" #endif -#define COLOR_RESET "\033[0m" - /* Line type for coloring */ enum line_type { - LINE_FILE = 0, + LINE_FILE, LINE_HEADER, LINE_HUNK, LINE_ADDED, LINE_REMOVED, + LINE_MAX }; /* ANSI color codes for diff output */ -static char*color_codes[] = { +static const char *const color_codes[LINE_MAX] = { [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 */ + [LINE_REMOVED] = "\033[31m" /* Red for removed lines */ }; /* This can be invoked as interdiff, combinediff, or flipdiff. */ @@ -127,7 +126,6 @@ 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; @@ -137,19 +135,15 @@ static struct file_list *files_in_patch1 = NULL; static void print_color (FILE *output_file, enum line_type type, const char *format, ...) { - const char *color_start = ""; - const char *color_end = ""; + const char *color_start = NULL; va_list args; /* Only colorize if colors are enabled AND we're outputting to stdout */ - if (use_colors && output_file == 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') + if (color_start) fputs (color_start, output_file); /* Print the formatted content */ @@ -158,8 +152,8 @@ static struct file_list *files_in_patch1 = NULL; va_end (args); /* Print color end code */ - if (color_end[0] != '\0') - fputs (color_end, output_file); + if (color_start) + fputs ("\033[0m", output_file); } /* checks whether file needs processing and sets context */ From badc0f1232d532ec4ac322f24a49e80de060223d Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Wed, 17 Sep 2025 12:22:56 +0100 Subject: [PATCH 8/9] Avoid name collision with LINE_MAX --- src/interdiff.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/interdiff.c b/src/interdiff.c index 0f8e757e..f3f474b0 100644 --- a/src/interdiff.c +++ b/src/interdiff.c @@ -66,11 +66,11 @@ enum line_type { LINE_HUNK, LINE_ADDED, LINE_REMOVED, - LINE_MAX + LINE_TYPE_MAX }; /* ANSI color codes for diff output */ -static const char *const color_codes[LINE_MAX] = { +static const char *const color_codes[LINE_TYPE_MAX] = { [LINE_FILE] = "\033[1m", /* Bold for filenames */ [LINE_HEADER] = "\033[1m", /* Bold for headers */ From d30408f9e1ec5f0c5abb88572d888d17ae09c8e5 Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Wed, 17 Sep 2025 12:42:32 +0100 Subject: [PATCH 9/9] Update NEWS --- NEWS | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/NEWS b/NEWS index 3678644e..ad05c3f3 100644 --- a/NEWS +++ b/NEWS @@ -14,6 +14,10 @@ Patchutils news compatibility (will change to 'strip' in version 0.5.0). Addresses GitHub issues #22, #27, #59, and #68. + Fixed interdiff --color option handling and improved color output. + Replaced external diff --color dependency with internal ANSI color + codes. Addresses GitHub issue #119. + Code improvements and build system enhancements. Added redirectfd() utility function to redirect stdout without reassignment. Enhanced CI testing with musl support for better compatibility testing.