From: Matt McCutchen <hashproduct@gmail.com>
Date: Sun, 22 Jul 2007 15:05:51 GMT
Subject: Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats
Message-ID: <3bbc18d20707220805hd95c4ccsc48f140888403391@mail.gmail.com>
In-Reply-To: <7vd4ylt3eh.fsf@assigned-by-dhcp.cox.net>

On 7/22/07, Junio C Hamano <gitster@pobox.com> wrote:
> Thanks.  Will apply.

Jakub, thanks for picking this up.  I was running out of energy to
push what began as an offer of a possibly useful local customization
any further toward adoption.

That said: the backward compatibility code for gitweb _site_
configuration is broken because it is inside an if statement that only
runs for gitweb _repository_ configuration.  I tested it with a
gitweb_config.perl with

$feature{'snapshot'}{'default'} = ['x-gzip', 'gz', 'gzip'];
$feature{'snapshot'}{'override'} = 0;

and gitweb generated "snapshot ( )" (no visible link).  Inspection of
the source revealed links to "sf=x-gzip", "sf=gz", and "sf=gzip" with
empty strings for the link text.  The expected behavior is
"_snapshot_", linking to "sf=tgz", with a tooltip indicating "tar.gz"
format.

Moving the code out of the `if' wouldn't solve the problem by itself
because feature_snapshot is only reached if override is enabled, but
the site configuration compatibility needs to work whether or not
override is enabled.  That's why Junio was considering adding a
separate function gitweb_bc_feature_snapshot.  I think a nicer
alternative would be to change gitweb_check_feature to call the sub
(if any) whether or not override is enabled and let the sub check
$feature{'foo'}{'override'} itself to decide whether to look for an
override.  Then each feature_* sub is the central point for both
override processing and backward compatibility for that feature.

Matt