Lazily evaluate blocks pragmas. #265
Conversation
The specific example in the docs page about what does not work, is exactly what we need to make this useful. Fixes bobthecow#264
|
Note: the style checks are failing - but I haven't touched any of those lines. The unit tests are passing on all branches, but then it is running the style checks again and failing. |
|
Yeah. Unfortunately style checks are a bit of a moving target, and it's pretty hard to lock them down to a specific version. As long as code you write passes the style checks, I wouldn't worry about it. |
|
Thanks for taking this on, it'll make things a lot better. That said, I'm super not a fan of the eval :) Any ideas for doing it without? |
|
Yeah - I don't like the eval either - I should be able to create a real function in the class (not anonymous) and pass it around instead - this should be quicker and avoid the eval nastiness. |
|
Maybe we could use actual inheritance to implement this? It seems like it should work: foo.mustache:<!DOCTYPE html>
<html>
<head>
<title>{{$ title }}default title{{/ title }}</title>
{{$ stylesheets }}
<link rel="stylesheet" href="/styles.css">
{{/ stylesheets }}
</head>
<body>
{{$ body }}{{/ body }}
</body>
</html>bar.mustache:{{< foo }}
{{$ title }}this is my awesome title{{/ title }}
{{$ body }}
<p>Some body stuffs here</p>
{{/ body }}
{{/ foo }}compiled templates:class FooTemplate {
public function render() {
return (
<!DOCTYPE html>
<html>
<head>
<title>{$this->blockd5d3db1765287eef77d7927cc956f50a()}</title>
{$this->blockc7f5051a037ab8a1011827d1a393494b()}
</head>
<body>
{$this->block841a2d689ad86bd1611447453c22c6fc()}
</body>
</html>
);
}
// md5('title') block
protected function blockd5d3db1765287eef77d7927cc956f50a() {
return 'default title';
}
// md5('stylesheets') block
protected function blockc7f5051a037ab8a1011827d1a393494b() {
return (
<link rel="stylesheet" href="/styles.css">
);
}
// md5('body') block
protected function block841a2d689ad86bd1611447453c22c6fc() {
return '';
}
}
class BarTemplate extends FooTemplate {
// md5('title') block
protected function blockd5d3db1765287eef77d7927cc956f50a() {
return 'this is my awesome title';
}
// md5('body') block
protected function block841a2d689ad86bd1611447453c22c6fc() {
return (
<p>Some body stuffs here</p>
);
}
}(Note that the "compiled templates" here are more pseudocode than actual output, but you should get the idea) |
|
@jazzdan Poke. Have any thoughts on the matter? :) |
|
The problem with using actual template inheritance will be when a template includes more than one template. Right now I'm thinking I can add the function to the template class, and pass it through the block context as a callable. |
|
Ahh, right. Partials wouldn't play nice with that. 👍 for a function on the template class and a callable that you pass around in the block context. That seems doable. |
damyon
force-pushed
the
PULL_264
branch
2 times, most recently
from
ceb2267
to
8897e1e
Compare
|
OK - this version looks good - all unit tests are passing (I added one more). It includes those block function changes - no more eval. |
| $value = $this->section%s($context, \'\', true); | ||
| $newContext[%s] = %s$value; | ||
| $blockid = %s; | ||
| $newContext[%s] = array($this, \'block\' . $blockid); |
There was a problem hiding this comment.
the $blockid is guaranteed to be a string (md5) so why not compile to a string literal?
$newContext[%s] = array($this, \'block%s\');You'd need to pass in both the exported and unexported $id, but that seems worthwhile?
|
I'm a lot more comfortable with this. It's past my bedtime now, so I'll need to review it in more detail when I'm a bit more alert :) |
|
Thanks for the review - I'll fix those things over the weekend. |
|
Fixed those 2 things you noted above. |
|
Thanks again! This has been merged into the dev branch, and will go in the next release. |
|
This has been released in v2.9.0 :) |
Patch for issue #264.
The unit tests are unmodified and all passing - the new unit tests cover the exact use case documented on the blocks pragma page, and include an additional test for whitespace conformance.
With regards to the solution, I originally tried using a lambahelper to reparse the source - but hit whitespace issues because there is logic in the tokeniser about removing whitespace for standalone tags etc. This solution creates an anonymous function to defer the execution of the generated code.