in reply to Request for Perl::Critic Testimonials

The most weird error messages i get from Perl::Critic are those: 
Regular expression without '/m' flag
Regular expression without '/x' flag

[download]
They look very funny when they both refer to the line like this: 
next if /^;/; #skip comments

[download]
I think you can set a length limit for the regular expressions to receive a warning about /x flag. Maybe, regexes shorter than e.g. 7 characters do not need /x flag? Or even better—remove warnings for regexes containing less than 5 metacharacters, e.g. /(foo|bar)/? The regexes containing only ONE interpolated variable (like the one mentioned below) must not cause them, too.

The warning about /m simply looks weird for me though I can understand why do you use it. But it must not appear to the lines like 

$string =~ /$re/;

[download]
The next bug: the line 
local $" = '|';

[download]
causes two warnings: 
Variable declared as 'local'
Magic punctuation variable used

[download]

while it must cause only the second warning! To use or not to use the punctuation variables is some kind of personal style, but not local'izing them is a severe error!

A little tip about the web-interface: You could add a possibility for the user to re-sort your warnings list by severity or by categories. The list would also look much better if it was represented by a table rather than by a simple linklist :)

Despite all my remarks you have done a great work ad i hope that my comments will help you to improve it :)

Sorry for my bad English—it's not my native language :)

Replies are listed 'Best First'.
Re^2: Request for Perl::Critic Testimonials (some weirdnesses of the module)
by Herkum (Parson) on Jul 15, 2006 at 20:14 UTC 
    next if /^;/; #skip comments

    [download]
    It is complaining the xm switches are not included Change it to this,

    Update: Also wrong per Jeronim

    next if /^;/xms;

    [download]
    local $" = '|';

    [download]
    Don't use local variables, use my. Don't the special variables like $", use the English module.

    Updated: Suggested but not working code

    use English qw( -no_match_vars );

my $LIST_SEPERATOR = q{|};

    [download]

    Wrong spelling and yes will break your Code, Both pointed out by Jeronim and Chromatic...

    use English qw( -no_match_vars );

{
local $LIST_SEPARATOR = q{|}; 
}

    [download]

    Would love to delete the post for just being wrong... Oh well :)

[reply]
[d/l]
[select]
      This will get you through the critic...

      ... and break your code.

      use Test::More tests => 1;

use English '-no_match_vars';

my $LIST_SEPARATOR = '|';
my @items          = qw( foo bar baz );

my $interpolated   = "@items";
is( $interpolated, 'foo|bar|baz', 'somehow package globals and lexical
+s merged' );

      [download]
[reply]
[d/l]
      You decided that my English is so poor that i can't read the docs of Perl::Critic and understand why these warnings appeared? ;)

      Your comment contains many errors :( Yes, if i modify my code according to your recommendations, Perl::Critic will stop complaining—but my code will stop working :)

      Your recommendation to use 

      next if /^;/xms;

      [download]
      is very weird. The using of /s and /m switches together causes a very unexpected interpretation of the RegEx:

      perlretut says:
      both s and m modifiers (//sm): Treat string as a single long line, but detect multiple lines. '.' matches any character, even "\n". ^ and $, however, are able to match at the start or end of any line within the string.

      Do you still think that it's a good idea to add /ms to the end of each regex?

      Don't use local variables, use my. Don't the special variables like $", use the English module. 
      use English qw( -no_match_vars ); 
my $LIST_SEPERATOR = q{|};

      [download]

      1. The Perl built-in variables lose their magic capabilities when declared as my. Didn't you know?
      2. There is no $LIST_SEPERATOR. Use $LIST_SEPARATOR instead.
      3. If you don't believe me, try the following code: 
        #!/usr/bin/perl

use warnings;
use strict;

use English qw( -no_match_vars );
{
local $LIST_SEPARATOR = q{!};

my @list = qw/this is my list/;

print "@list\n";
}

        [download]
        and then try to replace 'local' with 'my'.
        BTW, Perl::Critic does not complain about local'ising in the code above ;)

            s;;Just-me-not-h-Ni-m-P-Ni-lm-I-ar-O-Ni;;tr?IerONim-?HAcker ?d;print
[reply]
[d/l]
[select]

In Section Meditations