Re: [PATCH] gitweb: snapshot cleanups & support for offering multiple formats

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

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