On Wed, May 12, 2010 at 10:35:32AM -0400, Eli Barzilay wrote:
> But with `-p' it was doing something confusing: I used two files that
> were recently renamed, and the result was the correct log history, but
> the first patch that was shown (the rename) showed the two files as
> added. (That's even when I added `-C' and `-M'.) This happens even
> with a single path. OTOH, using `--follow' with `-p' and a single
> path without your patch produces the expected result where the first
> patch is a rename (even without `-C'/`-M').
Ah, yeah, I see. The problem is that my code is doing something like:
1. Do a sha1-only diff with our current path list.
2. If there were any created files, they might be renames.
Put aside the old diff results. Do a new diff, looking for renames.
3. If there are renames, add them to our path list.
4. Restore the old diff results.
5. Proceed with other desired diff options (rename detection, showing
patches, etc).
But the during step (5), remember that we are still working with the old
diff results, which will not include the expanded path. Thus we won't
consider the new path as a rename source, and will fail to find the
rename.
The naive right way would be to re-do step (1) with the expanded path.
But there is an optimization, since we can use the diff results from (2)
directly, including avoiding re-doing the rename detection.
The only "downside" is that it means --follow actually impacts the diff
generation by implying --find-copies-harder. And I put downside in
quotes because it is probably not a big deal. We have already spent the
CPU time to find the answer, so it is silly not to show it. I can't
imagine why somebody would want --follow, but would _not_ want rename
detection in the resulting diff.
Bo's version of the patch does that optimization. When I clean up the
patch (probably sometime next week), I'll take those changes.
-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