From: Jakub Narebski <jnareb@gmail.com>
Date: Sun, 05 Oct 2008 08:19:39 GMT
Subject: Re: [PATCHv4] gitweb: parse parent..current syntax from pathinfo
Message-ID: <200810051019.39702.jnareb@gmail.com>
In-Reply-To: <200810040331.27605.jnareb@gmail.com>

On Sun, 4 Oct 2008, Jakub Narebski wrote:
> On Thu, 2 Oct 2008, Giuseppe Bilotta wrote:
 
> > diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> > index 3e5b2b7..89e360f 100755
> > --- a/gitweb/gitweb.perl
> > +++ b/gitweb/gitweb.perl
> > @@ -534,7 +534,9 @@ if ($path_info && !defined $action) {
> >  
> >  # we can now parse ref and pathnames in PATH_INFO
> >  if ($path_info) {
> > -     my ($refname, $pathname) = split(/:/, $path_info, 2);
> > +     $path_info =~ /^((.+?)(:(.+))?\.\.)?(.+?)(:(.+))?$/;
> > +     my ($parentrefname, $parentpathname, $refname, $pathname) = (
> > +             $2, $4, $5, $7);
> 
> Style: I would use (but that is perhaps matter of taste)
> 
> +       my ($parentrefname, $parentpathname, $refname, $pathname) =
> +               ($2, $4, $5, $7);
> 
> Also it would be I think simpler to use instead non-catching grouping,
> i.e. (?: xxx ) extended pattern (see perlre(1)), and use 
> ($1, $2, $3, $4), or even simpler  'list = (string =~ regexp)'  form.  

Alternate solution would be to use split(..., ..., 2), but I think you
got the regular expression right.  Just mentioning...

-- 
Jakub Narebski
Poland