On Fri, Nov 30, 2007 at 06:36:56PM -0800, Junio C Hamano wrote: > > I have always been a bit confused about diffcore-break, so I am probably > > misunderstanding what you mean. But are you saying that > > diffcore-break.c:should_break should return 1 for typechanges? > > What I had in mind was to do something like that in spirit, but instead > break such a filepair inside diffcore-rename (iow, even when the user > did not say -B) early on. Ah, I see. BTW, I totally screwed up the tests I did earlier. Returning 1 from should_break _does_ produce the same results for my simple case (copy + typechange). > But after re-reading your patch and the surrounding code, that is > more or less what you are doing (without actually recording the extra > broken pair to be merged back later). I don't think we need to, because they are never actually "broken"; we simply consider the source a candidate for renaming, but keep the pair together to note the typechange. > which is essentially doing the same thing but only for the "remove the > regular file" half. One has to wonder how the lack of handling the > other half affects the outcome and still produce a result more intuitive > than the current code. AIUI, because we never broke the pair in the first place, we don't need to look for a source for that dest (the "add a new symlink" half). It's already part of the same filepair. Whether this is by design or simply a happy accident that we record both renames and typechanges in diff_filepairs, I'm not sure. Or perhaps I'm totally misunderstanding how the breaking works. > In your test case, the "new" symlink won't have any similar symlink that > is removed from the preimage, so registering it as a rename destination > would not make a difference (it will say "no match found, so create this > as usual"), but I am not convinced if that would work well in general. I don't know that it makes a difference. We are impacting only a 'typechange', which implies that we have a filepair in which both p->one and p->two are valid; thus, the current code doesn't use it as a rename dst at all. -Peff - To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
