git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Lidong Yan <yldhome2d2@gmail.com>
Cc: git@vger.kernel.org,  hi@arnes.space,  michal@isc.org,  peff@peff.net
Subject: Re: [PATCH v2] diff: ensure consistent diff behavior with -I<regex> across output formats
Date: Sun, 03 Aug 2025 21:36:02 -0700	[thread overview]
Message-ID: <xmqqzfcfd7pp.fsf@gitster.g> (raw)
In-Reply-To: <B1F1EBB7-1824-4776-B8B0-B557114546DB@gmail.com> (Lidong Yan's message of "Mon, 4 Aug 2025 09:56:22 +0800")

Lidong Yan <yldhome2d2@gmail.com> writes:

>> I do not quite get why ignore_match() has to know so much about how
>> the real code in diff.c that implements -I<regex> works, compared to
>> the illustration of "here is how to do it" Peff posted, though.  It
>> somehow feels too much duplicated code.
>
> I did copy some code from diffcore-pickaxe.c. I will use Peff's code in the
> next patch and try to refactor diff_flush() to make the code simpler. Though
> the reason I match the regular expression in ignore_match() is that I want to
> return early as soon as an unmatched change is found. And indeed, it's not
> worth writing the duplicated code for this unknown performance benefit.

In the production code, it would be truly worth doing the
optimization; we want to avoid running diff twice if we can.

But I think the refactoring of diff_flush() codepath would may
involve some new mode (perhaps DIFF_FORMAT_DRYRUN or something) that

 (1) does not produce any output, like DIFF_FORMAT_NO_OUTPUT, so
     that we do not need to play with /dev/null like Peff's
     illustration.

 (2) knows that the caller is only interested in each path having
     any change worth reporting, so that it can short-circuit once a
     change is found for each path.

So, just before you want to decide showing name or name-status,
you'd do this extra diff_flush() that is run only to learn if each
path has changes (with various "ignore" criteria) in the dry-run
mode, and it can do as much short-cut as it needs to.

Hmm?

  reply	other threads:[~2025-08-04  4:36 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-23  5:47 git-diff: --ignore-matching-lines has no effect on the output when --name-only is used hi
2025-07-23  8:00 ` Lidong Yan
2025-07-23 17:09   ` Junio C Hamano
2025-07-24  1:56     ` Lidong Yan
2025-07-24  2:16       ` Eric Sunshine
2025-07-24  3:38         ` Lidong Yan
2025-07-25  6:00     ` hi
2025-07-25  6:06       ` hi
2025-07-25  6:46       ` Lidong Yan
2025-07-25  8:08         ` hi
2025-07-25 11:11           ` Jeff King
2025-07-25 15:20             ` Junio C Hamano
2025-07-29  8:18               ` [PATCH] diff: ensure consistent diff behavior with -I<regex> across output formats Lidong Yan
2025-07-30  0:28                 ` Junio C Hamano
2025-08-02 10:22                   ` Jeff King
2025-08-03  8:42                     ` Lidong Yan
2025-08-03 15:43                     ` Junio C Hamano
2025-08-04  4:39                     ` Junio C Hamano
2025-08-04 12:42                       ` Jeff King
2025-08-03 14:51                   ` [PATCH v2] " Lidong Yan
2025-08-04  0:39                     ` Junio C Hamano
2025-08-04  1:56                       ` Lidong Yan
2025-08-04  4:36                         ` Junio C Hamano [this message]
2025-08-05  9:23                           ` Lidong Yan
2025-08-05 16:11                             ` Junio C Hamano
2025-08-06 12:33                     ` [PATCH v3] diff: ensure consistent diff behavior with ignore options Lidong Yan
2025-08-06 17:35                       ` Junio C Hamano
2025-08-07  1:23                         ` Lidong Yan
2025-08-06 20:56                       ` Junio C Hamano
2025-08-07  1:39                         ` Lidong Yan
2025-08-07  2:06                       ` [PATCH v4] " Lidong Yan
2025-08-07 21:27                         ` Junio C Hamano
2025-08-08  1:46                           ` Lidong Yan
2025-08-08  3:30                             ` [PATCH v5] " Lidong Yan
2025-10-16 14:55                               ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqzfcfd7pp.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=hi@arnes.space \
    --cc=michal@isc.org \
    --cc=peff@peff.net \
    --cc=yldhome2d2@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).