Re: [PATCH 3/4] gitweb: Simplify snapshot format detection logic in evaluate_path_info

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

On Mon, May 11, 2009 at 7:42 PM, Jakub Narebski <jnareb@gmail.com> wrote:
> This issue was caught by perlcritic in harsh severity level noticing
> that catch variable was used outside conditional thanks to the
> Perl::Critic::Policy::RegularExpressions::ProhibitCaptureWithoutTest
> policy.  See "Perl Best Practices", chapter 12. Regular Expressions,
> section 12.15. Captured Values:
>
>   Pattern matches that fail never assign anything to $1, $2, etc.,
>   nor do they leave those variables undefined. After an unsuccessful
>   pattern match, the numeric capture variables remain exactly as they
>   were before the match was attempted.
>
> New version is in my opinion much easier to understand; previous
> version worked correctly due to the fact that we returned from loop
> on first found match.
>
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> This is not the only place caught by this policy, but this is the one
> where fix was obviously needed to improve readibility of code.
>
> In _most_ (but not all) of other places we assume that output we parse
> is in given format, and that regexp would always match...
>
>  gitweb/gitweb.perl |    7 ++++---
>  1 files changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 097bd18..c72ae10 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -690,9 +690,10 @@ sub evaluate_path_info {
>                # format key itself, with a prepended dot
>                while (my ($fmt, $opt) = each %known_snapshot_formats) {
>                        my $hash = $refname;
> -                       my $sfx;
> -                       $hash =~ s/(\Q$opt->{'suffix'}\E|\Q.$fmt\E)$//;
> -                       next unless $sfx = $1;
> +                       unless ($hash =~ s/(\Q$opt->{'suffix'}\E|\Q.$fmt\E)$//) {
> +                               next;
> +                       }
> +                       my $sfx = $1;
>                        # a valid suffix was found, so set the snapshot format
>                        # and reset the hash parameter
>                        $input_params{'snapshot_format'} = $fmt;

Totally mea culpa on this, I was optimizing it the wrong way.

Acked-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>


-- 
Giuseppe "Oblomov" Bilotta