{"thread":{"id":"19262","subject":"[PATCH 0/5] gitweb: Some code cleanups (up to perlcritic --stern)","startedAt":"2009-05-10T00:03:50Z","lastAt":"2009-05-11T07:19:59Z","messageCount":16,"participants":["Jakub Narebski","Petr Baudis","Junio C Hamano","Daniel Pittman"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"113431","messageId":"200905100203.51744.jnareb@gmail.com","threadId":"19262","inReplyTo":null,"subject":"[PATCH 0/5] gitweb: Some code cleanups (up to perlcritic --stern)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-10T00:03:50Z","receivedAt":"2009-05-10T00:03:50Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"The following series consist of some code cleanups for gitweb.perl.\nThey're based on suggestions by perlcritic (Perl::Critic).  Most\npolicy modules are in turn based on Damian Conway's book \"Perl Best\nPractices\" (PBP).\n\nThis series was inspired by similar series of patches for git-send-email by\nBill Pemberton:\n  Subject: [PATCH 0/6] cleanups for git-send-email\n  Msg-Id: <1241010743-7020-1-git-send-email-wfp5p@virginia.edu>\n  URL: http://thread.gmane.org/gmane.comp.version-control.git/117881\n\nNot all suggestions by perlcritic were implemented (or, to be more\nexact, by its online version at http://perlcritic.com, which is\nrunning Perl-Critic version 1.096).\n\nBelow there is list of perlcritic suggestions, sorted by severity\n(gentle, stern, harsh, cruel, brutal): first list of patches in series\nwhich applied perlcritic suggestions, then suggestions not applied,\nwith short explanation why they were not used.\n\nThis series deals with suggestion with severity level >= 4 (gentle and\nstern suggestions); perlcritic suggestions with severity level >= 3\n(harsh) are in the works - but there are many more rules, and they are\nalso more subjective.\n\n\nShortlog (part 1/2):\n=================\nJakub Narebski (3):\n  gitweb: Remove function prototypes\n  gitweb: Do not use bareword filehandles\n  gitweb: Always use three argument form of open\n\nAll of those are for severity gentle (5).  The following policies\n(suggestions) with severity 5 were not implemented:\n\n* Perl::Critic::Policy::ValuesAndExpressions::ProhibitLeadingZeros\n  Write 'oct(755)' instead of '0755'.\n\n  We know what we are doing here; besides above Perl::Critic policy\n  has exceptions for chmod, mkdir, sysopen, umask, and we use octal\n  numbers for filemode.\n\n* Perl::Critic::Policy::Subroutines::ProhibitExplicitReturnUndef\n  Return failure with bare 'return' instead of 'return undef'.\n\n  First, this is a matter of taste; second, this would require more\n  detailed review. So it is left as it is... for now.\n\n* Perl::Critic::Policy::Subroutines::ProhibitNestedSubs\n  Declaring a named sub inside another named sub does not prevent\n  the inner sub from being global.\n\n  This is more about possibility of mismatched expectations.\n\n\nShortlog (part 2/2):\n=================\nJakub Narebski (1):\n  gitweb: Localize magic variable $/\n  gitweb: Use block form of map/grep in a few cases more\n\nAll of those are for severity stern (4).  The following policies\n(suggestions) with severity 4 were not implemented:\n\n* Perl::Critic::Policy::Subroutines::RequireArgUnpacking\n  Always unpack @_ first.\n\n  This requires careful handling (wrapper functions are exception of\n  this policy, but the tool does not detect it), and in most cases\n  this is the matter of the following techique:\n     my $a = shift;\n     my ($b, $c) = @_;\n  which could be improved.\n\n* Perl::Critic::Policy::Subroutines::RequireFinalReturn\n  End every path through a subroutine with an explicit return statement.\n\n  This is I think the matter of taste; note that this policy is not to\n  be followed blindly, according to documentation. I think that all\n  functions that do not return would be procedures (void as return\n  type) in C.\n\n* Perl::Critic::Policy::ValuesAndExpressions::ProhibitConstantPragma\n  Don't use 'constant FOO => 15', because it doesn't interpolate, but\n  the Readonly module.\n\n  The constants S_IFINVALID and S_IFGITLINK we declare follow naming\n  of filemode constants S_IFMT and S_IXUSR from Fcntl module we use in\n  our code.\n\n* Perl::Critic::Policy::InputOutput::RequireBriefOpen\n  Close filehandles as soon as possible after opening them\n  (because fFilehandles are a finite resource).\n  \n  In gitweb we use very small number of filehandles concurrently;\n  usually only one filehandle is open.  On the other hand for bigger\n  output we try to stream it.\n\n* Perl::Critic::Policy::ValuesAndExpressions::ProhibitMixedBooleanOperators\n  Don't mix high- and low-precedence booleans.\n\n  The code in question is list form of magic \"-|\" open ... or die ...,\n  where one of arguments uses '||' to set default value.  This is\n  intended, and not that cryptic.\n\n\nDiffstat:\n=========\n gitweb/gitweb.perl |   72 ++++++++++++++++++++++++++--------------------------\n 1 files changed, 36 insertions(+), 36 deletions(-)\n\n-- \nJakub Narebski\nShadeHawk on #git\n"},{"id":"113436","messageId":"200905100205.23733.jnareb@gmail.com","threadId":"19262","inReplyTo":"200905100203.51744.jnareb@gmail.com","subject":"[PATCHv2 1/5] gitweb: Remove function prototypes","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-10T00:05:23Z","receivedAt":"2009-05-10T00:05:23Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Use of function prototypes is considered bad practice in Perl.  The\nones used here didn't accomplish anything anyhow, so they've been\nremoved.\n\n>From perlsub(1):\n\n  [...] the intent of this feature [prototypes] is primarily to let\n  you define subroutines that work like built-in functions [...]\n  you can generate new syntax with it [...]\n\nWe don't want to have subroutines behaving exactly like built-in\nfunctions, we don't want to define new syntax / syntactic sugar, so\nprototypes in gitweb are not needed... and they can have unintended\nconsequences.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nPerl::Critic::Policy::Subroutines::ProhibitSubroutinePrototypes\n\n  Don't write 'sub my_function (@@) {}'.\n\n  Contrary to common belief, subroutine prototypes do not enable\n  compile-time checks for proper arguments. Don't use them.\n\nSee also Damian Conway's book \"Perl Best Practices\",\nchapter \"9.10. Prototypes\" (Don't use subroutine prototypes.)\n\n\nThis follows similar patch for git-send-email.perl by Bill Pemberton.\nAlso \"sub S_ISGITLINK($) {\" line caused `imenu` in my old cperl-mode\n(4.23) in GNU Emacs 21.4.1 to fail, sometimes.\n\nThis patch was send with slightly different commit message as\nstandalone patch earlier. This is the replacement patch, which differs\nonly in the commit message.\n\n gitweb/gitweb.perl |   12 +++++-------\n 1 files changed, 5 insertions(+), 7 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 3f99361..06e9160 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -838,7 +838,7 @@ exit;\n ## ======================================================================\n ## action links\n \n-sub href (%) {\n+sub href {\n \tmy %params = @_;\n \t# default is to use -absolute url() i.e. $my_uri\n \tmy $href = $params{-full} ? $my_url : $my_uri;\n@@ -1036,7 +1036,7 @@ sub esc_url {\n }\n \n # replace invalid utf8 character with SUBSTITUTION sequence\n-sub esc_html ($;%) {\n+sub esc_html {\n \tmy $str = shift;\n \tmy %opts = @_;\n \n@@ -1296,7 +1296,7 @@ use constant {\n };\n \n # submodule/subproject, a commit object reference\n-sub S_ISGITLINK($) {\n+sub S_ISGITLINK {\n \tmy $mode = shift;\n \n \treturn (($mode & S_IFMT) == S_IFGITLINK)\n@@ -2615,7 +2615,7 @@ sub parsed_difftree_line {\n }\n \n # parse line of git-ls-tree output\n-sub parse_ls_tree_line ($;%) {\n+sub parse_ls_tree_line {\n \tmy $line = shift;\n \tmy %opts = @_;\n \tmy %res;\n@@ -3213,7 +3213,6 @@ sub git_print_header_div {\n \t      \"\\n</div>\\n\";\n }\n \n-#sub git_print_authorship (\\%) {\n sub git_print_authorship {\n \tmy $co = shift;\n \n@@ -3269,8 +3268,7 @@ sub git_print_page_path {\n \tprint \"<br/></div>\\n\";\n }\n \n-# sub git_print_log (\\@;%) {\n-sub git_print_log ($;%) {\n+sub git_print_log {\n \tmy $log = shift;\n \tmy %opts = @_;\n \n-- \n1.6.3\n"},{"id":"113435","messageId":"200905100236.20158.jnareb@gmail.com","threadId":"19262","inReplyTo":"200905100203.51744.jnareb@gmail.com","subject":"[PATCH 2/5] gitweb: Do not use bareword filehandles","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-10T00:36:19Z","receivedAt":"2009-05-10T00:36:19Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"The script was using bareword filehandles.  This is considered a bad\npractice so they have been changed to indirect filehandles.\nChanges touch git_get_project_ctags and mimetype_guess_file.\n\nWhile at it rename local variable from $mime to $mimetype (in\nmimetype_guess_file) to better reflect its value (its contents).\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nPerl::Critic::Policy::InputOutput::ProhibitBarewordFileHandles\n\n  Write open my $fh, q{<}, $filename; instead of open FH, q{<}, $filename;.\n\n  Using bareword symbols to refer to file handles is particularly evil\n  because they are global, and you have no idea if that symbol already\n  points to some other file handle. You can mitigate some of that risk by\n  'local'izing the symbol first, but that's pretty ugly. Since Perl 5.6, you\n  can use an undefined scalar variable as a lexical reference to an\n  anonymous filehandle.\n\nSee also Damian Conway's book \"Perl Best Practices\",\nchapter \"10.1. Filehandles\" (Don't use bareword filehandles.)\n\n\nThis follows similar patch for git-send-email.perl by Bill Pemberton\nhttp://permalink.gmane.org/gmane.comp.version-control.git/117886\n\nCC-ed Pasky, who is responsible for code in both cases...\n\n gitweb/gitweb.perl |   22 +++++++++++-----------\n 1 files changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 06e9160..a9daa1d 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2065,18 +2065,18 @@ sub git_get_project_ctags {\n \tmy $ctags = {};\n \n \t$git_dir = \"$projectroot/$path\";\n-\tunless (opendir D, \"$git_dir/ctags\") {\n+\tunless (opendir my $dh, \"$git_dir/ctags\") {\n \t\treturn $ctags;\n \t}\n-\tforeach (grep { -f $_ } map { \"$git_dir/ctags/$_\" } readdir(D)) {\n-\t\topen CT, $_ or next;\n-\t\tmy $val = <CT>;\n+\tforeach (grep { -f $_ } map { \"$git_dir/ctags/$_\" } readdir($dh)) {\n+\t\topen my $ct, $_ or next;\n+\t\tmy $val = <$ct>;\n \t\tchomp $val;\n-\t\tclose CT;\n+\t\tclose $ct;\n \t\tmy $ctag = $_; $ctag =~ s#.*/##;\n \t\t$ctags->{$ctag} = $val;\n \t}\n-\tclosedir D;\n+\tclosedir $dh;\n \t$ctags;\n }\n \n@@ -2804,18 +2804,18 @@ sub mimetype_guess_file {\n \t-r $mimemap or return undef;\n \n \tmy %mimemap;\n-\topen(MIME, $mimemap) or return undef;\n-\twhile (<MIME>) {\n+\topen(my $mh, $mimemap) or return undef;\n+\twhile (<$mh>) {\n \t\tnext if m/^#/; # skip comments\n-\t\tmy ($mime, $exts) = split(/\\t+/);\n+\t\tmy ($mimetype, $exts) = split(/\\t+/);\n \t\tif (defined $exts) {\n \t\t\tmy @exts = split(/\\s+/, $exts);\n \t\t\tforeach my $ext (@exts) {\n-\t\t\t\t$mimemap{$ext} = $mime;\n+\t\t\t\t$mimemap{$ext} = $mimetype;\n \t\t\t}\n \t\t}\n \t}\n-\tclose(MIME);\n+\tclose($mh);\n \n \t$filename =~ /\\.([^.]*)$/;\n \treturn $mimemap{$1};\n-- \n1.6.3\n"},{"id":"113434","messageId":"200905100238.34838.jnareb@gmail.com","threadId":"19262","inReplyTo":"200905100203.51744.jnareb@gmail.com","subject":"[PATCH 3/5] gitweb: Always use three argument form of open","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-10T00:38:34Z","receivedAt":"2009-05-10T00:38:34Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"In most cases (except insert_file() subroutine) we used old two argument\nform of 'open' to open files for reading.  This can cause subtle bugs when\n$projectroot or $projects_list file starts with mode characters ('>', '<',\n'+<', '|', etc.) or with leading whitespace; and also when $projects_list\nfile or $mimetypes_file or ctags files end with trailing whitespace or '|'.\n\nAdditionally it is also more clear to explicitly state that we open those\nfiles for reading.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nPerl::Critic::Policy::InputOutput::ProhibitTwoArgOpen\n\n  Write open $fh, q{<}, $filename; instead of open $fh, \"<$filename\";.\n\n  The three-argument form of open (introduced in Perl 5.6) prevents subtle\n  bugs that occur when the filename starts with funny characters like\n  '>' or '<'.  It's also more explicitly clear to define the input mode of\n  the file, and not to e.g. use open( $fh, 'foo.txt' );\n\nSee also Damian Conway's book \"Perl Best Practices\",\nchapter \"10.4. Opening Cleanly\" (Use either the 'IO::File' module\nor the three-argument form of 'open'.)\n\n\nThis patch _textually_ depends on the previous patch (no bareword\nfilehandles), even if _conceptually_ they are quite independent.\n\n gitweb/gitweb.perl |   12 ++++++------\n 1 files changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex a9daa1d..852beb6 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2050,7 +2050,7 @@ sub git_get_project_description {\n \tmy $path = shift;\n \n \t$git_dir = \"$projectroot/$path\";\n-\topen my $fd, \"$git_dir/description\"\n+\topen my $fd, '<', \"$git_dir/description\"\n \t\tor return git_get_project_config('description');\n \tmy $descr = <$fd>;\n \tclose $fd;\n@@ -2069,7 +2069,7 @@ sub git_get_project_ctags {\n \t\treturn $ctags;\n \t}\n \tforeach (grep { -f $_ } map { \"$git_dir/ctags/$_\" } readdir($dh)) {\n-\t\topen my $ct, $_ or next;\n+\t\topen my $ct, '<', $_ or next;\n \t\tmy $val = <$ct>;\n \t\tchomp $val;\n \t\tclose $ct;\n@@ -2129,7 +2129,7 @@ sub git_get_project_url_list {\n \tmy $path = shift;\n \n \t$git_dir = \"$projectroot/$path\";\n-\topen my $fd, \"$git_dir/cloneurl\"\n+\topen my $fd, '<', \"$git_dir/cloneurl\"\n \t\tor return wantarray ?\n \t\t@{ config_to_multi(git_get_project_config('url')) } :\n \t\t   config_to_multi(git_get_project_config('url'));\n@@ -2187,7 +2187,7 @@ sub git_get_projects_list {\n \t\t# 'libs%2Fklibc%2Fklibc.git H.+Peter+Anvin'\n \t\t# 'linux%2Fhotplug%2Fudev.git Greg+Kroah-Hartman'\n \t\tmy %paths;\n-\t\topen my ($fd), $projects_list or return;\n+\t\topen my $fd, '<', $projects_list or return;\n \tPROJECT:\n \t\twhile (my $line = <$fd>) {\n \t\t\tchomp $line;\n@@ -2250,7 +2250,7 @@ sub git_get_project_list_from_file {\n \t# 'libs%2Fklibc%2Fklibc.git H.+Peter+Anvin'\n \t# 'linux%2Fhotplug%2Fudev.git Greg+Kroah-Hartman'\n \tif (-f $projects_list) {\n-\t\topen (my $fd , $projects_list);\n+\t\topen(my $fd, '<', $projects_list);\n \t\twhile (my $line = <$fd>) {\n \t\t\tchomp $line;\n \t\t\tmy ($pr, $ow) = split ' ', $line;\n@@ -2804,7 +2804,7 @@ sub mimetype_guess_file {\n \t-r $mimemap or return undef;\n \n \tmy %mimemap;\n-\topen(my $mh, $mimemap) or return undef;\n+\topen(my $mh, '<', $mimemap) or return undef;\n \twhile (<$mh>) {\n \t\tnext if m/^#/; # skip comments\n \t\tmy ($mimetype, $exts) = split(/\\t+/);\n-- \n1.6.3\n"},{"id":"113433","messageId":"200905100239.40092.jnareb@gmail.com","threadId":"19262","inReplyTo":"200905100203.51744.jnareb@gmail.com","subject":"[PATCH 4/5] gitweb: Localize magic variable $/","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-10T00:39:39Z","receivedAt":"2009-05-10T00:39:39Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Instead of undefining and then restoring magic variable $/ (input\nrecord separator) for 'slurp mode', localize it.\n\nWhile at it, state explicitly that \"local $/;\" makes it undefined, by\nusing explicit  \"local $/ = undef;\".\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nPerl::Critic::Policy::Variables::RequireLocalizedPunctuationVars\n\n  Magic variables should be assigned as \"local\".\n\n  Punctuation variables (and their English.pm equivalents) are global\n  variables. Messing with globals is dangerous in a complex program as it\n  can lead to very subtle and hard to fix bugs. If you must change a magic\n  variable in a non-trivial program, do it in a local scope.\n\n  For example, to slurp a filehandle into a scalar, it's common to set the\n  record separator to undef instead of a newline. If you choose to do this\n  (instead of using File::Slurp!) then be sure to localize the global and\n  change it for as short a time as possible.\n\nSee also Damian Conway's book \"Perl Best Practices\",\n5.6. Localizing Punctuation Variables (If you're forced to modify\na punctuation variable, localize it.)\n\n\nPerl::Critic::Policy::Variables::RequireInitializationForLocalVars\n\n  Write  'local $foo = $bar;'  instead of just  'local $foo;'.\n\n  Most people don't realize that a localized copy of a variable does\n  not retain its original value. Unless you initialize the variable\n  when you local-ize it, it defaults to undef. If you want the\n  variable to retain its original value, just initialize it to\n  itself. If you really do want the localized copy to be undef, then\n  make it explicit.\n\n\n gitweb/gitweb.perl |   24 +++++++++++++-----------\n 1 files changed, 13 insertions(+), 11 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 852beb6..1cb3a4f 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3326,7 +3326,7 @@ sub git_get_link_target {\n \topen my $fd, \"-|\", git_cmd(), \"cat-file\", \"blob\", $hash\n \t\tor return;\n \t{\n-\t\tlocal $/;\n+\t\tlocal $/ = undef;\n \t\t$link_target = <$fd>;\n \t}\n \tclose $fd\n@@ -4801,11 +4801,10 @@ sub git_blob_plain {\n \t\t-content_disposition =>\n \t\t\t($sandbox ? 'attachment' : 'inline')\n \t\t\t. '; filename=\"' . $save_as . '\"');\n-\tundef $/;\n+\tlocal $/ = undef;\n \tbinmode STDOUT, ':raw';\n \tprint <$fd>;\n \tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n-\t$/ = \"\\n\";\n \tclose $fd;\n }\n \n@@ -4907,12 +4906,15 @@ sub git_tree {\n \t\t}\n \t}\n \tdie_error(404, \"No such tree\") unless defined($hash);\n-\t$/ = \"\\0\";\n-\topen my $fd, \"-|\", git_cmd(), \"ls-tree\", '-z', $hash\n-\t\tor die_error(500, \"Open git-ls-tree failed\");\n-\tmy @entries = map { chomp; $_ } <$fd>;\n-\tclose $fd or die_error(404, \"Reading tree failed\");\n-\t$/ = \"\\n\";\n+\n+\t{\n+\t\tlocal $/ = \"\\0\";\n+\t\topen my $fd, \"-|\", git_cmd(), \"ls-tree\", '-z', $hash\n+\t\t\tor die_error(500, \"Open git-ls-tree failed\");\n+\t\tmy @entries = map { chomp; $_ } <$fd>;\n+\t\tclose $fd\n+\t\t\tor die_error(404, \"Reading tree failed\");\n+\t}\n \n \tmy $refs = git_get_references();\n \tmy $ref = format_ref_marker($refs, $hash_base);\n@@ -5807,7 +5809,7 @@ sub git_search {\n \n \t\tprint \"<table class=\\\"pickaxe search\\\">\\n\";\n \t\tmy $alternate = 1;\n-\t\t$/ = \"\\n\";\n+\t\tlocal $/ = \"\\n\";\n \t\topen my $fd, '-|', git_cmd(), '--no-pager', 'log', @diff_opts,\n \t\t\t'--pretty=format:%H', '--no-abbrev', '--raw', \"-S$searchtext\",\n \t\t\t($search_use_regexp ? '--pickaxe-regex' : ());\n@@ -5877,7 +5879,7 @@ sub git_search {\n \t\tprint \"<table class=\\\"grep_search\\\">\\n\";\n \t\tmy $alternate = 1;\n \t\tmy $matches = 0;\n-\t\t$/ = \"\\n\";\n+\t\tlocal $/ = \"\\n\";\n \t\topen my $fd, \"-|\", git_cmd(), 'grep', '-n',\n \t\t\t$search_use_regexp ? ('-E', '-i') : '-F',\n \t\t\t$searchtext, $co{'tree'};\n-- \n1.6.3\n"},{"id":"113432","messageId":"200905100240.37772.jnareb@gmail.com","threadId":"19262","inReplyTo":"200905100203.51744.jnareb@gmail.com","subject":"[PATCH 5/5] gitweb: Use block form of map/grep in a few cases more","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-10T00:40:37Z","receivedAt":"2009-05-10T00:40:37Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Use block form of 'grep' i.e. 'grep {BLOCK} LIST' rather than\n'grep(EXPR, LIST)' in filter_snapshot_fmts subroutine.  This makes\ncode more readable, as expression is rather long, and statement above\nthere is 'map' with very similar expression also in the block form.\n\nRemove unnecessary and misleading parentheses around block form 'map'\narguments in quote_command subroutine.\n\nThe inner \"map\" in format_snapshot_links was left alone, as it is not\nclear whether adding parentheses or changing it into block form would\nimprove readibility and clarity of this code.\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nPerl::Critic::Policy::BuiltinFunctions::RequireBlockGrep\n\n  Write grep { $_ =~ /$pattern/ } @list instead of grep /$pattern/, @list.\n\n  The expression forms of grep and map are awkward and hard to read. Use the\n  block forms instead.\n\nSee also Damian Conway's book \"Perl Best Practices\", section\n8.13. Mapping and Grepping (Always use a block with a map and grep.)\n\nNOTE: In my opinion the expression form when using function-like call to\n\"grep\" or \"map\" e.g. grep(/$pattern/, @list) is readable enough.  In more\ncomplicated cases (with more complicated expressions, especially with\nexplicit $_) it might be better to use block form instead, as stated above.\n\nAnd what do *you* think?\n\n gitweb/gitweb.perl |    6 +++---\n 1 files changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 1cb3a4f..f465666 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -458,8 +458,8 @@ sub filter_snapshot_fmts {\n \t@fmts = map {\n \t\texists $known_snapshot_format_aliases{$_} ?\n \t\t       $known_snapshot_format_aliases{$_} : $_} @fmts;\n-\t@fmts = grep(exists $known_snapshot_formats{$_}, @fmts);\n-\n+\t@fmts = grep {\n+\t\texists $known_snapshot_formats{$_} } @fmts;\n }\n \n our $GITWEB_CONFIG = $ENV{'GITWEB_CONFIG'} || \"++GITWEB_CONFIG++\";\n@@ -1838,7 +1838,7 @@ sub git_cmd {\n # Try to avoid using this function wherever possible.\n sub quote_command {\n \treturn join(' ',\n-\t\t    map( { my $a = $_; $a =~ s/(['!])/'\\\\$1'/g; \"'$a'\" } @_ ));\n+\t\tmap { my $a = $_; $a =~ s/(['!])/'\\\\$1'/g; \"'$a'\" } @_ );\n }\n \n # get HEAD ref of given project as hash\n-- \n1.6.3\n"},{"id":"113449","messageId":"20090510075053.GA6058@machine.or.cz","threadId":"19262","inReplyTo":"200905100236.20158.jnareb@gmail.com","subject":"Re: [PATCH 2/5] gitweb: Do not use bareword filehandles","fromName":"Petr Baudis","fromEmail":"pasky@suse.cz","sentAt":"2009-05-10T07:50:53Z","receivedAt":"2009-05-10T07:50:53Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"On Sun, May 10, 2009 at 02:36:19AM +0200, Jakub Narebski wrote:\n> The script was using bareword filehandles.  This is considered a bad\n> practice so they have been changed to indirect filehandles.\n> Changes touch git_get_project_ctags and mimetype_guess_file.\n> \n> While at it rename local variable from $mime to $mimetype (in\n> mimetype_guess_file) to better reflect its value (its contents).\n> \n> Signed-off-by: Jakub Narebski <jnareb@gmail.com>\n> ---\n> Perl::Critic::Policy::InputOutput::ProhibitBarewordFileHandles\n> \n>   Write open my $fh, q{<}, $filename; instead of open FH, q{<}, $filename;.\n> \n>   Using bareword symbols to refer to file handles is particularly evil\n>   because they are global, and you have no idea if that symbol already\n>   points to some other file handle. You can mitigate some of that risk by\n>   'local'izing the symbol first, but that's pretty ugly. Since Perl 5.6, you\n>   can use an undefined scalar variable as a lexical reference to an\n>   anonymous filehandle.\n> \n> See also Damian Conway's book \"Perl Best Practices\",\n> chapter \"10.1. Filehandles\" (Don't use bareword filehandles.)\n> \n> \n> This follows similar patch for git-send-email.perl by Bill Pemberton\n> http://permalink.gmane.org/gmane.comp.version-control.git/117886\n> \n> CC-ed Pasky, who is responsible for code in both cases...\n\nYeah, the book I learnt Perl from many years ago used bareword\nfilehandles (but it was an excellent textbook in most other aspects)\nso this is a custom I have to work hard to evict. ;-)\n\nAcked-by: Petr Baudis <pasky@suse.cz>\n"},{"id":"113452","messageId":"200905101105.40025.jnareb@gmail.com","threadId":"19262","inReplyTo":"200905100205.23733.jnareb@gmail.com","subject":"Re: [PATCHv2 1/5] gitweb: Remove function prototypes","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-10T09:05:38Z","receivedAt":"2009-05-10T09:05:38Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, 10 May 2009, Jakub Narebski wrote:\n\n> This patch was send with slightly different commit message as\n> standalone patch earlier. This is the replacement patch, which differs\n> only in the commit message.\n\nI see that v1 version of patch (send as standalone patch) was already \naccepted.  Just skip this patch then, as it differs only in that it has \nmore wordy commit message...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"113453","messageId":"200905101127.51963.jnareb@gmail.com","threadId":"19262","inReplyTo":"20090510075053.GA6058@machine.or.cz","subject":"Re: [PATCH 2/5] gitweb: Do not use bareword filehandles","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-10T09:27:51Z","receivedAt":"2009-05-10T09:27:51Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Sun, 10 May 2009, Petr Baudis wrote:\n> On Sun, May 10, 2009 at 02:36:19AM +0200, Jakub Narebski wrote:\n\n> > ---\n> > Perl::Critic::Policy::InputOutput::ProhibitBarewordFileHandles\n> > \n> >   Write open my $fh, q{<}, $filename; instead of open FH, q{<}, $filename;.\n> > \n> >   Using bareword symbols to refer to file handles is particularly evil\n> >   because they are global, and you have no idea if that symbol already\n> >   points to some other file handle. You can mitigate some of that risk by\n> >   'local'izing the symbol first, but that's pretty ugly. Since Perl 5.6, you\n> >   can use an undefined scalar variable as a lexical reference to an\n> >   anonymous filehandle.\n> > \n> > See also Damian Conway's book \"Perl Best Practices\",\n> > chapter \"10.1. Filehandles\" (Don't use bareword filehandles.)\n> > \n> > \n> > This follows similar patch for git-send-email.perl by Bill Pemberton\n> > http://permalink.gmane.org/gmane.comp.version-control.git/117886\n> > \n> > CC-ed Pasky, who is responsible for code in both cases...\n> \n> Yeah, the book I learnt Perl from many years ago used bareword\n> filehandles (but it was an excellent textbook in most other aspects)\n> so this is a custom I have to work hard to evict. ;-)\n> \n> Acked-by: Petr Baudis <pasky@suse.cz>\n\nWell, on one hand this is less of an issue for standalone script like\ngitweb is than for library / module.  On the other hand we use indirect\nfilehandles everywhere else, so it is also matter of style consistency.\n\n\"Perl Best Practices\" gives as an example of bad behavior which we can\nget by using bareword [global] filehandles: 1) automatic closing of\nother filehandle with the same name (e.g. not only you use FILE for\nbareword filehandle); 2) open might fail if there exist subroutine /\nconstant with the same name as filehandle e.g. EXDEV (from POSIX module).\n\nNote however that prior to Perl 5.6 it was not that easy to avoid\nusing bareword filehandles. Perhaps the book in question (or author of\nthe textbook you mention) started with earlier Perl...\n\n\nP.S. Git.pm generates surprisingly small amount of perlcritic-isms,\nmost of whose are either matter of style, or are false positives.\n-- \nJakub Narebski\nPoland\n"},{"id":"113505","messageId":"7vy6t4sbxj.fsf@alter.siamese.dyndns.org","threadId":"19262","inReplyTo":"200905100203.51744.jnareb@gmail.com","subject":"Re: [PATCH 0/5] gitweb: Some code cleanups (up to perlcritic --stern)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-11T00:47:20Z","receivedAt":"2009-05-11T00:47:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> The following series consist of some code cleanups for gitweb.perl.\n> They're based on suggestions by perlcritic (Perl::Critic).\n\nNice.\n\nBut this series, when queued to 'pu', seems to break t9500; I haven't\nlooked at the breakage myself yet.\n\nJakub, did you run this through the testsuite already (the problem could\nwell be on my end and that is why I am asking)?\n"},{"id":"113507","messageId":"200905110321.08109.jnareb@gmail.com","threadId":"19262","inReplyTo":"200905100236.20158.jnareb@gmail.com","subject":"[PATCH v2 2/5] gitweb: Do not use bareword filehandles","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-11T01:21:06Z","receivedAt":"2009-05-11T01:21:06Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"gitweb: Do not use bareword filehandles\n\nThe script was using bareword filehandles.  This is considered a bad\npractice so they have been changed to indirect filehandles.\n\nChanges touch git_get_project_ctags and mimetype_guess_file;\nwhile at it rename local variable from $mime to $mimetype (in\nmimetype_guess_file) to better reflect its value (its contents).\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nI'm extremely sorry for this stupid mistake. Below there is interdiff\n(indented, as to not be mistaken for diff itself)\n\n\tdiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n\tindex a9daa1d..584644c 100755\n\t--- a/gitweb/gitweb.perl\n\t+++ b/gitweb/gitweb.perl\n\t@@ -2065,9 +2065,8 @@ sub git_get_project_ctags {\n\t        my $ctags = {};\n\t \n\t        $git_dir = \"$projectroot/$path\";\n\t-       unless (opendir my $dh, \"$git_dir/ctags\") {\n\t-               return $ctags;\n\t-       }\n\t+       opendir my $dh, \"$git_dir/ctags\"\n\t+               or return $ctags;\n\t        foreach (grep { -f $_ } map { \"$git_dir/ctags/$_\" } readdir($dh)) {\n\t                open my $ct, $_ or next;\n\t                my $val = <$ct>;\n\n gitweb/gitweb.perl |   25 ++++++++++++-------------\n 1 files changed, 12 insertions(+), 13 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex 06e9160..584644c 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -2065,18 +2065,17 @@ sub git_get_project_ctags {\n \tmy $ctags = {};\n \n \t$git_dir = \"$projectroot/$path\";\n-\tunless (opendir D, \"$git_dir/ctags\") {\n-\t\treturn $ctags;\n-\t}\n-\tforeach (grep { -f $_ } map { \"$git_dir/ctags/$_\" } readdir(D)) {\n-\t\topen CT, $_ or next;\n-\t\tmy $val = <CT>;\n+\topendir my $dh, \"$git_dir/ctags\"\n+\t\tor return $ctags;\n+\tforeach (grep { -f $_ } map { \"$git_dir/ctags/$_\" } readdir($dh)) {\n+\t\topen my $ct, $_ or next;\n+\t\tmy $val = <$ct>;\n \t\tchomp $val;\n-\t\tclose CT;\n+\t\tclose $ct;\n \t\tmy $ctag = $_; $ctag =~ s#.*/##;\n \t\t$ctags->{$ctag} = $val;\n \t}\n-\tclosedir D;\n+\tclosedir $dh;\n \t$ctags;\n }\n \n@@ -2804,18 +2803,18 @@ sub mimetype_guess_file {\n \t-r $mimemap or return undef;\n \n \tmy %mimemap;\n-\topen(MIME, $mimemap) or return undef;\n-\twhile (<MIME>) {\n+\topen(my $mh, $mimemap) or return undef;\n+\twhile (<$mh>) {\n \t\tnext if m/^#/; # skip comments\n-\t\tmy ($mime, $exts) = split(/\\t+/);\n+\t\tmy ($mimetype, $exts) = split(/\\t+/);\n \t\tif (defined $exts) {\n \t\t\tmy @exts = split(/\\s+/, $exts);\n \t\t\tforeach my $ext (@exts) {\n-\t\t\t\t$mimemap{$ext} = $mime;\n+\t\t\t\t$mimemap{$ext} = $mimetype;\n \t\t\t}\n \t\t}\n \t}\n-\tclose(MIME);\n+\tclose($mh);\n \n \t$filename =~ /\\.([^.]*)$/;\n \treturn $mimemap{$1};\n-- \n1.6.3\n"},{"id":"113508","messageId":"200905110329.40666.jnareb@gmail.com","threadId":"19262","inReplyTo":"200905100238.34838.jnareb@gmail.com","subject":"[PATCH v2 3/5] gitweb: Always use three argument form of open","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-11T01:29:40Z","receivedAt":"2009-05-11T01:29:40Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":">From 94638fb6edf3ea693228c680a6a30271ccd77522 Mon Sep 17 00:00:00 2001\nFrom: Jakub Narebski <jnareb@gmail.com>\nDate: Mon, 11 May 2009 03:25:55 +0200\nSubject: [PATCH] gitweb: Localize magic variable $/\n\nInstead of undefining and then restoring magic variable $/ (input\nrecord separator) for 'slurp mode', localize it.\n\nWhile at it, state explicitely that \"local $/;\" makes it undefined, by\nusing explicit  \"local $/ = undef;\".\n\nSigned-off-by: Jakub Narebski <jnareb@gmail.com>\n---\nThis also required correction. I am extremely sorry about that.\nBelow there is (whitespace mangled) interdiff to previous version\n\n\tdiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\n\tindex 76c0684..4efeeed 100755\n\t--- a/gitweb/gitweb.perl\n\t+++ b/gitweb/gitweb.perl\n\t@@ -4906,11 +4906,12 @@ sub git_tree {\n\t        }\n\t        die_error(404, \"No such tree\") unless defined($hash);\n\t \n\t+       my @entries = ();\n\t        {\n\t                local $/ = \"\\0\";\n\t                open my $fd, \"-|\", git_cmd(), \"ls-tree\", '-z', $hash\n\t                        or die_error(500, \"Open git-ls-tree failed\");\n\t-               my @entries = map { chomp; $_ } <$fd>;\n\t+               @entries = map { chomp; $_ } <$fd>;\n\t                close $fd\n\t                        or die_error(404, \"Reading tree failed\");\n\t        }\n\n\n\n gitweb/gitweb.perl |   25 ++++++++++++++-----------\n 1 files changed, 14 insertions(+), 11 deletions(-)\n\ndiff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl\nindex e7cab90..4efeeed 100755\n--- a/gitweb/gitweb.perl\n+++ b/gitweb/gitweb.perl\n@@ -3325,7 +3325,7 @@ sub git_get_link_target {\n \topen my $fd, \"-|\", git_cmd(), \"cat-file\", \"blob\", $hash\n \t\tor return;\n \t{\n-\t\tlocal $/;\n+\t\tlocal $/ = undef;\n \t\t$link_target = <$fd>;\n \t}\n \tclose $fd\n@@ -4800,11 +4800,10 @@ sub git_blob_plain {\n \t\t-content_disposition =>\n \t\t\t($sandbox ? 'attachment' : 'inline')\n \t\t\t. '; filename=\"' . $save_as . '\"');\n-\tundef $/;\n+\tlocal $/ = undef;\n \tbinmode STDOUT, ':raw';\n \tprint <$fd>;\n \tbinmode STDOUT, ':utf8'; # as set at the beginning of gitweb.cgi\n-\t$/ = \"\\n\";\n \tclose $fd;\n }\n \n@@ -4906,12 +4905,16 @@ sub git_tree {\n \t\t}\n \t}\n \tdie_error(404, \"No such tree\") unless defined($hash);\n-\t$/ = \"\\0\";\n-\topen my $fd, \"-|\", git_cmd(), \"ls-tree\", '-z', $hash\n-\t\tor die_error(500, \"Open git-ls-tree failed\");\n-\tmy @entries = map { chomp; $_ } <$fd>;\n-\tclose $fd or die_error(404, \"Reading tree failed\");\n-\t$/ = \"\\n\";\n+\n+\tmy @entries = ();\n+\t{\n+\t\tlocal $/ = \"\\0\";\n+\t\topen my $fd, \"-|\", git_cmd(), \"ls-tree\", '-z', $hash\n+\t\t\tor die_error(500, \"Open git-ls-tree failed\");\n+\t\t@entries = map { chomp; $_ } <$fd>;\n+\t\tclose $fd\n+\t\t\tor die_error(404, \"Reading tree failed\");\n+\t}\n \n \tmy $refs = git_get_references();\n \tmy $ref = format_ref_marker($refs, $hash_base);\n@@ -5806,7 +5809,7 @@ sub git_search {\n \n \t\tprint \"<table class=\\\"pickaxe search\\\">\\n\";\n \t\tmy $alternate = 1;\n-\t\t$/ = \"\\n\";\n+\t\tlocal $/ = \"\\n\";\n \t\topen my $fd, '-|', git_cmd(), '--no-pager', 'log', @diff_opts,\n \t\t\t'--pretty=format:%H', '--no-abbrev', '--raw', \"-S$searchtext\",\n \t\t\t($search_use_regexp ? '--pickaxe-regex' : ());\n@@ -5876,7 +5879,7 @@ sub git_search {\n \t\tprint \"<table class=\\\"grep_search\\\">\\n\";\n \t\tmy $alternate = 1;\n \t\tmy $matches = 0;\n-\t\t$/ = \"\\n\";\n+\t\tlocal $/ = \"\\n\";\n \t\topen my $fd, \"-|\", git_cmd(), 'grep', '-n',\n \t\t\t$search_use_regexp ? ('-E', '-i') : '-F',\n \t\t\t$searchtext, $co{'tree'};\n-- \n1.6.3\n"},{"id":"113509","messageId":"200905110333.52127.jnareb@gmail.com","threadId":"19262","inReplyTo":"7vy6t4sbxj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/5] gitweb: Some code cleanups (up to perlcritic --stern)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-11T01:33:51Z","receivedAt":"2009-05-11T01:33:51Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 11 May 2009, Junio C Hamano wrote:\n\n> But this series, when queued to 'pu', seems to break t9500; I haven't\n> looked at the breakage myself yet.\n\nI'm sorry about that. My bad. The fix is in the email (unless you\nprefer for me to just resend the series)...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"113513","messageId":"7viqk8s20j.fsf@alter.siamese.dyndns.org","threadId":"19262","inReplyTo":"200905110333.52127.jnareb@gmail.com","subject":"Re: [PATCH 0/5] gitweb: Some code cleanups (up to perlcritic --stern)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-05-11T04:21:32Z","receivedAt":"2009-05-11T04:21:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jakub Narebski <jnareb@gmail.com> writes:\n\n> On Mon, 11 May 2009, Junio C Hamano wrote:\n>\n>> But this series, when queued to 'pu', seems to break t9500; I haven't\n>> looked at the breakage myself yet.\n>\n> I'm sorry about that. My bad. The fix is in the email (unless you\n> prefer for me to just resend the series)...\n\nThat's Ok.  I had them near the tip of 'pu', and I can just replace them.\n\nBut this episode does not give much confidence in Perl::Critic does it?\nThe runtime \"use strict\" diagnosed undeclared globals in the cleaned up\ncode, but presumably the Critic did not complain anything about it, right?\n"},{"id":"113514","messageId":"877i0op6hb.fsf@rimspace.net","threadId":"19262","inReplyTo":"7viqk8s20j.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/5] gitweb: Some code cleanups (up to perlcritic --stern)","fromName":"Daniel Pittman","fromEmail":"daniel@rimspace.net","sentAt":"2009-05-11T05:13:20Z","receivedAt":"2009-05-11T05:13:20Z","isPatch":true,"sender":{"key":"daniel@rimspace.net","avatar":"https://gravatar.com/avatar/67f2f02d3521b163f6b7a0dec5943d33fbc5a2a6d9702ab3fef99032e59e692f?d=mp&s=160"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n> Jakub Narebski <jnareb@gmail.com> writes:\n>> On Mon, 11 May 2009, Junio C Hamano wrote:\n>>\n>>> But this series, when queued to 'pu', seems to break t9500; I haven't\n>>> looked at the breakage myself yet.\n>>\n>> I'm sorry about that. My bad. The fix is in the email (unless you\n>> prefer for me to just resend the series)...\n>\n> That's Ok.  I had them near the tip of 'pu', and I can just replace them.\n>\n> But this episode does not give much confidence in Perl::Critic does it?\n> The runtime \"use strict\" diagnosed undeclared globals in the cleaned up\n> code, but presumably the Critic did not complain anything about it, right?\n\nPerl::Critic is about coding style, not about tests like 'use strict'\nthat are detected by Perl already, for better or worse.\n\nIn other words: like checkpatch on the LKML, not like sparse. ;)\n\nRegards,\n        Daniel\n"},{"id":"113516","messageId":"200905110920.01231.jnareb@gmail.com","threadId":"19262","inReplyTo":"7viqk8s20j.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 0/5] gitweb: Some code cleanups (up to perlcritic --stern)","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2009-05-11T07:19:59Z","receivedAt":"2009-05-11T07:19:59Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 11 May 2009, Junio C Hamano wrote:\n> Jakub Narebski <jnareb@gmail.com> writes: \n>> On Mon, 11 May 2009, Junio C Hamano wrote:\n>>\n>>> But this series, when queued to 'pu', seems to break t9500; I haven't\n>>> looked at the breakage myself yet.\n>>\n>> I'm sorry about that. My bad. The fix is in the email (unless you\n>> prefer for me to just resend the series)...\n> \n> That's Ok.  I had them near the tip of 'pu', and I can just replace them.\n> \n> But this episode does not give much confidence in Perl::Critic does it?\n> The runtime \"use strict\" diagnosed undeclared globals in the cleaned up\n> code, but presumably the Critic did not complain anything about it, right?\n\nWell, IIRC it didn't complain because I didn't run Perl::Critic (or to\nbe more exact http://perlcritic.com) on cleaned up code... But I guess\nthat Perl::Critic might not try to catch them because 'use strict'\ncatches them.\n\nP.S. Of course Perl::Critic is not perfect. Most funny quirk was it\ncomplaining in --harsh (severity 3) mode \n  Main code has high complexity score (54) at line 1, column 1.\n  Consider refactoring.  Severity: 3\nabout \"#!/usr/bin/perl\" line!\n\n-- \nJakub Narebski\nPoland\n"}]}