List Info

Thread: PATCH for Catalyst::Plugin::ConfigLoader




PATCH for Catalyst::Plugin::ConfigLoader
user name
2007-08-21 08:54:14
Hi,

ConfigLoader seems to use regexps that are too limited in 
finalize_config. This means that when passing View::TT an
INCLUDE_PATH 
with more than one path specified (such as 
"__HOME__/template:__HOME__/template2"), only the
first __HOME__ is 
substituted. Attached is a patch to handle this case, and
the identical 
case for using __path_to(some/path)__.

Hope this is okay.

Greg.

-- 
Greg Sheard <greg.sheardwcn.co.uk>
Not speaking for WCN plc in any way.
#include "long_disclaimer.h"

_______________________________________________
Catalyst-dev mailing list
Catalyst-devlists.rawmode.org
http://lists.rawmode.org/mailman/listinfo/catalyst-dev


  
Re: PATCH for Catalyst::Plugin::ConfigLoader
country flaguser name
United States
2007-08-21 09:17:58
Greg Sheard wrote:
> ConfigLoader seems to use regexps that are too limited
in 
> finalize_config. This means that when passing View::TT
an INCLUDE_PATH 
> with more than one path specified (such as 
> "__HOME__/template:__HOME__/template2"), only
the first __HOME__ is 
> substituted. Attached is a patch to handle this case,
and the identical 
> case for using __path_to(some/path)__.

Thanks! Applied, except for one small change:

> +            s{__path_to((.+))__}{ $c->path_to(
split( '/', $1 ) ) }eg;

is now:

s{__path_to((.+?))__}{ $c->path_to( split( '/', $1 ) )
}eg;

so that the .+ is non-greedy.

-Brian

_______________________________________________
Catalyst-dev mailing list
Catalyst-devlists.rawmode.org
http://lists.rawmode.org/mailman/listinfo/catalyst-dev


Re: PATCH for Catalyst::Plugin::ConfigLoader
user name
2007-08-21 09:23:11
Brian Cassidy wrote:
> Greg Sheard wrote:
>> ConfigLoader seems to use regexps that are too
limited in 
>> finalize_config. This means that when passing
View::TT an INCLUDE_PATH 
>> with more than one path specified (such as 
>> "__HOME__/template:__HOME__/template2"),
only the first __HOME__ is 
>> substituted. Attached is a patch to handle this
case, and the 
>> identical case for using __path_to(some/path)__.
> 
> Thanks! Applied, except for one small change:
> 
>> +            s{__path_to((.+))__}{
$c->path_to( split( '/', $1 ) ) }eg;
> 
> is now:
> 
> s{__path_to((.+?))__}{ $c->path_to( split( '/', $1
) ) }eg;
> 
> so that the .+ is non-greedy.

Superb, cheers. The non-greedy change makes sense; I hadn't
thought 
about that!

Greg.

-- 
Greg Sheard <greg.sheardwcn.co.uk>
Not speaking for WCN plc in any way.
#include "long_disclaimer.h"

_______________________________________________
Catalyst-dev mailing list
Catalyst-devlists.rawmode.org
http://lists.rawmode.org/mailman/listinfo/catalyst-dev


Re: PATCH for Catalyst::Plugin::ConfigLoader
country flaguser name
United States
2007-08-21 20:05:02
Sorry to hijack this thread... but it reminded me...
something I've been
wanting for a while is:

  $c->config-> = {
      foo => sub { bar($_[0]) . baz($_[1]) }
      ...
  }

Then in the config file:

   hello: __foo(Hello,World)__
   ...

Basically, let me specify my own macros in addition to (or
overriding)
__HOME__ and __path_to()__.  I'll write this, but if you
want to do it
instead, all the better 

Regards,
Jonathan rockway


_______________________________________________
Catalyst-dev mailing list
Catalyst-devlists.rawmode.org
http://lists.rawmode.org/mailman/listinfo/catalyst-dev


Re: PATCH for Catalyst::Plugin::ConfigLoader
country flaguser name
United States
2007-08-22 10:03:16
On Aug 21, 2007, at 9:05 PM, Jonathan Rockway wrote:

> Sorry to hijack this thread... but it reminded me...
something I've  
> been
> wanting for a while is:
>
>   $c->config-> = {
>       foo => sub { bar($_[0]) . baz($_[1]) }
>       ...
>   }
>
> Then in the config file:
>
>    hello: __foo(Hello,World)__
>    ...
>
> Basically, let me specify my own macros in addition to
(or overriding)
> __HOME__ and __path_to()__.  I'll write this, but if
you want to do it
> instead, all the better 
>
I actually did something similar to this a while back, I
keep meaning  
to clean it up and document and tests, but you know how it
goes,  
never enough TUITs  

I took a slightly different approach though, abstracting out
the  
function that is applied to values, so you can subclass
it...

sub finalize_config {
     my $c = shift;
     my $v = Data::Visitor::Callback->new(
         plain_value => sub {
             return unless defined $_;
             $c->config_substitutions( $_ );
         }
     );
     $v->visit( $c->config );
}

sub config_substitutions {
     my $c = shift;
     for ( _ ) {
         s{ $c->path_to( '' ) }e;
         s{__path_to((.+))__}{ $c->path_to( split( '/',
$1 ) ) }e;
     }
}


This way you can add a config_substitutions method to your
app class  
for anything you want to add...

package MyApp;
sub config_substitutions {
	my $c = shift;
	$c->NEXT::config_substitutions( _ );
	for ( _ ) {
		s{__foo(.*?)__};
	}
}


-- 
Jason Kohles
emailjasonkohles.com
http://www.jasonkohles.co
m/
"A witty saying proves nothing."  -- Voltaire



_______________________________________________
Catalyst-dev mailing list
Catalyst-devlists.rawmode.org
http://lists.rawmode.org/mailman/listinfo/catalyst-dev


Re: PATCH for Catalyst::Plugin::ConfigLoader
country flaguser name
United States
2007-08-22 12:43:58
Jason Kohles wrote:
> On Aug 21, 2007, at 9:05 PM, Jonathan Rockway wrote:
> 
>> Sorry to hijack this thread... but it reminded
me... something I've been
>> wanting for a while is:
>>
>>   $c->config-> = {
>>       foo => sub { bar($_[0]) . baz($_[1]) }
>>       ...
>>   }
> 
> sub config_substitutions {
>     my $c = shift;
>     for ( _ ) {
>         s{ $c->path_to( '' ) }e;
>         s{__path_to((.+))__}{ $c->path_to( split(
'/', $1 ) ) }e;
>     }
> }

I've committed a patch that combines the above two options
[1].

-Brian

[1] http://dev.catalystframework.org/svnweb/Catal
yst/revision/?rev=6716

_______________________________________________
Catalyst-dev mailing list
Catalyst-devlists.rawmode.org
http://lists.rawmode.org/mailman/listinfo/catalyst-dev


[1-6]

about | contact  Other archives ( Real Estate discussion Medical topics )