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

>>> 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.
> 
> If we need the correct mode in %diffinfo, this is not difficult:
> 
> $from_mode="blob";
> if (defined $hash_base && defined $file_name)
> ($to_mode,$hash)=git_get_hash_by_path($hash_base,$file_name);

>>>> 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 (hash) 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).
> 
> The <mode> must be discarded, as it is wrong for anything which as not
> a mode of 100644, as we specify a two blob as parameter to git-diff.

>> But I think it is better left for later patch.
> 
> As git_patchset_body requires an information about the compared file
> as parameter, a new formating function will be needed. I'm not sure,
> if the overhead of git_get_hash_by_path is this worth. Additionally,
> if we have the hash passed by parameter in most cases, there is no
> need to call it in these cases.

>> diff --git a/x b/x
>> deleted file mode 100644 (file)
>> index 190a180..873fb8d
>> --- a/x
>> +++ /dev/null
>> @@ -1 +0,0 @@
>> -123
>> diff --git a/ b/
>> new file mode 120000 (symlink)
>> index 190a180..873fb8d
>> --- /dev/null
>> +++ b/
>> @@ -0,0 +1 @@
>> +file3
>> \ No newline at end of file
> 
> This was generated by "diff to current" in the history view of a file,
> which was changed between symlink and normal file.

> mfg Martin Kögler
> PS:
> I created a blob with a "strange" filename: &()=!"§$%[<>]*#+_;.
> In the result of the blob view, the " is not escaped in the filename in the header
> and a strange content type is returned:
> 
> $ telnet localhost 80
> Trying 127.0.0.1...
> Connected to localhost.
> Escape character is '^]'.
> GET /gitweb/gitweb.cgi?p=t/.git;a=blob;f=%26()%3D%21%22%A7%24%25%5B%3C%3E%5D%2A%23%2B_%3B.;hb=7bfed2588bee66b33db544830606fa6606478fd9 HTTP/1.0
> 
> HTTP/1.1 200 OK
> Date: Wed, 28 Mar 2007 19:55:36 GMT
> Server: Apache
> Content-disposition: inline; filename="&()=!"§$%[<>]*#+_;."
> Expires: Thu, 29 Mar 2007 19:55:39 GMT
> Connection: close
> Content-Type: application/vnd.mif
> 
> xx