Fixed bug in Twig_Loader_Cakephp interface

[lhilton]

Fixed bug in Twig_Loader_Cakephp interface that prevented multiple-template includes and fallback behaviors.

Twig_LoaderInterface defined: https://github.com/fabpot/Twig/blob/master/lib/Twig/LoaderInterface.php

Twig include "array of templates" defined: http://twig.sensiolabs.org/doc/tags/include.html

Signed-off-by: Lee Hilton lee.hilton@hipstercreative.com

[WyriHaximus]

Thanks for the PR @lhilton it looks great but could you take a look at the tests aswel before I merge it?

[lhilton]

It is failing because the interface is now emitting the Twig_Error_Loader event for missing templates. It looks like Twig_Loader_cakephpTest.php is testing for View/layout.tpl, which is apparently missing.

Either that is the wrong template file, or it needs to be included in the project. I am not sure what your intent is with trying to load this file. I am also not sure how your test is building the environment for this plugin either. If you could provide some direction for this, I can do another commit.

On April 16, 2014 at 10:56:24 AM, Cees-Jan Kiewiet (notifications@github.com) wrote:

Thanks for the PR @lhilton it looks great but could you take a look at the tests aswel before I merge it?

—
Reply to this email directly or view it on GitHub.

[WyriHaximus]

Yeah I see, your changes require new tests. In the original test it just did a call to resolve the file name and return that as cache key. But since you changed that and do a file_exists check and thrown an exception tests need to test all possible outcomes.

I have a separate cake installation on my local development machine where I develop and test plugins on. Loading the deps with composer globally inside that installation :(. (Can't wait for Cake3 tbh, but this does for now.) Also preparing the environment is scripted: https://github.com/WyriHaximus/travis/blob/master/before_script.sh If you feel unconfortable doing that I don't mind doing it myself, you made a great PR even without tests.

[lhilton]

I’ll give it a go, I’m going to start forcing 100% test coverage with my employees on the next project we start. This will give me a learning opportunity ahead of that.

I’ll give your before_script.sh a look and possibly ping you with any questions that come out of it.

On April 16, 2014 at 12:52:45 PM, Cees-Jan Kiewiet (notifications@github.com) wrote:

Yeah I see, your changes require new tests. In the original test it just did a call to resolve the file name and return that as cache key. But since you changed that and do a file_exists check and thrown an exception tests need to test all possible outcomes.

I have a separate cake installation on my local development machine where I develop and test plugins on. Loading the deps with composer globally inside that installation :(. (Can't wait for Cake3 tbh, but this does for now.) Also preparing the environment is scripted: https://github.com/WyriHaximus/travis/blob/master/before_script.sh If you feel unconfortable doing that I don't mind doing it myself, you made a great PR even without tests.

—
Reply to this email directly or view it on GitHub.

[WyriHaximus]

Ok cool, let me know if you run into anything.

[WyriHaximus]

@lhilton Did you have any change to work on this? If not, I'm going ahead tomorrow merging your PR and fixing the tests myself :).