Re: [PATCH] gitweb: Support comparing blobs (files) with different names

[mkoegler@auto.tuwien.ac.at (Martin Koegler)]

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.

If you think, its safe, I can simplify git_blobdiff. I propose
doing the following way (pseudo-code):

sub git_blobdiff {
$file_parent ||= $file_name;

if (defined $hash_base && defined $file_name && !defined $hash)
$hash=git_get_hash_by_path($hash_base,$file_name);

if (defined $hash_parent_base && defined $file_parent && !defined $hash_parent)
$hash_parent=git_get_hash_by_path($hash_parent_base,$file_parent);

if (!defined $hash || ! defined $hash_parent )
die_error(....);

$diffinfo{'from_mode'} = $diffinfo{'to_mode'} = "blob";
$diffinfo{'from_file'} = $file_parent || $hash_parent;
$diffinfo{'to_file'}   = $file_name || $hash;
$diffinfo{'status'} = '2';
$diffinfo{'to_id'}   = $hash;
$diffinfo{'from_id'}   = $hash_parent;

 open $fd, "-|", git_cmd(), "diff", '-p', @diff_opts,
    $hash_parent, $hash
    or die_error(undef, "Open git-diff failed");

/* print output */

}

Else I will keep a reworked version of my patch.

> 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.

mfg Martin Kögler