Re: [PATCHv4] gitweb: parse project/action/hash_base:filename PATH_INFO

[Petr Baudis <pasky@suse.cz>]

On Thu, Oct 02, 2008 at 02:10:29AM +0200, Giuseppe Bilotta wrote:
> This patch enables gitweb to parse URLs with more information embedded
> in PATH_INFO, reducing the need for CGI parameters. The typical gitweb
> path is now $project/$action/$hash_base:$file_name or
> $project/$action/$hash
> 
> This is mostly backwards compatible with the old-style gitweb paths,
> except when $project/$branch was used to access a branch whose name
> matches a gitweb action.
> 
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>

Just nits...

> ---
>  gitweb/gitweb.perl |   90 +++++++++++++++++++++++++++++++---------------------
>  1 files changed, 54 insertions(+), 36 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index e7e4d6b..f088681 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -519,9 +550,19 @@ sub evaluate_path_info {
>  	# do not change any parameters if an action is given using the query string
>  	return if $action;
>  	$path_info =~ s,^\Q$project\E/*,,;
> +
> +	# next comes the action
> +	$action = $path_info;
> +	$action =~ s,/.*$,,;
> +	if (exists $actions{$action}) {
> +		$path_info =~ s,^$action/*,,;
> +	} else {
> +		$action  = undef;

Extra whitespace.

> +	}
> +
>  	my ($refname, $pathname) = split(/:/, $path_info, 2);
>  	if (defined $pathname) {
> -		# we got "project.git/branch:filename" or "project.git/branch:dir/"
> +		# we got "project.git/action/branch:filename" or "project.git/action/branch:dir/"
>  		# we could use git_get_type(branch:pathname), but it needs $git_dir
>  		$pathname =~ s,^/+,,;
>  		if (!$pathname || substr($pathname, -1) eq "/") {

But the old variant is still possible, maybe the comments should
indicate that the action/ part is optional.

> @@ -534,8 +575,9 @@ sub evaluate_path_info {
>  		$file_name ||= validate_pathname($pathname);
>  	} elsif (defined $refname) {
>  		# we got "project.git/branch"
> -		$action ||= "shortlog";
> -		$hash   ||= validate_refname($refname);
> +		$action    ||= "shortlog";
> +		$hash      ||= validate_refname($refname);
> +		$hash_base ||= validate_refname($refname);
>  	}
>  }
>  evaluate_path_info();

What is this good for?

-- 
				Petr "Pasky" Baudis
People who take cold baths never have rheumatism, but they have
cold baths.