git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] diff: ensure consistent diff behavior with -I<regex> across output formats

From
Junio C Hamano <gitster@pobox.com>
Date
Aug 3, 2025, 15:43 UTC
Message-ID
<xmqqo6swfm11.fsf@gitster.g>
In-Reply-To
<20250802102249.GA3738980@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 68 quoted lines
> 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.
Previous: Lidong YanNext: Junio C Hamano
Message 17 of 35 in “git-diff: --ignore-matching-lines has no effect on the output when --name-only is used”
  1. hi@arnes.spaceJul 23, 2025
  2. Lidong YanJul 23, 2025
  3. Junio C HamanoJul 23, 2025
  4. Lidong YanJul 24, 2025
  5. Eric SunshineJul 24, 2025
  6. Lidong YanJul 24, 2025
  7. hi@arnes.spaceJul 25, 2025
  8. hi@arnes.spaceJul 25, 2025
  9. Lidong YanJul 25, 2025
  10. hi@arnes.spaceJul 25, 2025
  11. Jeff KingJul 25, 2025
  12. Junio C HamanoJul 25, 2025
  13. diff: ensure consistent diff behavior with -I<regex> across output formatsLidong Yan, Jul 29, 2025
  14. Junio C HamanoJul 30, 2025
  15. Jeff KingAug 2, 2025
  16. Lidong YanAug 3, 2025
  17. Junio C HamanoAug 3, 2025
  18. Junio C HamanoAug 4, 2025
  19. Jeff KingAug 4, 2025
  20. diff: ensure consistent diff behavior with -I<regex> across output formatsLidong Yan, Aug 3, 2025
  21. Junio C HamanoAug 4, 2025
  22. Lidong YanAug 4, 2025
  23. Junio C HamanoAug 4, 2025
  24. Lidong YanAug 5, 2025
  25. Junio C HamanoAug 5, 2025
  26. diff: ensure consistent diff behavior with ignore optionsLidong Yan, Aug 6, 2025
  27. Junio C HamanoAug 6, 2025
  28. Lidong YanAug 7, 2025
  29. Junio C HamanoAug 6, 2025
  30. Lidong YanAug 7, 2025
  31. diff: ensure consistent diff behavior with ignore optionsLidong Yan, Aug 7, 2025
  32. Junio C HamanoAug 7, 2025
  33. Lidong YanAug 8, 2025
  34. diff: ensure consistent diff behavior with ignore optionsLidong Yan, Aug 8, 2025
  35. Johannes SchindelinOct 16, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.