On Thu, 16 Oct 2008, 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,
> $project/$branch[:$filename], except when it was used to access a branch
> whose name matches a gitweb action.
...in which case old code would access branch, and new code would treat
name as action. This shouldn't matter in practice, unless there are
branches named exactly as gitweb actions.
[But I think current commit description is enough. Just in case...]
>
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@xxxxxxxxx>
I like it. For what it is worth:
Acked-by: Jakub Narebski <jnareb@xxxxxxxxx>
> ---
> gitweb/gitweb.perl | 37 +++++++++++++++++++++++++++++++------
> 1 files changed, 31 insertions(+), 6 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index c5254af..6d0dc26 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -534,23 +534,48 @@ sub evaluate_path_info {
> return if $input_params{'action'};
> $path_info =~ s,^\Q$project\E/*,,;
>
> + # next, check if we have an action
> + my $action = $path_info;
> + $action =~ s,/.*$,,;
> + if (exists $actions{$action}) {
> + $path_info =~ s,^$action/*,,;
> + $input_params{'action'} = $action;
> + }
> +
> + # list of actions that want hash_base instead of hash
...and can have no file_name ('f') parameter.
> + my @wants_base = (
> + 'tree',
> + 'history',
> + );
I like this solution. It makes path_info URL unambiguous: it is
<project>/log/<hash> (where <hash> is usually refname), and
<project>/tree/<hash_base> which is just shortcut for <project>/tree/<hash_base>:/
It means that the @wants_base list contains actions which require / use
$hash_base and $file_name, but $file_name might be not set, which denotes
top directory (root tree).
> +
> my ($refname, $pathname) = split(/:/, $path_info, 2);
> if (defined $pathname) {
> - # we got "project.git/branch:filename" or "project.git/branch:dir/"
> + # we got "branch:filename" or "branch:dir/"
> # we could use git_get_type(branch:pathname), but it needs $git_dir
Well, this comment was there... but actually I think we avoid
git_get_type("branch:pathname") because currently it is additional fork.
And with convention that filename part of path_info ends in '/' for
directories (quite sensible I think) it is not needed. Moreso after
next patch in series, when we state action explicitly, and the only
difference _might_ be for 'history' view (which is for directories
and for files both).
> $pathname =~ s,^/+,,;
> if (!$pathname || substr($pathname, -1) eq "/") {
> - $input_params{'action'} = "tree";
> + $input_params{'action'} ||= "tree";
> $pathname =~ s,/$,,;
> } else {
> - $input_params{'action'} = "blob_plain";
> + $input_params{'action'} ||= "blob_plain";
> }
> $input_params{'hash_base'} ||= $refname;
> $input_params{'file_name'} ||= $pathname;
> } elsif (defined $refname) {
> - # we got "project.git/branch"
> - $input_params{'action'} = "shortlog";
> - $input_params{'hash'} ||= $refname;
> + # we got "branch". in this case we have to choose if we have to
^ ^
|-|-- ??? (typo?)
> + # set hash or hash_base.
> + #
> + # Most of the actions without a pathname only want hash to be
> + # set, except for the ones specified in @wants_base that want
> + # hash_base instead. It should also be noted that hand-crafted
> + # links having 'history' as an action and no pathname or hash
> + # set will fail, but that happens regardless of PATH_INFO.
Ah, I see that the comment about what makes action to be put into
@wants_base is here.
> + $input_params{'action'} ||= "shortlog";
> + if (grep {$input_params{'action'} eq $_} @wants_base) {
Style: I would use here (but perhaps it is just me) for better
readibility
+ if (grep { $input_params{'action'} eq $_ } @wants_base) {
> + $input_params{'hash_base'} ||= $refname;
> + } else {
> + $input_params{'hash'} ||= $refname;
> + }
> }
> }
> evaluate_path_info();
> --
> 1.5.6.5
>
>
--
Jakub Narebski
Poland
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
