nervetattoo / li3_twig

Fork of the Lithium Twig integration
5 stars 7 forks source link

do not overwrite the default handler #1

Open eLod opened 13 years ago

eLod commented 13 years ago

i'm new to li3 so please forgive me if i overlooked something, but i installed both li3_twig and li3_docs plugins and after activating twig everything disappeared as it replaces the default handler (when registering the media type in boostrap). i created a short workaround to be able to use the old default config and use twig if a template is presented, however i think the right approach would be to get the View/Media check the presented templates, eg. one could register multiple templating engines and the View/Media should pick the right one (based on preference and presence). i'm not sure where to stick this logic and how to make the configuration convenient. another point would be to support mixed types (eg. simple php layout with twig template).

diff --git a/config/bootstrap.php b/config/bootstrap.php
index 8dcbf6e..14120a4 100644
--- a/config/bootstrap.php
+++ b/config/bootstrap.php
@@ -20,7 +20,7 @@ Libraries::add('Twig', array(
 /**
  * Add Twig to recognized media types.
  */
-Media::type('default', null, array(
+Media::type('twig', null, array(
        'view' => '\lithium\template\View',
        'loader' => '\li3_twig\template\Loader',
        'renderer' => '\li3_twig\template\view\adapter\Twig',
@@ -30,4 +30,32 @@ Media::type('default', null, array(
        )
 ));

+/**
+ * Add filter to detect Twig template.
+ */
+Media::applyFilter('_handle', function($self, $params, $chain) {
+    if ($params['handler']['view']) {
+       $twig_handler =  $self::invokeMethod('_handlers', array('twig'));
+       $twig_handler = array_filter($twig_handler, function($val) { return $val; });
+       $handler = $twig_handler + $params['handler'];
+       $options = $handler;
+
+       if (isset($options['request'])) {
+           $options += $options['request']->params;
+           unset($options['request']);
+       }
+
+       $loader = Libraries::instance('adapter.template.view', $handler['loader'], $options);
+       unset($options['data'], $options['paths']);
+       $options = array_filter($options, function($val) { return $val && is_string($val); });
+       foreach((array) $loader->template('template', $options) as $path) {
+           if (is_file($path)) {
+               $params['handler'] = $handler;
+               break;
+           }
+       }
+    }
+    return $chain->next($self, $params, $chain);
+});
+
 ?>
nervetattoo commented 13 years ago

Hi,

You are absolutely right; the plugin should work this way :) Its just not come up yet, the need to mix different template libraries in one app.

I dont really like checking for presence, its unneeded overhead 80% of the time (you normally use just 1 templating system). Statting the filesystem should be last resort as its a scaling and performance issue.

Apart form that, your fix works great. I primarily see three approaches:

  1. File system presence
  2. Explicitly configuring for your app when registering twig by stating which controllers uses twig.
  3. Fallback to inbuilt php templates when an exception is thrown for missing template.
  4. Somehow configure the actual foreign library that does not use twig to use php templates. (Twig creating the support for it)

And I dont really like any of them, and number 4 I dont know if is very doable at all. What would you prefer, strictly from the usage side?

eLod commented 13 years ago

i think it should be like:

i don't know if this sounds reasonable, i tried to cover the major use-cases.