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

["Giuseppe Bilotta" <giuseppe.bilotta@gmail.com>]

On Thu, Oct 2, 2008 at 5:34 PM, Petr Baudis <pasky@suse.cz> wrote:

> Just nits...
>> +             $action  = undef;
>
> Extra whitespace.

Right, fixed.

>> +     }
>> +
>>       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.

I put the action/ part in square brackets. (e.g.
project.git/[action/]branch:filename). Is this good enough?

>> @@ -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?

The purpose of what? setting both $hash and $hash_base was something
that I found was needed in some extreme cases, as discussed with
Jakub. Proposals for recommended cleaner but equally fast way to
handle it. If you're referring to the whitespace, I was just lining up
the entries. Should I do it in a separate patch?

-- 
Giuseppe "Oblomov" Bilotta