Re: [PATCHv6 2/5] gitweb: generate project/action/hash URLs

[Jakub Narebski <jnareb@gmail.com>]

On Thu, 16 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.
> 
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>

Nice, well commented, and clean.

Acked-by: Jakub Narebski <jnareb@gmail.com>

> ---
>  gitweb/gitweb.perl |   33 ++++++++++++++++++++++++++++++---
>  1 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 6d0dc26..5337d40 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -724,14 +724,41 @@ 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
> +
> +		# When the script is the root DirectoryIndex for the domain,
> +		# $href here would be something like http://gitweb.example.com/
> +		# Thus, we strip any trailing / from $href, to spare us double
> +		# slashes in the final URL
> +		$href =~ s,/$,,;

I like having this comment here.

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

Which meant that we prefer http://base_url/project over http://base_url/project/summary

> +		if (defined $params{'action'}) {
> +			$href .= "/".esc_url($params{'action'}) unless $params{'action'} eq 'summary';
>  			delete $params{'action'};
>  		}
> +
> +		# 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'};
> +		}
>  	}
>  
>  	# now encode the parameters explicitly
> -- 
> 1.5.6.5
> 
> 

-- 
Jakub Narebski
Poland