On Sun, Nov 16, 2008 at 10:11 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Giuseppe Bilotta <giuseppe.bilotta@xxxxxxxxx> 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
--
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
