Re: [PATCH] diff: ensure consistent diff behavior with -I<regex> across output formats
From: Junio C Hamano <hidden>
Date: 2025-08-03 15:43:57
Jeff King [off-list ref] writes:
quoted hunk
We already have the diff_from_contents flag which is used for --exit-code. We should be able to see where that logic is applied and do something similar. It looks like it happens in diff_flush(), which makes sense: if (output_format & DIFF_FORMAT_NO_OUTPUT && options->flags.exit_with_status && options->flags.diff_from_contents) { /* * run diff_flush_patch for the exit status. setting * options->file to /dev/null should be safe, because we * aren't supposed to produce any output anyway. */ diff_free_file(options); options->file = xfopen("/dev/null", "w"); options->close_file = 1; options->color_moved = 0; for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; if (check_pair_status(p)) diff_flush_patch(p, options); if (options->found_changes) break; } } So here's a naive application of the same technique:diff --git a/diff.c b/diff.c index 76291e238c..0fe6eb7443 100644 --- a/diff.c +++ b/diff.c@@ -6845,8 +6845,28 @@ void diff_flush(struct diff_options *options) DIFF_FORMAT_CHECKDIFF)) { for (i = 0; i < q->nr; i++) { struct diff_filepair *p = q->queue[i]; - if (check_pair_status(p)) - flush_one_pair(p, options); + + if (!check_pair_status(p)) + continue; + + if (options->flags.diff_from_contents) { + FILE *orig_out = options->file; + int orig_changes = options->found_changes; + int skip; + + options->file = xfopen("/dev/null", "w"); + diff_flush_patch(p, options); + skip = !options->found_changes; + + fclose(options->file); + options->file = orig_out; + options->found_changes = orig_changes; + + if (skip) + continue; + } + + flush_one_pair(p, options); } separator++; }which works on a trivial example. It affects all of raw, name-only, name-status, and checkdiff. I know Junio said that --raw should not be affected, but I'm not sure I agree. Anyway, it should be possible to split the logic by output type.
This is much more palatable to avoid code duplication and repetition, compared to what was posted before. A new code that loops over q->queue[] one more time (which is yucky but cannot be helped due to output order as you noted earlier) and runs diff_flush_patch(), like the one we see above, is probably the best we can do. The beauty of the above approach is that the new code only needs to know about a single .diff_from_contents bit, and does not have to care what features cause the .diff_from_contents bit to be flipped on. Doing anything more would be maintenance nightmare. Thanks.