Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

do not overwrite the default handler #1

Open
eLod opened this issue Jul 4, 2011 · 2 comments
Open

do not overwrite the default handler #1

eLod opened this issue Jul 4, 2011 · 2 comments

Comments

@eLod
Copy link

eLod commented Jul 4, 2011

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
Copy link
Owner

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
Copy link
Author

eLod commented Jul 4, 2011

i think it should be like:

  • rename the default to simple (or any canonical name), make it the default (e.g. double it)
  • twig plugin should register with another name and change the default to itself preferable only for the app and not for external libraries (but should be configurable to disable this)
  • render should use the default, unless explicitly overriden with something like $this->_render['type'] (on a per action basis, or even better filterable)
  • plus: make it configurable (e.g. by path, exclude/include rules, etc.) to which controllers it should apply to, e.g. enable only in app/, or excluding a specific library,etc.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants