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

[Jakub Narebski <jnareb@gmail.com>]

On Tue, 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.

_Perhaps_ it should be noted that even though gitweb accepted
'project/hash' and 'project/hash_base:file_name' path_info URLs, it
generated links with only 'project/' in path_info.

> 
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
>  gitweb/gitweb.perl |   32 +++++++++++++++++++++++++++++---
>  1 files changed, 29 insertions(+), 3 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index ec4326f..2c380ac 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -687,14 +687,40 @@ sub href (%) {
>  
>  	my ($use_pathinfo) = gitweb_check_feature('pathinfo');
>  	if ($use_pathinfo) {
> -		# use PATH_INFO for project name
> +		# try to put as many parameters as possible in PATH_INFO:
> +		#   - project name
> +		#   - action
> +		#   - hash or hash_base:filename
> +
> +		# Strip any trailing / from $href, or we might get double
> +		# slashes when the script is the DirectoryIndex

Perhaps example, like $href='gitweb.example.com/', could be put here.

> +		#

I think that we can lose this empty line comment here.

> +		$href =~ s,/$,,;
> +
> +		# Then add the project name, if present
>  		$href .= "/".esc_url($params{'project'}) if defined $params{'project'};
>  		delete $params{'project'};
>  
> -		# Summary just uses the project path URL
> -		if (defined $params{'action'} && $params{'action'} eq 'summary') {
> +		# Summary just uses the project path URL, any other action is
> +		# added to the URL
> +		if (defined $params{'action'}) {
> +			$href .= "/".esc_url($params{'action'}) unless $params{'action'} eq 'summary';
>  			delete $params{'action'};
>  		}

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

Hmmmm...

Shouldn't the code first check for $file_name, then add either 
"$hash_base:$file_name" (url-escaped), or "$hash" (not "$hash_base")?

>  	}
>  
>  	# now encode the parameters explicitly

Thank you very much for work on improving gitweb.

-- 
Jakub Narebski
Poland