Skip to content
This repository has been archived by the owner on Jan 26, 2019. It is now read-only.

FilesystemLoader and Testing #115

Open
DaveStein opened this issue May 13, 2015 · 8 comments
Open

FilesystemLoader and Testing #115

DaveStein opened this issue May 13, 2015 · 8 comments

Comments

@DaveStein
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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

@DaveStein
Copy link
Contributor Author

@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
Copy link
Contributor

@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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link
Contributor

@JustBlackBird I like the idea.

@DaveStein
Copy link
Contributor Author

@JustBlackBird @fzeroubigd i got my PR for this here #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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@DaveStein @fzerorubigd @JustBlackBird