Re: [PATCHv4] gitweb: generate project/action/hash URLs

[Jakub Narebski <jnareb@gmail.com>]

On Fri, 3 October 2008, Giuseppe Bilotta wrote:
> On Fri, Oct 3, 2008 at 3:48 AM, Jakub Narebski <jnareb@gmail.com> wrote:
>> On Tue, 2 Oct 2008, Giuseppe Bilotta wrote:

>>> +             # Finally, we put either hash_base:file_name or hash
>>> +             if (defined $params{'hash_base'}) {
>>> +                     $href .= "/".esc_url($params{'hash_base'});
>>> +                     if (defined $params{'file_name'}) {
>>> +                             $href .= ":".esc_url($params{'file_name'});
>>> +                             delete $params{'file_name'};
>>> +                     }
>>> +                     delete $params{'hash'};
>>> +                     delete $params{'hash_base'};
>>> +             } elsif (defined $params{'hash'}) {
>>> +                     $href .= "/".esc_url($params{'hash'});
>>> +                     delete $params{'hash'};
>>> +             }
>>
>> Hmmmm...
>>
>> Shouldn't the code first check for $file_name, then add either
>> "$hash_base:$file_name" (url-escaped), or "$hash" (not "$hash_base")?
> 
> Hm, your idea is probably better indeed, if we can ensure that
> $file_name is always set for generated links that need $hash_base (I
> mean, as opposed to the root tree case we were discussing for the
> first patch).

Hmm... I'm just worrying here about diluting meaning of params passed
via path_info.  We had either project/hash, or project/hash_base:file_name;
now the hashy parameter in path_info can be hash or hash_base and we
don't know which.  But perhaps I am worrying over nothing...

A short comment though would be nice (that we can have $hash or
$hash_base for case without $file_name)
-- 
Jakub Narebski
Poland