git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Date
May 11, 2009, 17:52 UTC
Message-ID
<cb7bb73a0905111052g4f3f7e23yfbe54bf3cb3e2518@mail.gmail.com>
In-Reply-To
<20090511173950.15152.61267.stgit@localhost.localdomain>
On Mon, May 11, 2009 at 7:42 PM, Jakub Narebski <jnareb@gmail.com> wrote:
Show 44 quoted lines
> 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
Previous: Jakub NarebskiNext: Jakub Narebski
Message 5 of 6 in “gitweb: Some code cleanups, part 2 (low hanging fruit)”
  1. 0/4 gitweb: Some code cleanups, part 2 (low hanging fruit)Jakub Narebski, May 11, 2009
  2. 1/4 gitweb: Replace wrongly added tabs with spacesJakub Narebski, May 11, 2009
  3. 2/4 gitweb: Use capturing parentheses only when you intend to captureJakub Narebski, May 11, 2009
  4. 3/4 gitweb: Simplify snapshot format detection logic in evaluate_path_infoJakub Narebski, May 11, 2009
  5. Giuseppe BilottaMay 11, 2009
  6. 4/4 gitweb: Remove unused $hash_base parameter from normalize_link_targetJakub Narebski, May 11, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.