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

[Jakub Narebski <jnareb@gmail.com>]

On Thu, 2 Oct 2008, Giuseppe Bilotta wrote:

> When generating path info URLs, reduce the number of CGI parameters by
> embedding action and hash_parent:filename or hash in the path.

I think this is good.

>---

> +		# 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'};
> +		}
>  	}

That I'm not sure about, both the layout of conditional (shouldn't
we check $file_name first), and the fact that we remove parameter
which is not passed, and can be even not recoverable (for example
both 'hash' and 'hash_base' set, 'hash' != 'hash_base', and
'file_name' not set).  So the code above probably has some strange
corner cases, but I guess it wouldn't be triggered by links generated
by gitweb.

But I guess that is "good enough", especially that 'tree' and 'history'
action links can have 'file_name' unset if they refer to top tree, and
they still need 'hash_base'.

-- 
Jakub Narebski
Poland