On Wed, Dec 09, 2020 at 04:25:18PM -0800, Elijah Newren wrote: > On Wed, Dec 9, 2020 at 2:40 PM Taylor Blau <me@xxxxxxxxxxxx> wrote: > > > Technically, there is one difference in the end result for the code. IF > > > > s/IF/If ? > > Indeed; will fix. :-). In fairness, I did read it with _emphasis_, so it at least gave me a laugh! > > > the caller of diffcore-rename provides a set of pairs with a duplicate > > > filename in the sources (an erroneous input condition), the old code > > > would simply ignore the duplicate (without warning or error). The new > > > code will simply swallow it and thus allow multiple pairings for the > > > "same" source file. Checking for this error condition is expensive > > > (basically undoing the optimization) and the only existing callers > > > already avoid passing such an invalid input. If someone really wants to > > > add some error checking here on this input condition, I believe they > > > should add a separate function which can be called before > > > diffcore_rename(), and then other callers that don't need additional > > > checks because of their design (such as merge-ort) can avoid the extra > > > overhead. > > > > It seems like this is currently impossible to trigger, making any extra > > (expensive) checking of it worthless, no? > > I believe that's what it currently amounts to, and I debated just > ripping the paragraph out. > > [ ... ] No, no need to be sorry: this is exactly the sort of discussion I had hoped to provoke by sanity checking my understanding of what you had written. I think that the paragraph does belong in the patch message, and interested readers can find our discussion here. Thanks, Taylor
