On Thu, Oct 2, 2008 at 5:34 PM, Petr Baudis <pasky@xxxxxxx> 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 -- 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