merged branch dantleech/globals_not_array (PR #944)
authorFabien Potencier <fabien.potencier@gmail.com>
Wed, 2 Jan 2013 15:57:55 +0000 (16:57 +0100)
committerFabien Potencier <fabien.potencier@gmail.com>
Wed, 2 Jan 2013 15:57:55 +0000 (16:57 +0100)
commit8447ca17185c082831c4d46e416f39bdeba937d9
tree8b3664137dd0f4c5afa9d99a7ea4d37f737f260d
parent25f7b0c7eb456f5c39f1e8163da538de5d0d717b
parent9f5fa532ef352ccc5881a791df9abaeb0ee57a83
merged branch dantleech/globals_not_array (PR #944)

This PR was squashed before being merged into the master branch (closes #944).

Commits
-------

9f5fa53 Upgrade problem

Discussion
----------

Upgrade problem

Hi

When upgrading to HEAD I got the error

Warning: array_key_exists() expects parameter 2 to be array, null given
in /home/daniel/www/DCMS/vendor/twig/twig/lib/Twig/Environment.php line
975

Caused by adding globals in the initRuntime method in my extensions.

This patch makes addGlobal handle the case where Envieonment::$globals is null.

---------------------------------------------------------------------------

by stof at 2012-12-31T14:33:17Z

Your patch does not work properly. It will allow setting the ``bar`` global when you have not called ``getGlobals`` yet now (which is why your new test is failing on Travis)

---------------------------------------------------------------------------

by dantleech at 2012-12-31T14:45:28Z

Yes, sorry, I pushed the wrong version of Environment.php. Tests should pass now.

---------------------------------------------------------------------------

by stof at 2012-12-31T14:55:29Z

I think you are now forbidding to replace an existing global without calling ``getGlobals`` first when the runtime is initialized

---------------------------------------------------------------------------

by dantleech at 2012-12-31T15:12:20Z

I think that this is the correct logic.

Throw the Exception "Unable to add global "%s" as the runtime or  the extensions have already been initialized" IF the global has not already been registered.

So, if ``$globals === null`` then this is indeed the case. The logic is the same, it just supports the default NULL value for $globals. I think.

---------------------------------------------------------------------------

by stof at 2012-12-31T15:52:50Z

no, if ``$globals === null``, it means you have not yet loaded all the globals registered previously, so you cannot know if it has already been registered this way. What you need to do is calling ``initGlobals`` in this case to be sure they are initialized

---------------------------------------------------------------------------

by dantleech at 2013-01-01T09:18:45Z

ok. updated, I think I understand -- so it is required to use ``getGlobals`` to register globals, but we cannot add globals after/during ``initRuntime``. But we could replace them:

````php
public function getGlobals()
{
   return array(
     'existing' => $this->someGlobal
   );
}
pubic function initRuntime(\Twig_Environment $e)
{
  $e->addGlobal('existing', $this->foo);  // OK
  $e->addGlobal('non-existing', $this->bar);  // FAIL
}
````

---------------------------------------------------------------------------

by darklow at 2013-01-02T15:11:46Z

This error also gives us lot of headaches, after any twig file changed first reload, got this error on array_key_exists.
Hopefully this PR could be verified and approved. Thank you.