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

[Junio C Hamano <gitster@pobox.com>]

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"?

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

> @@ -767,7 +785,7 @@ our $git_dir;
>  $git_dir = "$projectroot/$project" if $project;
>  
>  # list of supported snapshot formats
> -our @snapshot_fmts = gitweb_check_feature('snapshot');
> +our @snapshot_fmts = gitweb_get_feature('snapshot');
>  @snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);

And this may be a good change from that point of view, but...

> @@ -810,7 +828,7 @@ sub href (%) {
>  		}
>  	}
>  
> -	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.