From: Thomas Sibley Date: Fri, 6 Nov 2015 02:51:30 +0000 (-0800) Subject: Revert incorrect fixes for parsing of attributes with empty parens X-Git-Tag: 0.090103~7 X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=1d00b2ffb8806bd0a8190ee8580a85895e48f8e4 Revert incorrect fixes for parsing of attributes with empty parens 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. --- diff --git a/lib/Catalyst/Action.pm b/lib/Catalyst/Action.pm index ea3e3e4..3497e21 100644 --- a/lib/Catalyst/Action.pm +++ b/lib/Catalyst/Action.pm @@ -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 diff --git a/lib/Catalyst/Controller.pm b/lib/Catalyst/Controller.pm index 1766168..d0a8570 100644 --- a/lib/Catalyst/Controller.pm +++ b/lib/Catalyst/Controller.pm @@ -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 ) {