On Thu, Mar 27, 2007, Martin Koegler wrote: > On Tue, Mar 27, 2007 at 01:56:24AM +0100, Jakub Narebski wrote: >> Martin Koegler wrote: >>> My idea is, that if I got hb:f and hpb:fp, the user exactly specified >>> the blobs to be compared. Then I don't want any guessing logic. >> >> I'd rather you reuse the "no hash_parent" code, which also hand-crafts >> diffinfo. Perhaps removing "git-diff-tree hpb hb -- f" code entirely. >> Besides, code dealing with "blobdiff" coming from "commit", "commitdiff" >> and "history" views are tested to work as expected, not so with >> arbitrary diffs. > > I don't like the whole rename detection code, so I offer to simplify > git_blobdiff. For all calls to git_blobdiff (except those from git_history), > I'm sure, that I can assume $file_parent ||= $file_name. That was the idea. Perhaps I haven't said it clearly, but I wanted to suggest to remove the whole git-diff-tree code, and use git-diff to generate diff between blobs. > If you think, its safe, I can simplify git_blobdiff. I propose > doing the following way (pseudo-code): > $file_parent ||= $file_name; [...] > $hash=git_get_hash_by_path($hash_base,$file_name); [...] > $hash_parent=git_get_hash_by_path($hash_parent_base,$file_parent); [...] > open $fd, "-|", git_cmd(), "diff", '-p', @diff_opts, > $hash_parent, $hash > or die_error(undef, "Open git-diff failed"); [...] > Else I will keep a reworked version of my patch. The trouble with this is that we may lose mode change (symlink to ordinary file etc.) because we hand-generate %diffinfo. >> By the way, if you call git_get_hash_by_path (which is expensive, as it >> calls git command), you can use resulting hash in place of >> hash_base:filename as an argument to git-diff. > > I must check, if we need to resolve $hash ($hash_parent) by > git_get_hash_by_path, if we construct it out of $hash_base and > $file_name. Maybe we can avoid this call. We can use "$hash_base:$file_name" as second parameter to git-diff etc., but I don't think we want to create links with "$hash_base:$file_name" instead of sha-1 id of a blob as 'h' parameter. It can be first implementation, thought, and later we can try to use "index <hash>..<hash> <mode>" lines from extended header to get $hash and $hash_parent (with exception of pure rename, but then we need only one invocation of git_get_hash_by_path subroutine). But I think it is better left for later patch. -- Jakub Narebski Poland - 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