Revert incorrect fixes for parsing of attributes with empty parens
Thomas Sibley [Fri, 6 Nov 2015 02:51:30 +0000 (18:51 -0800)]
This partially reverts commits:

  • "Args() wasn't being processed as unlimited number of args, due to…" (43b44b3)
  • "Fixed :Path() empty brackets attribute not registering action. Fixes: #104" (dd6a9f2)

leaving the tests added by them for the Path() bug in GH #104 but
reverting the code changes.

The code changes caused Attribute() to parse equivalent to Attribute('')
instead of Attribute.  The mistake was not noticed because Path, Path(),
and Path('') should all produce equivalent behaviour.  However, the same
is not true of other attributes.  For example, Args.

Parsing empty parens as undef rather than an empty string is the correct
solution.  Empty strings are achieved explicitly with Attribute('').

Reverting these changes fixes the regression in the handling of Args(),
which started producing the following error at app startup under debug
mode:

    Out of memory during list extend
    at .../Catalyst/DispatchType/Chained.pm line 101.

The following commit will make the correct fix.

lib/Catalyst/Action.pm
lib/Catalyst/Controller.pm

index ea3e3e4..3497e21 100644 (file)
@@ -52,9 +52,7 @@ has number_of_args => (
     if( ! exists $self->attributes->{Args} ) {
       # When 'Args' does not exist, that means we want 'any number of args'.
       return undef;
-    } elsif(
-      !defined($self->attributes->{Args}[0]) || 
-      $self->attributes->{Args}[0] eq '' ) {
+    } elsif(!defined($self->attributes->{Args}[0])) {
       # When its 'Args' that internal cue for 'unlimited'
       return undef;
     } elsif(
@@ -140,7 +138,6 @@ has args_constraints => (
 
     return [] unless scalar(@arg_protos);
     return [] unless defined($arg_protos[0]);
-    return [] if ($arg_protos[0] eq '' && scalar(@arg_protos) == 1);
 
     # If there is only one arg and it looks like a number
     # we assume its 'classic' and the number is the number of
index 1766168..d0a8570 100644 (file)
@@ -399,7 +399,7 @@ sub _parse_attrs {
 
         # Parse out :Foo(bar) into Foo => bar etc (and arrayify)
 
-        if ( my ( $key, $value ) = ( $attr =~ /^(.*?)(?:\(\s*(.*?)\s*\))?$/ ) )
+        if ( my ( $key, $value ) = ( $attr =~ /^(.*?)(?:\(\s*(.+?)\s*\))?$/ ) )
         {
 
             if ( defined $value ) {