From e824430ce969970154a4621527ea3f8a86159dcf Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Mon, 20 Oct 2025 14:48:37 +0100 Subject: [PATCH 1/4] Document -s/--status option in grepdiff help text The -s/--status option has been implemented and working in grepdiff (inherited from filterdiff) but was not documented in the help text. The help text previously only mentioned it for lsdiff: -s, --status (lsdiff) Updated to include grepdiff and clarify the status indicators: -s, --status (lsdiff, grepdiff) show file additions (+), removals (-), and modifications (!) This matches the documentation in grepdiff(1) man page and brings the help text in sync with the actual functionality. Note: The legacy filterdiff implementation has a bug where grepdiff mode shows '!' for all files instead of the correct +/-/! indicators. This will be addressed separately. The help text documents the correct, intended behavior. Assisted-by: Claude Code --- src/filterdiff.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/filterdiff.c b/src/filterdiff.c index 4a00a7e..7bb2956 100644 --- a/src/filterdiff.c +++ b/src/filterdiff.c @@ -1587,8 +1587,8 @@ const char * syntax_str = " prefix pathnames in old files with PREFIX\n" " --addnewprefix=PREFIX\n" " prefix pathnames in new files with PREFIX\n" -" -s, --status (lsdiff)\n" -" show file additions and removals (lsdiff)\n" +" -s, --status (lsdiff, grepdiff)\n" +" show file additions (+), removals (-), and modifications (!) (lsdiff, grepdiff)\n" " -v, --verbose\n" " verbose output -- use more than once for extra verbosity\n" " -E, --extended-regexp (grepdiff)\n" From 463b0c53736804df238235205336179bb6829733 Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Mon, 20 Oct 2025 14:48:47 +0100 Subject: [PATCH 2/4] Add grepdiff --status test from master This test currently has incorrect expected output - it expects '!' for all matching files, but according to the grepdiff documentation, the -s/--status flag should show: + for file additions - for file removals ! for file modifications The next commit will fix the test expectations to match the documented behavior. Assisted-by: Claude Code --- tests/grepdiff-status/run-test | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100755 tests/grepdiff-status/run-test diff --git a/tests/grepdiff-status/run-test b/tests/grepdiff-status/run-test new file mode 100755 index 0000000..721a80e --- /dev/null +++ b/tests/grepdiff-status/run-test @@ -0,0 +1,38 @@ +#!/bin/sh + +# Test grepdiff -s/--status option +# Shows file status: + (addition), - (removal), ! (modification) + +. ${top_srcdir-.}/tests/common.sh + +cat << EOF > diff +--- /dev/null ++++ newfile +@@ -0,0 +1 @@ ++content +--- oldfile ++++ /dev/null +@@ -1 +0,0 @@ +-content +--- modified ++++ modified +@@ -1 +1 @@ +-old ++new +EOF + +${GREPDIFF} -s 'content' diff 2>errors >output || exit 1 +[ -s errors ] && exit 1 + +cat << EOF | cmp - output || exit 1 +! newfile +! oldfile +EOF + +# Test with modification +${GREPDIFF} -s 'new' diff 2>errors >output2 || exit 1 +[ -s errors ] && exit 1 + +cat << EOF | cmp - output2 || exit 1 +! modified +EOF From 3657a34d493d2652a5bc160ff2b59eedb48263fe Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Mon, 20 Oct 2025 14:49:22 +0100 Subject: [PATCH 3/4] Fix grepdiff --status test expectations The previous test expected '!' for all matching files, but this is incorrect according to the grepdiff(1) documentation. The -s/--status flag should show: + for file additions (old file is /dev/null) - for file removals (new file is /dev/null) ! for file modifications (both files exist) This matches the behavior documented for both grepdiff and lsdiff. The legacy filterdiff implementation in --grep mode has a bug where it always shows '!' regardless of the actual file operation. This test now correctly verifies the documented behavior. The test now comprehensively checks all three status indicators: - First test (pattern 'content'): verifies +, -, and ! - Second test (pattern 'new'): verifies ! for modifications - Third test (with --empty-files-as-absent): verifies that empty files with only additions are treated as new files and show + Note: The implementation does not yet pass these tests. The fixes will come in a later commit. Assisted-by: Claude Code --- tests/grepdiff-status/run-test | 39 ++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/tests/grepdiff-status/run-test b/tests/grepdiff-status/run-test index 721a80e..7b2bec4 100755 --- a/tests/grepdiff-status/run-test +++ b/tests/grepdiff-status/run-test @@ -1,7 +1,10 @@ #!/bin/sh # Test grepdiff -s/--status option -# Shows file status: + (addition), - (removal), ! (modification) +# According to the documentation, -s should show: +# + for file additions +# - for file removals +# ! for file modifications . ${top_srcdir-.}/tests/common.sh @@ -19,20 +22,48 @@ cat << EOF > diff @@ -1 +1 @@ -old +new +--- another-modified ++++ another-modified +@@ -1,2 +1,2 @@ + context +-old content ++new content EOF +# Test additions and deletions (files with 'content' pattern) ${GREPDIFF} -s 'content' diff 2>errors >output || exit 1 [ -s errors ] && exit 1 cat << EOF | cmp - output || exit 1 -! newfile -! oldfile ++ newfile +- oldfile +! another-modified EOF -# Test with modification +# Test modification only (file with 'new' pattern) ${GREPDIFF} -s 'new' diff 2>errors >output2 || exit 1 [ -s errors ] && exit 1 cat << EOF | cmp - output2 || exit 1 ! modified +! another-modified +EOF + +# Test with --empty-files-as-absent +# File with only additions should be treated as new file +cat << EOF > diff3 +--- emptyfile ++++ emptyfile +@@ -0,0 +1 @@ ++content +@@ -60 +60 @@ +-old ++new +EOF + +${GREPDIFF} -s --empty-files-as-absent 'content' diff3 2>errors >output3 || exit 1 +[ -s errors ] && exit 1 + +cat << EOF | cmp - output3 || exit 1 ++ emptyfile EOF From 517a615a40d83718172d793e89364469604fc9f0 Mon Sep 17 00:00:00 2001 From: Tim Waugh Date: Mon, 20 Oct 2025 15:07:16 +0100 Subject: [PATCH 4/4] Fix grepdiff --status by passing status as pointer The grepdiff --status flag was not working correctly, particularly with --empty-files-as-absent. The issue was that status needed to be updated after empty file processing but before filename display. This commit refactors the code to pass status by pointer to the diff processing functions (do_unified, do_context, do_git_diff_no_hunks), allowing them to update the status value at the correct time. Changes: - Modified function signatures to accept char *status instead of char status - Added status update logic after empty file processing in each function - Updated all callers to pass &status instead of status - Removed duplicate status update logic that was previously done after the functions returned This ensures grepdiff -s displays the correct status (+/-/!) for all file types, including: - New files (show +) - Deleted files (show -) - Modified files (show !) - Empty files when --empty-files-as-absent is used All 187 tests pass. Assisted-by: Claude Code --- src/filterdiff.c | 64 +++++++++++++++++++++++++++++++----------------- 1 file changed, 41 insertions(+), 23 deletions(-) diff --git a/src/filterdiff.c b/src/filterdiff.c index 7bb2956..33c94f7 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,6 +386,16 @@ 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) { @@ -394,7 +404,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; @@ -455,7 +465,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) { @@ -671,7 +681,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); } @@ -746,6 +756,16 @@ 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; } @@ -753,7 +773,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) { @@ -1020,7 +1040,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); } @@ -1150,6 +1170,16 @@ 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; } @@ -1180,7 +1210,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 @@ -1375,19 +1405,13 @@ 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 (!orig_file_exists) - status = '+'; - else if (!new_file_exists) - status = '-'; - + if (match && show_status && mode == mode_list) display_filename (start_linenum, status, p, patchname); - } /* Clean up */ free (git_old_name); @@ -1476,19 +1500,13 @@ 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 (!orig_file_exists) - status = '+'; - else if (!new_file_exists) - status = '-'; - + if (match && show_status && mode == mode_list) display_filename (start_linenum, status, p, patchname); - } switch (result) { case EOF: