From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Jake Zimmerman <jake@zimmerman.io>,
Lidong Yan <yldhome2d2@gmail.com>,
git@vger.kernel.org
Subject: Re: Regression in `git diff --quiet HEAD` when a new file is staged
Date: Wed, 22 Oct 2025 09:48:37 -0700 [thread overview]
Message-ID: <xmqqikg6zxui.fsf@gitster.g> (raw)
In-Reply-To: <20251022091112.GB853931@coredump.intra.peff.net> (Jeff King's message of "Wed, 22 Oct 2025 05:11:12 -0400")
Jeff King <peff@peff.net> writes:
> On Tue, Oct 21, 2025 at 07:38:03AM -0700, Junio C Hamano wrote:
>
>> > So really, the regression fix should probably cover both of them (which
>> > it would if we move the /dev/null redirection into the flush_quietly()
>> > variant).
>>
>> Do you mean something like this on top of your patch for 'maint',
>> and the latest from Lidong to the 'master' front, then?
>
> Yep, exactly (though with the "o->file" restoration that Lidong
> pointed out).
>
>> Having calls to this helper in two loops in one function looks a bit
>> awkward but the conditions to enter these two loops are mutually
>> exclusive, so it is not like we can remember the result of the calls
>> we make in the first loop and reuse in the second loop, so this
>> probably is the best we can do.
>
> Yeah. I suspect there is some formulation along the lines of: if we have
> diff_from_contents set but are not looking at a content-level diff, then
> up-front in diff_flush() we should quietly flush each to find out what
> is changed and what is not. But the loop for NAME_STATUS, etc, needs to
> know _which_ pairs still had changes (whereas --quiet only cares about
> whether there were any changes at all). So we'd have to store that
> somewhere.
>
> And of course the chance of regressing some unconsidered corner case is
> high. Definitely not something we should entertain while doing another
> regression fix. ;)
Of course. The "redirect inside flush_quietly()" change by itself
is turning out to be tricky enough for the other caller of the
helper.
Here is what I have on top of your patch right now, after ditching
the idea to move the redirect to flush_quietly() because it would
mean redirecting N times for a N-path patch, but one thing that is
frustrating is that I cannot come up with a scenario or test in
which it makes a difference to this other caller if we forget to
restore o->file member.
diff --git c/diff.c i/diff.c
index 9b8d658b9e..ceb57d1ef8 100644
--- c/diff.c
+++ i/diff.c
@@ -6814,6 +6814,16 @@ void diff_flush(struct diff_options *options)
DIFF_FORMAT_NAME |
DIFF_FORMAT_NAME_STATUS |
DIFF_FORMAT_CHECKDIFF)) {
+ /*
+ * make sure diff_Flush_patch_quietly() to be silent.
+ */
+ FILE *saved_file = options->file;
+ int saved_color_moved = options->color_moved;
+
+ if (options->flags.diff_from_contents) {
+ options->file = xfopen("/dev/null", "w");
+ options->color_moved = 0;
+ }
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
@@ -6826,6 +6836,11 @@ void diff_flush(struct diff_options *options)
flush_one_pair(p, options);
}
+ if (options->flags.diff_from_contents) {
+ fclose(options->file);
+ options->file = saved_file;
+ options->color_moved = saved_color_moved;
+ }
separator++;
}
next prev parent reply other threads:[~2025-10-22 16:48 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-17 0:09 Regression in `git diff --quiet HEAD` when a new file is staged Jake Zimmerman
2025-10-17 7:51 ` Jeff King
2025-10-17 8:36 ` [PATCH] diff: restore redirection to /dev/null for diff_from_contents Jeff King
2025-10-17 18:22 ` Junio C Hamano
2025-10-19 21:09 ` Johannes Schindelin
2025-10-21 7:52 ` Jeff King
2025-10-17 11:44 ` Regression in `git diff --quiet HEAD` when a new file is staged Johannes Schindelin
2025-10-17 17:45 ` Junio C Hamano
2025-10-18 1:04 ` Lidong Yan
2025-10-18 9:42 ` Jeff King
2025-10-18 9:40 ` Jeff King
2025-10-18 15:23 ` Junio C Hamano
2025-10-21 7:36 ` Jeff King
2025-10-21 14:38 ` Junio C Hamano
2025-10-22 4:46 ` Lidong Yan
2025-10-22 9:14 ` Jeff King
2025-10-22 14:20 ` Lidong Yan
2025-10-22 14:31 ` Junio C Hamano
2025-10-22 16:28 ` Junio C Hamano
2025-10-22 9:11 ` Jeff King
2025-10-22 16:48 ` Junio C Hamano [this message]
2025-10-23 12:01 ` Jeff King
2025-10-23 12:15 ` Jeff King
2025-10-23 13:35 ` Junio C Hamano
2025-10-22 17:39 ` Junio C Hamano
2025-10-23 0:33 ` Lidong Yan
2025-10-23 13:42 ` Junio C Hamano
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=xmqqikg6zxui.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jake@zimmerman.io \
--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).