On Thu, Dec 4, 2008 at 2:48 AM, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
> On Thu, Dec 4, 2008 at 01:20, Giuseppe Bilotta wrote:
>> On Thu, Dec 4, 2008 at 12:55 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>> Giuseppe Bilotta <giuseppe.bilotta@xxxxxxxxx> writes:
>>>> +
>>>> + # The maximum number of patches in a patchset generated in patch
>>>> + # view. Set this to 0 or undef to disable patch view, or to a
>>>> + # negative number to remove any limit.
>>>> + 'patches' => {
>>>> + 'override' => 1,
>>>> + 'default' => [16]},
>
> Errr... you need something like 'sub' => \&feature_patches for override
> to actually work, I think.
Oops, right.
>> I always assumed that the disabled default was related to how invasive
>> the changes would be (to the UI or computationally-wise). As for the
>> overridability, that's actually the only reason why it would make
>> sense to put in the %feature hash ... otherwise a conf-settable
>> $patch_max (as in v2) would have been enough.
>
> Add to that the fact that this patch just adds the new view, like for
> example in the case of 'snapshot' link, which was turned on... but fact,
> it was by default not overridable. I would agree that it can be turned
> on with low limit but not overridable in introductory patch.
Ok, I'll make it non-overridable, and keep this 16 limit for starters.
Or would you suggest even lower?
>>>> sub git_commitdiff {
>>>> my $format = shift || 'html';
>>>> +
>>>> + my $patch_max = gitweb_check_feature('patches');
>>>> + if ($format eq 'patch') {
>>>> + die_error(403, "Patch view not allowed") unless $patch_max;
>>>> + }
>>>> +
>>>
>>> Should you have to pay overhead for the check-feature call even when
>>> the $format is not "patch"?
>>
>> Actually I wasn't sure if I could use my within the if block, and have
>> the value visible outside (it's used further down when picking the
>> options to pass to format-patch). And since it was used in the second
>> patch anyway to choose whether to add the 'patch' link in html view or
>> not, I just put it outside the block.
>
> You have to use _declaration_ ourside block, but assignment can happen
> inside:
>
> + my $patch_max;
> + if ($format eq 'patch') {
> + $patch_max = gitweb_check_feature('patches');
> + die_error(403, "Patch view not allowed") unless $patch_max;
> + }
Right, stupid me.
> (Side note: doesn't it uses spaces instead of tabs for align?)
I'll check.
--
Giuseppe "Oblomov" Bilotta
--
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
