[Templates] Bug report against Template::Plugin::URL

Josh Rosenbaum josh@infogears.com
Fri, 13 Jan 2006 01:09:28 -0700


This is a multi-part message in MIME format.
--------------020203090804030907060702
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit

> Sorry Josh, my attention has been elsewhere.
> This is what I've got outstanding from you:
[SNIP]
> * IGNORE_PLUGIN_BASE/IGNORE_STD_PLUGINS patch
>   http://tt2.org/pipermail/templates/2005-August/007691.html
> 
>   I don't like this I'm afraid.  On an aesthetic level, I think options
>   should be positive (DO_THIS) rather than negative (DONT_DO_THIS) wherever
>   possible.  So it would be better to allow the user to explicitly set
>   STD_PLUGINS and/or PLUGIN_BASE to undef.  Also, Template::Provider::Allow
>   and T::P::Deny provide a more general mechanism for restricting access
>   to resources.
> 
>   But above all else there's no tests or documentation, so that one doesn't 
>   make the grade.

Sorry for the delay in my response Andy. I've been extremely busy lately. I've got a better patch here (attached) and am working on docs. (Already created tests, but will attach later with docs.) Hopefully that will work better. I also am going to get rid of the STD_PLUGINS part of that previous patch as there was no real need for it. I must've been smoking crack at the time, as you can simply override Template::Plugins::STD_PLUGINS. ;) The PLUGIN_BASE deal cannot simply be set to undef, as we still add 'Template::Plugins' no matter what. So I'm adding a config option: 'ADD_DEFAULT_PLUGIN_BASE'. (As can be seen in patch.) I've also included another option "PLUGINS_USE_LOWERCASE" which just says that all keys in STD_PLUGINS and PLUGINS are lowercase, and that we can use those plugins in a case insensitive manner. (ie: [% USE TABLE %])

[SNIP]


>> I offered to enter them into CVS myself back in 
>> the day, but never heard back from Andy, so I'm assuming that's a rejection 
>> stamp for CVS access. ;)
> 
> Sorry Josh, but I don't have any recollection of you asking, so it's not a 
> rejection stamp.  However if you want to be a TT maintainer then you do need 
> to convince me that you would be as commited to the documentation and tests 
> as to the codebase. 

I don't have any problem with this at all. Whatever it takes to get fixes and important features in. :) We'll let my tests/docs speak for themselves once I have time to complete working on the docs for this patch. (Maybe a week from now as I'm pretty busy at the moment and it's hard to get time.)

Also, one other thing I noticed in a later email from you:

> Andy Wardley wrote:
>> > Yes, the default behaviour should stay the same for now.  We'll make it
>> > right for TT3.  
> 
> I've reverted the CVS version back to the original, broken behaviour.


I think the patch in CVS was fine from what I gathered. What we needed was to simply default the global $JOINER ($JOINT ;) ) to '&' rather than '&', which would create the same default behavior here. I think the patch suggested was a good one as it allowed changing the joiner which is probably important.

Happy New Year everyone,

-- Josh

--------------020203090804030907060702
Content-Type: text/plain;
 name="Plugins.pm.214.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
 filename="Plugins.pm.214.diff"

--- ../lib/Template/Plugins.pm  2004-10-04 04:27:39.000000000 -0600
+++ Template/Plugins.pm 2005-12-27 21:42:48.000000000 -0700
@@ -150,12 +150,17 @@
     if (ref $pbase ne 'ARRAY') {
         $pbase = $pbase ? [ $pbase ] : [ ];
     }
-    push(@$pbase, 'Template::Plugin');
+
+    $self->{ ADD_DEFAULT_PLUGIN_BASE } = !exists($params->{ ADD_DEFAULT_PLUGIN_BASE }) || $params->{ ADD_DEFAULT_PLUGIN_BASE } || 0;
+    if ($self->{ ADD_DEFAULT_PLUGIN_BASE }){
+      push(@$pbase, 'Template::Plugin');
+    }

     $self->{ PLUGIN_BASE } = $pbase;
     $self->{ PLUGINS     } = { %$STD_PLUGINS, %$plugins };
     $self->{ TOLERANT    } = $params->{ TOLERANT }  || 0;
     $self->{ LOAD_PERL   } = $params->{ LOAD_PERL } || 0;
+    $self->{ PLUGINS_USE_LOWERCASE } = $params->{ PLUGINS_USE_LOWERCASE } || 0;
     $self->{ FACTORY     } = $factory || { };
     $self->{ DEBUG       } = ( $params->{ DEBUG } || 0 )
                              & Template::Constants::DEBUG_PLUGINS;
@@ -177,7 +182,7 @@
     my ($self, $name, $context) = @_;
     my ($factory, $module, $base, $pkg, $file, $ok, $error);

-    if ($module = $self->{ PLUGINS }->{ $name }) {
+    if ($module = $self->{ PLUGINS }->{ ($self->{ PLUGINS_USE_LOWERCASE } ? lc($name) : $name) } ) {
         # plugin module name is explicitly stated in PLUGIN_NAME
         $pkg = $module;
         ($file = $module) =~ s|::|/|g;
@@ -263,7 +268,7 @@
     my $format = "    %-16s => %s\n";
     my $key;

-    foreach $key (qw( TOLERANT LOAD_PERL )) {
+    foreach $key (qw( TOLERANT LOAD_PERL PLUGINS_USE_LOWERCASE ADD_DEFAULT_PLUGIN_BASE)) {
        $output .= sprintf($format, $key, $self->{ $key });
     }

--------------020203090804030907060702--