-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Partial blocks and inline partials #9
Conversation
try { | ||
$templates->register('@partial-block', $this->fn); | ||
return $context->engine->transform($this->name, $context, $this->start, $this->end, ''); | ||
} catch (TemplateNotFoundException $e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nicer if mustache template loaders would either:
- have a
provides()
method to test for template existance, or - accept a
default
closure to execute if the template does not exist
Catching an exception for control flow does not feel right [TM]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of throwing an exception when load() is called, an Input instance is returned. Its exists() method can be used to test whether the loaded template exists. See xp-forge/handlebars#9 (review)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Templates
class needs some more tests
|
||
/** @return com.github.mustache.TemplateListing */ | ||
public function listing() { | ||
return $this->delegate->listing(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not correct, the listing is missing the templates registered in $this->templates
; also, it fails if $this->delegate
is null.
*/ | ||
public function register($name, $content) { | ||
$previous= isset($this->templates[$name]) ? $this->templates[$name] : null; | ||
if ($content instanceof Node) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This branch is not tested
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ Done!
@@ -13,6 +13,8 @@ static function __static() { | |||
self::$byName['unless']= XPClass::forName('com.handlebarsjs.UnlessBlockHelper'); | |||
self::$byName['with']= XPClass::forName('com.handlebarsjs.WithBlockHelper'); | |||
self::$byName['each']= XPClass::forName('com.handlebarsjs.EachBlockHelper'); | |||
self::$byName['>']= XPClass::forName('com.handlebarsjs.PartialBlockHelper'); | |||
self::$byName['*inline']= XPClass::forName('com.handlebarsjs.InlinePartialHelper'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pull request does not support generic decorators as e.g.
{{#grid data}}
{{#*column "Column 1"}}{{foo}}{{/column}}
{{#*column "Column 2"}}{{bar}}{{/column}}
{{/grid}}
...just *inline
as a special case! This is the same as the original implementation in handlebars-java
|
||
$source= $templates->source($this->name); | ||
if ($source->exists()) { | ||
$previous= $templates->register('@partial-block', $block); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsure about correct context here, need some more tests...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decorators aren't 100% correct yet
$source= $templates->source($this->name); | ||
if ($source->exists()) { | ||
$this->fn->enter($context); | ||
$previous= $templates->register('@partial-block', $this->fn); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will re-run the decorators unnecessarily...
Reading https://cloudfour.com/thinks/the-hidden-power-of-handlebars-partials/ motivated me to implement partial blocks and inline partials.
page.handlebars
layout.handlebars
See also http://handlebarsjs.com/partials.html and handlebars-lang/handlebars.js#1082