fvsch / kirby-twig

Twig templating support for Kirby CMS 2. For Kirby 3, use https://github.com/amteich/kirby-twig
MIT License
70 stars 8 forks source link

Add support for plugins #10

Closed thasmo closed 7 years ago

thasmo commented 8 years ago

In my plugin I register a template:

$kirby->set('template', 'commands', __DIR__ . '/templates/commands.php');

When I change this to use a twig extension ...

$kirby->set('template', 'commands', __DIR__ . '/templates/commands.twig');

... it doesn't work anymore.

Do I have to create an empty commands.php template and just add the commands.twig or can I get rid of the commands.php file somehow?

thasmo commented 8 years ago

Ow, actually I can't get it work with plugins at all.

$kirby->set('template', 'commands', __DIR__ . '/templates/commands.php'); just uses the PHP template.

$kirby->set('template', 'commands', __DIR__ . '/templates/commands.twig'); throws an error.

Twig_Error_Loader
S:\Project\dsta\website-learn\public\site\templates\:
Unable to find template "S:/Project/dsta/website-learn/public/site/plugins/dsta/templates/commands.twig" (looked into: S:\Project\dsta\website-learn\public\site\templates).
thasmo commented 8 years ago

Had a look at the source code and figured out that plugin support is probably still missing. I wrote a quick hack to get it running for plugins, but it's definitely not a secure way. The core change to use plugin templates is to support multiple template directories, e.g. all templates directories of installed plugins.

fvsch commented 8 years ago

Hey, thanks for the report. Indeed for now there is a single template include root: site/templates. I’m thinking about expanding it to more directories.

One solution would be to change the templating root to site, but existing templates would need to change all {% include 'something.twig' %} to {% include 'templates/something.twig' %}, so this can only be done in a 3.0 release.

Another solution would be to declare three roots in the Twig Environment (in that order):

  1. site/snippets
  2. site/templates
  3. site/plugins

So that:

Variant: use site/templates and site as roots.

Finally: maybe provide a setting to allow users to override template roots.

Any thoughts?

thasmo commented 8 years ago

I think namespaces could be the proper way to go. There are bundle namespaces when using it in combination with Symfony, but in standalone there is still a simple namespace-mechanism which should be enough in a Kirby context. I haven't dug into this too deep but I think it should work.

thasmo commented 8 years ago

Using the simple namespace, plugin templates could be loaded/accessed like this:

$loader->addPath('site/plugins/pluginname/templates/', 'pluginname');
$twig->render('@pluginname/index.html', array());

This will also enable a plugin to include/render templates from another plugin and the site.

Looks like a perfect fit for Kirby plugins.

fvsch commented 8 years ago

We have to account for the fact that plugins are run in alphabetical order and the Twig plugin will be run late (and in any case there’s not way to run it first), so other plugins don’t have access to it. Only workaround is to set some config. So a plugin could do:

$kirby->set('option', 'twig.namespace.pluginname', __DIR__ . DS . 'templates');

And the Twig plugin could look up all twig.namespace.* options (basically looping on all options) and declare the namespaced paths. (A single twig.namespaces option with a key=>value array would be better but that means each plugin must add a new value without overwriting previous settings, and that’s a bit risky.)

So that’s doable, but I’m not sure how it solves your use case. Could you describe it more in depth?

For instance if you just need to make this work:

$kirby->set('template', 'commands', __DIR__ . '/templates/commands.twig');

it can be done by declaring site/plugins as a secundary include path, and trimming /root-dir/site/plugins from the path before rendering it. Hackish (as is the current solution for site/templates templates anyway), but it works.

On the other hand, declaring a namespace might work too, and is not much more complicated:

$kirby->set('option', 'twig.namespace.pluginname', __DIR__ . DS . 'templates');
$kirby->set('template', 'commands', '@pluginname/commands.twig');

I’m tempted to go with the simpler solution, but that second solution (with Twig namespaces) is a bit more elegant and might help with things like installing a plugin with Composer in the vendor directory.

Do you have a secundary use case where the namespace solution would be useful?

fvsch commented 8 years ago

For the record, I plan to go with Twig namespaces and:

And that’s about it for the default namespaces: templates, snippets, plugins.

You could then register a template as:

$kirby->set('template', 'commands', '@plugins/pluginname/templates/commands.twig');

But if you want to enable installation with Composer (in the vendor directory), it’s probably better to register your own namespace:

$kirby->set('option', 'twig.namespace.pluginname', __DIR__ . DS . 'templates');
$kirby->set('template', 'commands', '@pluginname/commands.twig');

I still need to write the actual code that does that, but it shouldn't be difficult.

fvsch commented 8 years ago

I’ve added support for Twig namespaces (latest commit in master). @thasmo, do you want to try it out?

I’m also wondering about the security implications. If a user defines a namespace pointing to a folder below the site root, it could give (read-only) access to any file. I need to check if Twig’s include() and source() obey PHP's include_path settings, to get started.

fvsch commented 7 years ago

I can confirm that Twig’s include() and source() functions do obey PHP’s open_basedir setting (obviously). That’s good enough for me, since we’re not doing anything more risky than a PHP template.

I’ve also documented the namespace feature. So I’ll close this issue now. @thasmo, if you have any problems, please reopen.