Re: [PATCHv4] gitweb: parse project/action/hash_base:filename PATH_INFO
- From
Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
- Date
- Oct 2, 2008, 19:30 UTC
- Message-ID
- <cb7bb73a0810021230u2ec512c0l577b3146cffccb3e@mail.gmail.com>
- In-Reply-To
- <20081002153403.GQ10360@machine.or.cz>
On Thu, Oct 2, 2008 at 5:34 PM, Petr Baudis <pasky@suse.cz> wrote:
> Just nits... >> + $action = undef; > > Extra whitespace.
Right, fixed.
Show 12 quoted lines
>> + }
>> +
>> 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?
Show 14 quoted lines
>> @@ -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