[RFC|Merged] Improved template engine

These requests for comments/change have lead to an implemented feature that has been successfully merged into the 3.1/Ascraeus branch. Everything listed in this forum will be available in phpBB 3.1.
Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Improved template engine

Post by 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
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Improved template engine

Post by 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.

User avatar
nickvergessen
Former Team Member
Posts: 733
Joined: Sun Oct 07, 2007 11:54 am
Location: Stuttgart, Germany
Contact:

Re: [RFC|Accepted] Improved template engine

Post by 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 :P
(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
Member of the Development-TeamNo Support via PM

Oleg
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Improved template engine

Post by 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
Registered User
Posts: 500
Joined: Thu Jan 04, 2007 11:47 pm

Re: [RFC|Accepted] Improved template engine

Post by igorw »

Good work, go on. :)

TerryE
Registered User
Posts: 95
Joined: Sat May 23, 2009 12:24 am
Contact:

Re: [RFC|Accepted] Improved template engine

Post by TerryE »

nn- wrote: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
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Improved template engine

Post by 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
Registered User
Posts: 95
Joined: Sat May 23, 2009 12:24 am
Contact:

Re: [RFC|Accepted] Improved template engine

Post by 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
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Improved template engine

Post by 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
Posts: 1150
Joined: Tue Feb 23, 2010 2:38 am
Contact:

Re: [RFC|Accepted] Improved template engine

Post by Oleg »

nn- wrote: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.

Post Reply