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

On Fri, 3 October 2008, Giuseppe Bilotta wrote:

On Fri, Oct 3, 2008 at 3:48 AM, Jakub Narebski [off-list ref] wrote:

quoted

On Tue, 2 Oct 2008, Giuseppe Bilotta wrote:

quoted

quoted

+             # 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