XaminProject / handlebars.php

Handlebars processor for php
331 stars 132 forks source link

FilesystemLoader and Testing #115

Open DaveStein opened 9 years ago

DaveStein commented 9 years ago

I just swapped my templates from mustache to handlebars using this lib. I have a unit test to verify that given a certain file, it will return a certain output.

// Calls Handlebars loadTemplate and then render on the template
$output = $this->_templater->view( 'place', [ 'friend' => 'Person' ] );
$this->assertEquals( '<body><p></p>Hello Person</body>', $output );

I am using https://github.com/mikey179/vfsStream to mock the file system, but that doesn't work with this lib because of realpath being used in the constructor.

The directory comes in as vfs://mydir. You can see in this popular Mustache loader, how they handle it: https://github.com/bobthecow/mustache.php/blob/master/src/Mustache/Loader/FilesystemLoader.php#L52

What do you think?

DaveStein commented 9 years ago

Alternatively the optional handling can go into one protected method while the directory checking goes into another, both of which would be called in constructor. Then people can extend to do custom behavior, such as accounting for vsf.

fzerorubigd commented 9 years ago

@DaveStein Thanks, I look into it if I get more spare time. And better if you can create a PR for this :)

DaveStein commented 9 years ago

@fzerorubigd which way do you like better? Accounting for :// inside or making it more extendable? Keep in mind to make it extendible I'd need to make $_baseDir protected rather than private.

fzerorubigd commented 9 years ago

@DaveStein using :// inside, is not good, changing the private field to protected is not a big deal. so I vote for the {more extendible} way.

DaveStein commented 9 years ago

kk I'll fork and make the change sometime between Sunday and Tuesday or so. I'll be away this weekend or I'd do it sooner :)

JustBlackBird commented 9 years ago

I believe supporting of custom stream wrappers (not only vfs://) could be useful in many cases not only for testing. For example they are used in Drupal 7 a lot. At the same time I don't like how the check is done in mustache.php. May be it's better to use something like stream_get_wrappers function.

What do you think @fzerorubigd ?

fzerorubigd commented 9 years ago

@JustBlackBird I like the idea.

DaveStein commented 9 years ago

@justblackbird @fzeroubigd i got my PR for this here https://github.com/XaminProject/handlebars.php/pull/117... maybe after that Just can add the streaming bit. My change makes things more extendable and then his makes things better on top of that.