Re: [PATCH 3/3] diffcore-rename: fall back to -C when -C -C busts the rename limit

From: Jeff King <peff@peff.net>
Date: Mar 23, 2011, 16:50 UTC
Message-ID: <20110323165040.GA4068@sigill.intra.peff.net>
In-Reply-To: <7vfwqdg574.fsf@alter.siamese.dyndns.org>

On Wed, Mar 23, 2011 at 09:41:19AM -0700, Junio C Hamano wrote:

Show 5 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > "find the highest limit needed and report once" strategy you used above?
> 
> Wasn't "find the highest limit" your invention in merge-recursive?  The

Hmm, you're right. I was thinking the code you were modifying was called per-diff, but it's not. Though looking at it again, I think we could technically still warn several times, one for each recursive call to merge_recursive. Your call to show() does fix that (because it checks o->call_depth).

So I think the code after your patch does the right thing.

> As to the warnings in "log" output, I actually prefer leaving saved_* out
> and showing the warning per internal diff invocation. My initial iteration
> was indeed coded that way, and I did the "find the highest" only to mimic
> what was already in merge-recursive.

I think they are two different cases, because the user never gets to see the intermediate results of merge-recursive. That is, at the end of the merge we tell them "here's the result, and by the way, we limited renames." But for something like "log" or "diff-tree --stdin", it is about doing N independent diffs, and the user gets to see the result of each. So if we can match the warnings to the actual diffs in the output, that is much more useful.

But in the case of something like:

  git rev-list HEAD | git diff-tree --stdin >foo.out

I don't think there is a way to match the warnings to their respective commits. They are on two different streams, and putting the warning to stdout would pollute the output.

-Peff