Re: [PATCH] gitweb: fixes to gitweb feature check code

["Giuseppe Bilotta" <giuseppe.bilotta@gmail.com>]

On Sun, Nov 16, 2008 at 10:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
>
>> The gitweb_check_feature routine was being used for two different
>> purposes: retrieving the actual feature value (such as the list of
>> snapshot formats or the list of additional actions), and to check if a
>> feature was enabled.
>
>> +# check if a given feature is enabled or not, returning the first (and only)
>> +# value of the feature. Comfort code, allowing the use of
>> +#   my $bool_feat = gitweb_check_feature('bool_feat');
>> +# or
>> +#   gitweb_check_feature('bool_feat') or somecode;
>> +# instead of
>> +#   my ($bool_feat) = gitweb_git_feature('bool_feat');
>> +# or
>> +#   (gitweb_check_feature('bool_feat'))[0] or somecode;
>> +# respectively
>
> What's "Comfort code"?

Code that provides comfort? 8-D

> I'd agree that introduction of gitweb_get_feature() may help avoiding
> mistakes at the call sites for Perl illiterates like myself.

>> -     my ($use_pathinfo) = gitweb_check_feature('pathinfo');
>> +     my $use_pathinfo = gitweb_check_feature('pathinfo');
>
> ... I do not think changes like these are warranted.  They have been using
> the function _correctly_ by calling it in the list context and I think
> they will continue to work with your patch.

Because the returned scalar value gets properly promoted to array?
Maybe, but I would say that keeping the () is confusing and
inconsistent. After all, the purpose of the patch is to _eliminate_
(the need for) such constructs.

-- 
Giuseppe "Oblomov" Bilotta