git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Narebski <jnareb@gmail.com>
To: git@vger.kernel.org
Cc: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>,
	Jakub Narebski <jnareb@gmail.com>
Subject: [PATCH 3/4] gitweb: Simplify snapshot format detection logic in evaluate_path_info
Date: Mon, 11 May 2009 19:42:47 +0200	[thread overview]
Message-ID: <20090511173950.15152.61267.stgit@localhost.localdomain> (raw)
In-Reply-To: <20090511173025.15152.94215.stgit@localhost.localdomain>

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;

  parent reply	other threads:[~2009-05-11 17:42 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-11 17:36 [PATCH 0/4] gitweb: Some code cleanups, part 2 (low hanging fruit) Jakub Narebski
2009-05-11 17:37 ` [PATCH 1/4] gitweb: Replace wrongly added tabs with spaces Jakub Narebski
2009-05-11 17:39 ` [PATCH 2/4] gitweb: Use capturing parentheses only when you intend to capture Jakub Narebski
2009-05-11 17:42 ` Jakub Narebski [this message]
2009-05-11 17:52   ` [PATCH 3/4] gitweb: Simplify snapshot format detection logic in evaluate_path_info Giuseppe Bilotta
2009-05-11 17:45 ` [PATCH 4/4] gitweb: Remove unused $hash_base parameter from normalize_link_target Jakub Narebski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090511173950.15152.61267.stgit@localhost.localdomain \
    --to=jnareb@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=giuseppe.bilotta@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).