Re: [PATCH] combine-diff: handle --find-object in multitree code path

From: Jeff King <peff@peff.net>
Date: Sep 30, 2020, 22:46 UTC
Message-ID: <20200930224611.GC1908000@coredump.intra.peff.net>
In-Reply-To: <CAPx1Gvd7WpAgUGgZkZARY7JcFj8nbDJ6zEDTSaBt2=xR535E-g@mail.gmail.com>

On Wed, Sep 30, 2020 at 01:06:02PM -0700, Chris Torek wrote:

Show 13 quoted lines

> On Wed, Sep 30, 2020 at 4:54 AM Jeff King <peff@peff.net> wrote:
> > I'm a little nervous that the second "wart" may actually be making
> > things worse, because now we sometimes produce a wrong answer and
> > sometime a right one, and it can be difficult to know which options
> > cause which (e.g., rename detection puts us onto the slow path). Is it
> > worse to sometimes be right and sometimes wrong, or to always be
> > consistently and predictably wrong? I suppose one could even argue that
> > the current semantics aren't "wrong", but just what we happen to
> > produce. But IMHO they are so un-useful as to be considered wrong.
> 
> "Predictably wrong" *is* actually useful while "unpredictably wrong"
> is, um, "less useful".  Perhaps just documenting exactly which options
> use which path?  Basically turn some of this into documentation.

Perhaps. The thing is that I do have a use case for which I need that answer, and I need it to be reliable and fast. So right now --find-object is not useful at all, and this makes it useful for at least my narrow case.

My big concern is just making the overall system more confusing. I agree that documenting _might_ help, but we're getting pretty far into internals here. I'm not sure I'd want to expose the user to a list of "these options trigger this code path, which behaves like this". It's likely to generate more confusion than it solves, and would likely bitrot anyway.

Maybe something like this would help:

diff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt
index b10ff4caa6..f61af6af18 100644
--- a/Documentation/diff-generate-patch.txt
+++ b/Documentation/diff-generate-patch.txt
@@ -200,3 +200,7 @@ parents).  When shown by `git diff-files -c`, it compares the
 two unresolved merge parents with the working tree file
 (i.e. file1 is stage 2 aka "our version", file2 is stage 3 aka
 "their version").
+
+Note that the results of using `-c` or `--cc` with diff-filtering
+options such as `-S` or `--find-object` are currently poorly defined,
+and are subject to change.

I'd perhaps even put that in a BUGS section, but that can't be done from
an include file like this.

-Peff