XaminProject / handlebars.php

Handlebars processor for php
331 stars 134 forks source link

Added custom template class option #102

Closed jeffturcotte closed 9 years ago

jeffturcotte commented 9 years ago

This enhancement is fully backwards compatible and is meant to replicate some of the extensibility that is available in Twig.

My specific use case involves custom loaders/template classes that read yaml front-matter which can alter the context. I was able to achieve this in Twig through the use of a loader and their 'base_template_class' option and wanted to achieve similar functionality with this lib, so I added a 'template_class' option. The Template object now also receives it's name (when applicable) as that is important to retrieve any metadata that may be associated with it.

The new option ensures the class is a subclass of Handlebars\Template so current type hints continue to work throughout library code and custom helpers that developers may have written. Let me know if you have any comments/questions.

Usage:

<?php
use Handlebars\Loader\FilesystemLoader;
use Handlebars\Handlebars;
use Handlebars\Template;

class CustomTemplate extends Template {
    public function render($context)
    {
        // do whatever I want!
    }
}

$handlebars = new Handlebars([
    'loader' => new FilesystemLoader('/path'),
    'template_class' => 'CustomTemplate'
]);
fzerorubigd commented 9 years ago

Could you please provide some test for this?

JustBlackBird commented 9 years ago

I just want to insert my two cents.

I believe that template_class option can be useful for the core, and can be used in many projects.

At the same time I'm not sure about template name feature. @jeffturcotte why do you need it in the core? Why don't you just extend Handlebars\Handlebars class? As far as I know there is no similar feature in Handlebars.js and it works fine. So if we add the template name feature we make a step away from Handlebars.js.

fzerorubigd commented 9 years ago

we want to be compatible with handlebars.js in syntax, not implementation. so as long as the syntax is ok, then everything is open to change. also I know this template class means we can support other syntax, as long as the main handlebars functionality is there, I'm ok with that.

On Tue, Dec 16, 2014 at 1:38 PM, Dmitriy S. Simushev < notifications@github.com> wrote:

I just want to insert my two cents.

I believe that template_class option can be useful for the core, and can be used in many projects.

At the same time I'm not sure about template name feature. @jeffturcotte https://github.com/jeffturcotte why do you need it in the core? Why don't you just extend Handlebars\Handlebars class? As far as I know there is no similar feature in Handlebars.js and it works fine. So if we add the template name feature we make a step away from Handlebars.js.

— Reply to this email directly or view it on GitHub https://github.com/XaminProject/handlebars.php/pull/102#issuecomment-67137112 .


http://fzero.rubi.gd

JustBlackBird commented 9 years ago

@fzerorubigd I agree that implementations can be different. The template_class is only about implementation. But if we talking about name option it's more like a new feature that has no (and will have no) equivalent in JS version.

Handlebars.js uses very simple approach: template is just a raw source without any meta data. Thus template itself does not need its own name. All including, referencing, inheritance, etc. features are done with helpers or partials but not within a template renderer. That is why I want to know why @jeffturcotte needs it.

Frankly I'm ok with the PR :smile:. It's just thoughts about templates portability between PHP and JS versions of Handlebars and library design stuff.

jeffturcotte commented 9 years ago

@fzerorubigd Yes. I will add tests.

@JustBlackBird My specific use case for the name option is that my custom template class needs to access external metadata (stored in a custom loader) that is associated with the template. The only other solution I could work out to achieve this was hashing the source of the template in both the loader and the template class so it could identify itself. That would work, but seemed over the top and computationally intensive. By having the name option, there is an easy way for the template to have knowledge of its identity and custom Loaders can get superpowers. I think the name option is pretty simple and unobtrusive, but I'm open to other options here if you all have any ideas.

The name option does not affect the compatibility of the rendering engine between js and php. If someone created a custom template class (in php) that DID alter rendering, then it would be their responsibility to create the js equivalent in order to share templates. This is also true if you create a custom helper.

I did approach this at first by trying to extend Handlebars\Handlebars, but because a lot of the API is private, it would've been impossible without copying key tokenizing methods, which I would prefer to defer those to the core lib. I did think of creating a PR that altered some of private methods to protected, but came to the conclusion that this solution would be better long-term.

Thanks for the review.

jeffturcotte commented 9 years ago

@fzerorubigd @JustBlackBird

I added tests and just got rid of that $name stuff in Template for now. I still think it's harmless, but I found a different workaround and I'd rather be conservative with the enhancement for now. Thanks!

fzerorubigd commented 9 years ago

@jeffturcotte thank you. @JustBlackBird since the name parameter is out, there is no problem, so I merge this request.