From 229228ef1a07575956611e2fdac1f0e5dda9c51f Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Mon, 20 Oct 2025 17:00:24 +0100 Subject: [PATCH 1/5] Revert "Fix grepdiff --status by passing status as pointer" This reverts commit 968d8812d88a4349f13196e2aff087f90a2d0727. --- src/filterdiff.c | 64 +++++++++++++++++------------------------------- 1 file changed, 23 insertions(+), 41 deletions(-) diff --git a/src/filterdiff.c b/src/filterdiff.c index 33c94f7..7bb2956 100644 --- a/src/filterdiff.c +++ b/src/filterdiff.c @@ -359,7 +359,7 @@ static int do_git_diff_no_hunks (FILE *f, char **header, unsigned int num_headers, int match, char **line, size_t *linelen, unsigned long *linenum, unsigned long start_linenum, - char *status, const char *bestname, const char *patchname, + char status, const char *bestname, const char *patchname, int *orig_file_exists, int *new_file_exists, enum git_diff_type git_type) { @@ -386,16 +386,6 @@ do_git_diff_no_hunks (FILE *f, char **header, unsigned int num_headers, break; } - /* Update status based on file existence (do this early so returns below have correct status) */ - if (status != NULL && mode != mode_filter && show_status && - orig_file_exists != NULL && new_file_exists != NULL) { - if (!*orig_file_exists) - *status = '+'; - else if (!*new_file_exists) - *status = '-'; - /* else: keep existing '!' value for modifications */ - } - /* If this diff matches the filter, display it */ if (match) { if (mode == mode_filter) { @@ -404,7 +394,7 @@ do_git_diff_no_hunks (FILE *f, char **header, unsigned int num_headers, fputs (header[i], stdout); } else if (mode == mode_list && !displayed_filename) { if (!show_status) { - display_filename (start_linenum, *status, + display_filename (start_linenum, status, bestname, patchname); } displayed_filename = 1; @@ -465,7 +455,7 @@ static int do_unified (FILE *f, char **header, unsigned int num_headers, int match, char **line, size_t *linelen, unsigned long *linenum, - unsigned long start_linenum, char *status, + unsigned long start_linenum, char status, const char *bestname, const char *patchname, int *orig_file_exists, int *new_file_exists) { @@ -681,7 +671,7 @@ do_unified (FILE *f, char **header, unsigned int num_headers, if (!displayed_filename) { displayed_filename = 1; display_filename (start_linenum, - *status, bestname, + status, bestname, patchname); } @@ -756,16 +746,6 @@ do_unified (FILE *f, char **header, unsigned int num_headers, *new_file_exists = 0; } - /* Update status based on final file existence after empty file processing */ - if (status != NULL && mode != mode_filter && show_status && - orig_file_exists != NULL && new_file_exists != NULL) { - if (!*orig_file_exists) - *status = '+'; - else if (!*new_file_exists) - *status = '-'; - /* else: keep existing '!' value for modifications */ - } - return ret; } @@ -773,7 +753,7 @@ static int do_context (FILE *f, char **header, unsigned int num_headers, int match, char **line, size_t *linelen, unsigned long *linenum, - unsigned long start_linenum, char *status, + unsigned long start_linenum, char status, const char *bestname, const char *patchname, int *orig_file_exists, int *new_file_exists) { @@ -1040,7 +1020,7 @@ do_context (FILE *f, char **header, unsigned int num_headers, if (!displayed_filename) { displayed_filename = 1; display_filename(start_linenum, - *status, + status, bestname, patchname); } @@ -1170,16 +1150,6 @@ do_context (FILE *f, char **header, unsigned int num_headers, *new_file_exists = 0; } - /* Update status based on final file existence after empty file processing */ - if (status != NULL && mode != mode_filter && show_status && - orig_file_exists != NULL && new_file_exists != NULL) { - if (!*orig_file_exists) - *status = '+'; - else if (!*new_file_exists) - *status = '-'; - /* else: keep existing '!' value for modifications */ - } - return ret; } @@ -1210,7 +1180,7 @@ static int filterdiff (FILE *f, const char *patchname) int (*do_diff) (FILE *, char **, unsigned int, int, char **, size_t *, unsigned long *, unsigned long, - char *, const char *, const char *, + char, const char *, const char *, int *, int *); orig_file_exists = 0; // shut gcc up @@ -1405,13 +1375,19 @@ static int filterdiff (FILE *f, const char *patchname) /* Process the git diff (it will handle filename display) */ result = do_git_diff_no_hunks (f, header, num_headers, match, &line, &linelen, &linenum, - start_linenum, &status, p, patchname, + start_linenum, status, p, patchname, &orig_file_exists, &new_file_exists, git_type); /* Print filename with status if in list mode and matches */ - if (match && show_status && mode == mode_list) + if (match && show_status && mode == mode_list) { + if (!orig_file_exists) + status = '+'; + else if (!new_file_exists) + status = '-'; + display_filename (start_linenum, status, p, patchname); + } /* Clean up */ free (git_old_name); @@ -1500,13 +1476,19 @@ static int filterdiff (FILE *f, const char *patchname) result = do_diff (f, header, num_headers, match, &line, &linelen, &linenum, - start_linenum, &status, p, patchname, + start_linenum, status, p, patchname, &orig_file_exists, &new_file_exists); // print if it matches. - if (match && show_status && mode == mode_list) + if (match && show_status && mode == mode_list) { + if (!orig_file_exists) + status = '+'; + else if (!new_file_exists) + status = '-'; + display_filename (start_linenum, status, p, patchname); + } switch (result) { case EOF: From d8907e24d3907ea52ca1a46380c9b5a6bdd996c0 Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Mon, 20 Oct 2025 17:12:45 +0100 Subject: [PATCH 2/5] Document and support --empty-files-as-absent for grepdiff The --empty-files-as-absent option was documented for lsdiff but not properly supported or documented for grepdiff. This option is useful with grepdiff -s/--status to correctly identify file additions vs modifications when files have empty old versions. For grepdiff, -E already means --extended-regexp, so the short form cannot be used. This commit makes --empty-files-as-absent available as a long option only for grepdiff. Changes: - Updated help text to show --empty-files-as-absent for grepdiff (long form only, since -E means --extended-regexp) - Updated help text for lsdiff to show -E, --empty-files-as-absent - Changed option code from 'E' to '1000 + E' to avoid conflict with the -E short option for --extended-regexp - Added case handler for long form option in grepdiff mode - Updated man page documentation for both lsdiff and grepdiff sections with clearer descriptions and examples - Documented --empty-files-as-removed as an alias (still accepted but not shown in help text) All 188 tests pass (grepdiff-status test not yet added to TESTS). Assisted-by: Claude Code --- doc/patchutils.xml | 12 +++++++++--- src/filterdiff.c | 12 ++++++++++-- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/doc/patchutils.xml b/doc/patchutils.xml index b561852..b862571 100644 --- a/doc/patchutils.xml +++ b/doc/patchutils.xml @@ -1280,11 +1280,14 @@ filterdiff -i 'b/*/newname' git-patch-with-renames.patch]]> , + , Treat empty files as absent for the purpose of displaying file additions, modifications and - removals. + removals. For example, a file with an empty old version + and additions in the new version will be treated as a + file addition (+) rather than a modification (!). @@ -1646,12 +1649,15 @@ This is the same as gitdiff but uses git show instead of git diff. - , + + , Treat empty files as absent for the purpose of displaying file additions, modifications and - removals. + removals. For example, a file with an empty old version + and additions in the new version will be treated as a + file addition (+) rather than a modification (!). Note: For grepdiff, only the long form is available ( means for grepdiff). diff --git a/src/filterdiff.c b/src/filterdiff.c index 7bb2956..704983b 100644 --- a/src/filterdiff.c +++ b/src/filterdiff.c @@ -1599,6 +1599,8 @@ const char * syntax_str = #endif " -E, --empty-files-as-absent (lsdiff)\n" " treat empty files as absent (lsdiff)\n" +" --empty-files-as-absent (grepdiff)\n" +" treat empty files as absent (grepdiff)\n" " -f FILE, --file=FILE (grepdiff)\n" " read regular expressions from FILE (grepdiff)\n" " --filter run as 'filterdiff' (grepdiff, patchview, lsdiff)\n" @@ -1822,7 +1824,7 @@ int main (int argc, char *argv[]) {"remove-timestamps", 0, 0, 1000 + 'r'}, {"with-filename", 0, 0, 'H'}, {"no-filename", 0, 0, 'h'}, - {"empty-files-as-absent", 0, 0, 'E'}, + {"empty-files-as-absent", 0, 0, 1000 + 'E'}, {"number-files", 0, 0, 'N'}, {"clean", 0, 0, 1000 + 'c'}, {"strip-match", 1, 0, 'p'}, @@ -1835,7 +1837,7 @@ int main (int argc, char *argv[]) {"strip-match", 1, 0, 'p'}, {"status", 0, 0, 's'}, {"extended-regexp", 0, 0, 'E'}, - {"empty-files-as-removed", 0, 0, 'E'}, + {"empty-files-as-removed", 0, 0, 1000 + 'E'}, {"file", 1, 0, 'f'}, {"in-place", 0, 0, 1000 + 'w'}, {"git-prefixes", 1, 0, 1000 + 'G'}, @@ -1865,6 +1867,12 @@ int main (int argc, char *argv[]) empty_files_as_absent = 1; else syntax (1); break; + case 1000 + 'E': + /* Long form --empty-files-as-absent or --empty-files-as-removed */ + if (mode == mode_grep || mode == mode_list) + empty_files_as_absent = 1; + else syntax (1); + break; case 'f': if (mode == mode_grep) { regex_file_specified = 1; From 1cf3914a64ba7bc5e65cd4beeba917e66ef518b1 Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Mon, 20 Oct 2025 17:15:16 +0100 Subject: [PATCH 3/5] Add grepdiff-status test to TESTS This test verifies that grepdiff -s/--status correctly shows: + for file additions (old file is /dev/null) - for file removals (new file is /dev/null) ! for file modifications (both files exist) The test also verifies that --empty-files-as-absent works correctly with -s to treat files with empty old versions as additions rather than modifications. The test currently fails - it documents the correct expected behavior that needs to be implemented. Assisted-by: Claude Code --- Makefile.am | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile.am b/Makefile.am index 2294ed3..79b41f1 100644 --- a/Makefile.am +++ b/Makefile.am @@ -225,6 +225,7 @@ TESTS = tests/newline1/run-test \ tests/grepdiff8/run-test \ tests/grepdiff9/run-test \ tests/grepdiff-original-line-numbers/run-test \ + tests/grepdiff-status/run-test \ tests/number1/run-test \ tests/number2/run-test \ tests/number3/run-test \ From 338bd827776369d7e7d820e5518d1ef8fd6b1405 Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Mon, 20 Oct 2025 17:21:29 +0100 Subject: [PATCH 4/5] Fix grepdiff -s/--status to show correct file status indicators The grepdiff -s option was incorrectly showing '!' (modification) for all matching files, regardless of whether they were additions, deletions, or modifications. The issue was that grepdiff calls display_filename() from inside do_unified() and do_context() when it finds a matching line, but the status variable was always initialized to '!' and never updated based on file existence. This fix calculates the correct status (+/-/!) inside do_unified() and do_context() just before calling display_filename(), using: - orig_file_exists/new_file_exists to check if files exist - orig_is_empty/new_is_empty to check for empty files when --empty-files-as-absent is used The fix applies to both unified and context diff formats. All 194 tests pass (188 pass + 6 expected failures). Assisted-by: Claude Code --- src/filterdiff.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/src/filterdiff.c b/src/filterdiff.c index 704983b..9e70c19 100644 --- a/src/filterdiff.c +++ b/src/filterdiff.c @@ -669,9 +669,28 @@ do_unified (FILE *f, char **header, unsigned int num_headers, !regexecs (regex, num_regex, *line + 1, 0, NULL, 0)) { if (output_matching == output_none) { if (!displayed_filename) { + char display_status = status; + /* Update status based on file existence for grepdiff -s */ + if (show_status) { + int orig_absent = orig_file_exists && !*orig_file_exists; + int new_absent = new_file_exists && !*new_file_exists; + + /* Check for empty files if --empty-files-as-absent is set */ + if (empty_files_as_absent) { + if (orig_file_exists && *orig_file_exists && orig_is_empty) + orig_absent = 1; + if (new_file_exists && *new_file_exists && new_is_empty) + new_absent = 1; + } + + if (orig_absent) + display_status = '+'; + else if (new_absent) + display_status = '-'; + } displayed_filename = 1; display_filename (start_linenum, - status, bestname, + display_status, bestname, patchname); } @@ -1018,9 +1037,28 @@ do_context (FILE *f, char **header, unsigned int num_headers, 0, NULL, 0)) { if (output_matching == output_none) { if (!displayed_filename) { + char display_status = status; + /* Update status based on file existence for grepdiff -s */ + if (show_status) { + int orig_absent = orig_file_exists && !*orig_file_exists; + int new_absent = new_file_exists && !*new_file_exists; + + /* Check for empty files if --empty-files-as-absent is set */ + if (empty_files_as_absent) { + if (orig_file_exists && *orig_file_exists && orig_is_empty) + orig_absent = 1; + if (new_file_exists && *new_file_exists && new_is_empty) + new_absent = 1; + } + + if (orig_absent) + display_status = '+'; + else if (new_absent) + display_status = '-'; + } displayed_filename = 1; display_filename(start_linenum, - status, + display_status, bestname, patchname); } From 68429ac13c9bb08ca975e9ed5299c564ee204078 Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Mon, 20 Oct 2025 17:32:07 +0100 Subject: [PATCH 5/5] Add comprehensive test coverage for grepdiff -s Expanded the grepdiff-status test to cover additional code paths: 1. File deletion with --empty-files-as-absent: Tests when the new file is empty and should be treated as deleted (displays -) 2. Context diff format: Tests grepdiff -s with context diff format (*** / --- style) to verify correct status display for additions, deletions, and modifications 3. Context diff with --empty-files-as-absent: Tests empty file detection in context diff format These additional test cases ensure complete coverage of the status calculation logic in both do_unified() and do_context() functions, including the empty file handling branches. All 194 tests pass (188 pass + 6 expected failures). Assisted-by: Claude Code --- tests/grepdiff-status/run-test | 75 +++++++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/tests/grepdiff-status/run-test b/tests/grepdiff-status/run-test index 7b2bec4..380a60a 100755 --- a/tests/grepdiff-status/run-test +++ b/tests/grepdiff-status/run-test @@ -49,7 +49,7 @@ cat << EOF | cmp - output2 || exit 1 ! another-modified EOF -# Test with --empty-files-as-absent +# Test with --empty-files-as-absent for file addition # File with only additions should be treated as new file cat << EOF > diff3 --- emptyfile @@ -67,3 +67,76 @@ ${GREPDIFF} -s --empty-files-as-absent 'content' diff3 2>errors >output3 || exit cat << EOF | cmp - output3 || exit 1 + emptyfile EOF + +# Test with --empty-files-as-absent for file deletion +# File with only deletions should be treated as deleted +cat << EOF > diff4 +--- deletedfile ++++ deletedfile +@@ -1 +0,0 @@ +-content +@@ -60 +60 @@ +-old ++new +EOF + +${GREPDIFF} -s --empty-files-as-absent 'content' diff4 2>errors >output4 || exit 1 +[ -s errors ] && exit 1 + +cat << EOF | cmp - output4 || exit 1 +- deletedfile +EOF + +# Test with context diff format +cat << EOF > diff5 +*** /dev/null +--- newfile-ctx +*************** +*** 0 **** +--- 1 ---- ++ content +*** oldfile-ctx +--- /dev/null +*************** +*** 1 **** +- content +--- 0 ---- +*** modified-ctx +--- modified-ctx +*************** +*** 1 **** +! old content +--- 1 ---- +! new content +EOF + +${GREPDIFF} -s 'content' diff5 2>errors >output5 || exit 1 +[ -s errors ] && exit 1 + +cat << EOF | cmp - output5 || exit 1 ++ newfile-ctx +- oldfile-ctx +! modified-ctx +EOF + +# Test context diff with --empty-files-as-absent +cat << EOF > diff6 +*** emptyfile-ctx +--- emptyfile-ctx +*************** +*** 0 **** +--- 1 ---- ++ content +*************** +*** 60 **** +! old +--- 60 ---- +! new +EOF + +${GREPDIFF} -s --empty-files-as-absent 'content' diff6 2>errors >output6 || exit 1 +[ -s errors ] && exit 1 + +cat << EOF | cmp - output6 || exit 1 ++ emptyfile-ctx +EOF