Re: [PATCH v2] diff: disable rename detection with --quiet

On Sun, Nov 09, 2025 at 05:43:36PM +0100, René Scharfe wrote:

Detecting renames and copies improves diff's output.  This effort is
wasted if we don't show any.  Disable detection in that case.

This actually fixes the error code when using the options --cached,
--find-copies-harder, --no-ext-diff and --quiet together:
run_diff_index() indirectly calls diff-lib.c::show_modified(), which
queues even non-modified entries using diff_change() because we need
them for copy detection.  diff_change() sets flags.has_changes, though,
which causes diff_can_quit_early() to declare we're done after seeing
only the very first entry -- way too soon.

Using --cached, --find-copies-harder and --quiet together without
--no-ext-diff was not affected even before, as it causes the flag
flags.diff_from_contents to be set, which disables the optimization
in a different way.

This makes sense to me, and I can't think of a reason why you would want
rename detection on if we're not going to show the results (and likewise
I can't think of a way that a rename result would affect has_changes).

I wonder if we should _also_ take the hunk from v1 that teaches
can_quit_early() to avoid triggering when copy detection is on. It's
probably redundant now, but it feels to me like that's the place where
the correctness check should kick in. And the patch here is just
optimizing out the unnecessary work, but also happens to align things
for correctness downstream.

But I dunno. Maybe a check for a condition that we think can never be
triggered becomes too confusing for later maintenance.


You don't say in the commit message when this bug started. I briefly
wondered if it was caused by the recent diff_from_contents stuff we've
been discussing. But it's the opposite here (the bug happens when we
_don't_ set diff_from_contents). And I think it goes all the way back to
b4194828dc (diff-index --quiet: learn the "stop feeding the backend
early" logic, 2011-05-31).

-Peff