On Fri, 3 October 2008, Giuseppe Bilotta wrote:
> On Fri, Oct 3, 2008 at 3:48 AM, Jakub Narebski <jnareb@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
