[RFC|Merged] Improved template engine

[Oleg]

From the pull request:

A big issue here is that the entire _tpl_eval path appears to be busted. Forcing it to be taken in template#display breaks tests.

If anyone feels like taking a stab at it please go ahead.

[Oleg]

I took out most/all of the affected code under the umbrella of removing db storage of templates.

I am now further refactoring the template engine to return an "executor" (which I think I will rename to "rendered") object that can render a template, with implementations for include and eval-based rendering. https://github.com/p/phpbb3/commit/d06e ... dcf8cd524e

This broke everything because the template engine expects $this to have template variables, which in the executor/renderer is not the case. Therefore I will be adding a "context" class which will store variables used during template evaluation.

[nickvergessen]

(15:52:28) nickvergessen: well nn-, you have 4 template_* files
(15:52:38) nickvergessen: i think this is enough for a new folder
(15:53:45) nickvergessen: we already have tones of files in includes/
(15:53:59) nn-: if you feel strongly about it i'll move them
(15:54:00) nickvergessen: and most users will never need to touch the template engine
(15:54:09) nickvergessen: so I really think we can "hide" them in a folder

[Oleg]

I unbroke the obvious breakage but php code inclusion should be broken still as it has no test for it.

Everything relating to template inheritance needs to be tested, I'm not sure there are any automated tests for that in which case they should be added if possible.

php inclusion from scripts in subdirectories should be tested as well.

I will rename executor to renderer, move template classes into template subdirectory and get rid of leading underscores in various variable and function names.

Use of references needs to be checked.

[igorw]

Good work, go on.

[TerryE]

This broke everything because the template engine expects $this to have template variables, which in the executor/renderer is not the case. Therefore I will be adding a "context" class which will store variables used during template evaluation.

Why bother? I had to solve this prob in my own templating engine, and I did this by passing the equivalent of $this->_tpldata to the generated template as an argument (as $this does have the correct context for the wrapper which calls the renderer). If the argument is a value and not a reference, then (somewhat counter-intuitively) this a cheap operation due to the internal PHP RTS reference clone copy-on-write storage model.

[Oleg]

Two reasons:

1. There are at least two variables in the context (tpldata and rootref).

2. This moves data assignment functions' implementation into the context, more clearly separating them from other responsibilities of the template class (such as template location).

[TerryE]

OK, so what you are really talking about is a definite architecture change to the templating system: that is the separation on the data context and the template renderer. The data object contains all of the template data including _tpl_data and _rootref, and the render object uses the data object to render the html document. The only two other $this methods which get bound into the rendered code are _tpl_include and _php_include which could just as validly be static methods.

[Oleg]

I have been doing a bunch of work on this and I am considering splitting template location (this may or may not be the same as "loading") into another class.

It is the last "implementation" aspect still embedded in the template class. With the loading split we will have the following pieces:

• template - facade/driver/glue for the entire template engine, the only class client code directly instantiates and deals with. Has functions for assigning and clearing variables, setting "handle" to path mapping, displaying the templates.
• locator/loader - responsible for maintaining handle to path mappings, figuring out which file contains source for a given handle.
• compiler - compiles a given template source file, either returns compiled code or saves it to specified destination file.
• renderer - displays compiled template (include and eval implementations).

---

Something that was discussed on IRC:

php file inclusion will be performed relative to "php include root" which will be a parameter passed to template class. This will be set to phpbb_root in normal use and test directory in tests.

[Oleg]

Something that was discussed on IRC:

php file inclusion will be performed relative to "php include root" which will be a parameter passed to template class. This will be set to phpbb_root in normal use and test directory in tests.

I decided to not have a separate "php include root" variable, since it would be part of public API but currently it is only useful for tests. Tests have been hacked slightly to include relative to phpbb root.

One of the last remaining issues is the $orig_tpl_inherits_id variable. It seems to me that it should go away, but I'm having a hard time figuring out a use case when it will be actually used. I will add a test for template inheritance and kill $orig_tpl_inherits_id off most likely.