Upgrading Twig breaks Stacey

[andremalenfant]

There was a shortcut taken when integrating Twig. In Twig/Template.php function getAttribute, two lines of code were added to return the Stacey Page object for a string attribute corresponding to a path, using AssetFactory::get. This has to be added back when upgrading Twig. (took me a while to figure out).

The best approach would be to either build the whole graph of Page objects in the page data instead of just adding the page paths in the children array but for now, I added a Twig extension to my install to retrieve the Page object from the template.

[kolber]

The main reason I avoid building the full graph of page data inside the Page object itself (aside from the performance issues it would raise) is that it ends up creating circular object references. The page.parent's page.children contains the current page.

This is why we store arrays of unique identifiers (page urls) and only grab the necessary page data object when it is needed.

It would be great to see your Twig extension if you don't mind sharing it. I would really love to remove the changes I had to make to the Twig source, so that it can be upgraded independently.

[Sija]

@kolber You can subclass Twig_Template and use getAttribute() method to resolve path through Stacey's internals (check out https://github.com/Sija/Gizmo/blob/master/app/src/Gizmo/Twig/Template.php for example)

[seep]

I'm also having an issue with upgrading Twig. After adding the two lines Andre mentioned (which got Twig working again), I got this error when using the Stacey get function in a template:

An exception has been thrown during the compilation of a template ("Attribute "callable" does not exist for Node "Twig_Node_Expression_Function".") in "index.html".

I guess there has been a breaking change to Twig extensions. I haven't figured out a fix yet.

[andremalenfant]

Building the complete object graph in memory could defenitely be problematic for big sites, where there is not real need to transverse the whole hierarchy to render a page. On small sites thought, that would not be a problem and the performance/ressource issue would be a moot point as you most of the time need to generate a menu that contains all pages... The circular reference problem could be avoided if that's the direction you wanted to take but it might not be the good approach anyways...

If we go for the premise that the whole graph is not required/wanted, my solution was to add a function in twig-extensions.inc.php, similar to the get function, but much simpler:

#

get Asset object for a given path

# function getAsset($url) { return AssetFactory::get($url); }

The function has to be registered in getFunctions...

This allows the AssetFactory:get function to be invoked in templates, for example:

{% for childPath in page.root %} {% set child = getAsset(childPath) %} do something with child... {% endfor %}

It adds a line of code in the template but it is otherwise harmless. It also can be used to retrieve images or other assets:

{% for imagePath in page.images %} {% set image = getAsset(imagePath) %} do something with image... {% endfor %}

Ideally, there would be no need to "fetch" the actual asset and it would be automatically done in the background, like it is done with the "shortcut" code in the current Stacey's distribution of Twig (in Template.php). There is possibly other Twig extension types that would allow for more elegant code but I did not have the time to experiment more, maybe filters, not sure yet...

atonparker, I think you issue is that you used the existing "get" method in the stacey twig extension and it was not ment for this use, I think it was ment to change page context relative to the current page...

P.S. I cannot find how to attach code, sorry...

[seep]

Andre, I fixed a typo in my comment that may have been confusing. I was indeed using Stacey's get function to change the page context, like this:

{% set page = get('example/page') %}

A template that otherwise works with the new version of Twig will break with that tag. I did some digging and noticed the inheritance chain of Twig functions changed. This may be related.

PS, you can read about github formatting here.

[andremalenfant]

My error, I thought you were trying to achieve the same thing as me :) I never had to use the get function myself so cannot help here...

[itskingori]

@atonparker I also had the same issue with templates with that tag breaking ... have a look at my fix here, 3ebe715aba7d542b9027069dd7bd766160d051e5.

It gets rid of the Attribute "callable" does not exist for Node "Twig_Node_Expression_Function" error ...

You can also have look at my attempt at updating Stacey's Twig to 1.13.1 ~> #91 (cc: @andremalenfant )