Skip to content

Refactor template loading mechanism#8

Merged
thekid merged 17 commits intomasterfrom
refactor/template-loading
Sep 17, 2017
Merged

Refactor template loading mechanism#8
thekid merged 17 commits intomasterfrom
refactor/template-loading

Conversation

@thekid
Copy link
Member

@thekid thekid commented Sep 17, 2017

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.

Old code

Needs to use exceptions for flow control...

$previous= $templates->register('@partial-block', $this->fn);
try {
  return $context->engine->transform($this->name, $context, $this->start, $this->end, '');
} catch (TemplateNotFoundException $e) {
  return $this->fn->evaluate($context);
} finally {
  $templates->register('@partial-block', $previouss);
}

New code

Longer but can use exists() to check for templates

$source= $templates->source($this->name);
if ($source->exists()) {
  $previous= $templates->register('@partial-block', $this->fn);
  try {
    return $context->engine->render($source, $context, $this->start, $this->end, '');
  } finally {
    $templates->register('@partial-block', $previous);
  }
} else {
  return $this->fn->evaluate($context);
}

See xp-forge/handlebars#9 (review)

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)
* @return string The rendered output
*/
public function render($template, $arg, $start= '{{', $end= '}}', $indent= '') {
if ($template instanceof Template) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was dropped, use evaluate() if necessary.

* @test xp://com.github.mustache.unittest.FileBasedTemplateLoaderTest
*/
abstract class FileBasedTemplateLoader implements TemplateLoader, WithListing {
abstract class FileBasedTemplateLoader implements Templates, WithListing {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WithListing interface should be merged into the Templates class.

@thekid
Copy link
Member Author

thekid commented Sep 17, 2017

This is unfortunately not backwards compatible as the template loaders have changed type, and passing them to a method with type hint yields:

Argument 1 passed to com\handlebarsjs\HandlebarsEngine::withTemplates() must be an instance of com\github\mustache\TemplateLoader, instance of com\github\mustache\InMemory given

if ($source instanceof Source) {
return $source->compile($this->parser, $start, $end, $indent);
} else {
return new Template('<string>', $this->parser->parse(new StringTokenizer($source), $start, $end, $indent));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could also be:

$source= new Tokens('<string>', new StringTokenizer($source));
return $source->compile($this->parser, $start, $end, $indent); 

...but the inlined version saves us a couple of nanoseconds here:)

@thekid thekid merged commit 0d61ee2 into master Sep 17, 2017
@thekid thekid deleted the refactor/template-loading branch September 17, 2017 15:36
@thekid
Copy link
Member Author

thekid commented Sep 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant