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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 30, 2025, 00:28 UTC
Message-ID: <xmqqcy9io73j.fsf@gitster.g>
In-Reply-To: <20250729081820.34626-1-yldhome2d2@gmail.com>

Lidong Yan <yldhome2d2@gmail.com> writes:

Show 23 quoted lines

> Previously, the `-I<regex>` option was inconsistently applied across
> various `git diff` output formats. In some cases, files would appear
> in the `--name-only` output but not in the accompanying `--stat` or
> `-p` outputs, despite the user explicitly requesting to ignore certain
> changes using `-I<regex>`. To provide this consistency, Introduces
> the diffcore_ignore() function in the new diffcore-ignore.c file, which
> removes changes matching `-I<regex>`, and call diffcore_ignore() in
> diffcore_std().
>
> This patch ensures that the behavior of `-I<regex>` is applied
> consistently across multiple diff output formats (`--name-only`,
> `--name-status`, `--stat`, and `-p`). Only `--raw` and `--check` will
> ignore `-I<regex>` and retain the original output.
>
> Signed-off-by: Lidong Yan <yldhome2d2@gmail.com>
> ---
>  Makefile                |   1 +
>  diff.c                  |   2 +
>  diffcore-ignore.c       | 152 ++++++++++++++++++++++++++++++++++++++++
>  diffcore.h              |   1 +
>  t/t4013-diff-various.sh |  57 ++++++++++++++-
>  5 files changed, 211 insertions(+), 2 deletions(-)
>  create mode 100644 diffcore-ignore.c

The enthusiasm is appreciated, but the implementation raises two questions.

 * This special cases -I<pattern>, but any option that causes us to
   set the .diff_from_contents flag, not just -I<pattern>, can cause
   the raw blob comparison to be potentially different from what the
   blob contents are compared with various "ignore this class of
   changes" criteria.  Shouldn't "git diff -w --name-status" and the
   like get the same treatment?

 * Also, should we internally run diff twice, especially even when
   we are going to show the patch output and are not limited to
   FORMAT_NAME and FORMAT_NAME_STATUS?  Generally, running the real
   diff in any of the diffcore transformatin is a sign of trouble.

Also, the usual way to compose a log message of this project is to

     - Give an observation on how the current system works in the
       present tense (so no need to say "Currently X is Y", or
       "Previously X was Y" to describe the state before your change;
       just "X is Y" is enough), and discuss what you perceive as a
       problem in it.

     - Propose a solution (optional---often, problem description
       trivially leads to an obvious solution in reader's minds).

     - Give commands to somebody editing the codebase to "make it so",
       instead of saying "This commit does X".

in this order.