Re: [RFCv4 1/3] gitweb: add patch view

From: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Date: Dec 15, 2008, 13:48 UTC
Message-ID: <cb7bb73a0812150548w526a8c0eu13ec95785e0ab824@mail.gmail.com>
In-Reply-To: <200812151417.16372.jnareb@gmail.com>

On Mon, Dec 15, 2008 at 2:17 PM, Jakub Narebski <jnareb@gmail.com> wrote:

Show 8 quoted lines

>> +     my $patch_max;
>> +     if ($format eq 'patch') {
>> +             $patch_max = gitweb_check_feature('patches');
>> +             die_error(403, "Patch view not allowed") unless $patch_max;
>> +     }
>> +
>
> Hmmm... gitweb_check_feature

You're right, it's an abuse. I'll make it gitweb_get_feature()[0]

> I am wondering if we could somehow mark (encode) either $hash_parent
> or number of patches in proposed filename... but I don't think it is
> worth it.

Including hash_parent if defined is quite possible. I'm not sure it's really worth it considering that the typical usage would be to publish a patchset for a particular feature (in which case the hash/branch name would be enough).

Show 10 quoted lines

>> +     } elsif ($format eq 'patch') {
>> +             local $/ = undef;
>> +             print <$fd>;
>> +             close $fd
>> +                     or print "Reading git-format-patch failed\n";
>
> Nice, although... I'd prefer for Perl expert to say if it is better
> to dump file as a whole in such way (it might be quite large), or
> to do it line by line, i.e. without "local $/ = undef;", or using
> "print while <$fd>;" also without "local $/ = undef;".

I'm just sticking to whatever the existing code does :-)

As soon as you finish the patchset review I'll have a new version taking into consideration all the other suggestions and remarks I snipped from this reply.

-- 
Giuseppe "Oblomov" Bilotta