Martin Koegler wrote:
> On Mon, Mar 26, 2007 at 06:12:27PM +0100, Jakub Narebski wrote:
>> On Sun, Mar 25, 2007, Martin Koegler wrote:
>>> +sub git_treediff {
>>> + my $format = shift || 'html';
>>> + my $from = $file_parent || "";
>>> + my $to = $file_name || "";
>>
>> I'd use $file_name || ''; here, and of course
>>
>> + my $from = $file_parent || $file_name || '';
>>
>> The unwritten rule is that we use 'fp' parameter (thet $file_parent
>> variable is set) only
>
> How do I specifiy the root tree (=tree in commit) with hpb/fp, as fp
> can not be empty, but only undefined?
As I said in previous message, we can simply relax check for 'f' and 'fp'
parameters, by changing
!validate_pathname($file_name)
to
!defined validate_pathname($file_name)
Still, there are some places where we assume that 'f' and 'fp' cannot be
empty, like in above proposal. It would be:
+ my $from = (defined $file_parent ? $file_parent : $file_name) || '';
Again, I don't want to loose the assumption that we generate 'fp' _only_
if it is different from 'f'. Otherwise old links would cease working,
which breaks backwards-compatibility, and is not cool. "Cool UR_is don't
change."
>>> +
>>> + if (!defined $hash) {
>>> + if (!defined $hash_base) {
>>> + die_error('','tree parameter missing');
>>
>> This conflicts with the coding style used elsewhere in the gitweb
[...]
>> Examples:
>> die_error(undef, "Couldn't find base commit");
[...]
> I didn't know this. I'll change this.
What's strange in other place you used die_error accoring to coding
guidelines.
>>> +
>>> + } elsif ($format eq 'plain') {
>>> + my $filename = basename($project) . "-diff.patch";
>>> +
>>
>> In "commitdiff_plain" view we use
>>
>> my $filename = basename($project) . "-$hash.patch";
>>
>> Perhaps we should use the same: "-diff.patch" does not make much sense.
>> Is it a typo?
>
> No. What unique name do you propose? It needs to include $hash and $hase_parent.
> In this case, $hash could be $hash_base:$file_name, I'm not sure, how to escape
> $file_name.
For "commitdiff" case sole $hash is also not unique. But if not -hash,
then perhaps -treediff instead of simply -diff?
--
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