git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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++;
 	}
 


  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).