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

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 9, 2007, 23:48 UTC
Message-ID: <7vr6nht9yq.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <3bbc18d20707091552l29fb81b6v34da9cef3ec0df58@mail.gmail.com>

"Matt McCutchen" <hashproduct@gmail.com> writes:

Show 14 quoted lines

> On 7/8/07, Junio C Hamano <gitster@pobox.com> wrote:
>> Matt McCutchen <hashproduct@gmail.com> writes:
>>
>> > -sub gitweb_have_snapshot {
>> > -     my ($ctype, $suffix, $command) = gitweb_check_feature('snapshot');
>> > -     my $have_snapshot = (defined $ctype && defined $suffix);
>> > -
>> > -     return $have_snapshot;
>>
>> Although you are removing this function, you still have a couple
>> of callers left in the code.
>
> OK, I will revise the patch, submit it and see if I can get it to
> appear as a reply to this thread.

Thanks. For future reference, I caught it not by code inspection, but by running one of the tests (t9500).

> Incidentally, when only one format
> is offered, would you prefer the snapshot link to appear as
> "_snapshot_" (the same as before) or "_snapshot (tgz)_" instead of the
> "snapshot (_tgz_)" that the current patch does?

When only one format is offerred by the site, it is not like the end user has any choice, so "_snapshot_" is probably the most appropriate from the screen real-estate point-of-view.

The end user _might_ complain "Geez, I cannot grok zip, I can only expand tar. I would not have clicked the link if it said 'snapshot (_zip_)', but the stupid gitweb said '_snapshot_' and nothing else." So in that sense, we are robbing one choice the user has (i.e. "to decide not to click the link, based on the format of the data that would be given"), but I do not think that is something we would seriously want to worry about.