Beefy Boxes and Bandwidth Generously Provided by pair Networks
Think about Loose Coupling

Re: Request for Perl::Critic Testimonials (some weirdnesses of the module)

by Ieronim (Friar)
on Jul 15, 2006 at 18:34 UTC ( #561468=note: print w/replies, xml ) Need Help??

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
They look very funny when they both refer to the line like this:
next if /^;/; #skip comments
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/;

The next bug: the line
local $" = '|';
causes two warnings:
Variable declared as 'local' Magic punctuation variable used

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
    It is complaining the xm switches are not included Change it to this,

    Update: Also wrong per Jeronim

    next if /^;/xms;
    local $" = '|';
    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{|};

    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{|}; }

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

      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' );
      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;
      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{|};

      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"; }
        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

Log In?

What's my password?
Create A New User
Node Status?
node history
Node Type: note [id://561468]
[jrmcc]: Your problem was pregnant, missing a period!

How do I use this? | Other CB clients
Other Users?
Others contemplating the Monastery: (13)
As of 2018-05-22 16:47 GMT
Find Nodes?
    Voting Booth?