Make Twig engine available during install

I don't really like that we are duplicating code all over the place, but the install container is only meant temporary, so this is fine and not worth the trouble of creating a twig_setup function in container.

=> RTBC
=> Priority: Major, this blocks conversion work
=> Updated issue summary with commit message to include larowlan.

This was also proposed by larowlan in http://drupal.org/node/1939082#comment-7205220.

Updated issue summary.

We definitely need to get this in - it's blocking more and more conversion issues. We talked about this issue briefly in IRC yesterday but I hadn't had a chance to review the patch yet. If we're not worried about code duplication, why not just a straight copy and paste of Drupal\Core\CoreBundle::registerTwig() as in #1939082-12: Convert theme_status_messages() to Twig?

My biggest concern is the hardcoding of Twig settings. At least during conversion, having debug and others available would be very useful.

This patch:

+++ b/core/includes/install.core.incundefined
@@ -374,6 +375,31 @@ function install_begin_request(&$install_state) {
+        'cache' => TRUE,
...
+        'debug' => FALSE,
+        'auto_reload' => FALSE,

Patch from #1939082-12: Convert theme_status_messages() to Twig:

+++ b/core/includes/install.core.incundefined
@@ -374,6 +375,33 @@ function install_begin_request(&$install_state) {
+        'cache' => settings()->get('twig_cache', TRUE),
...
+        'debug' => settings()->get('twig_debug', FALSE),
+        'auto_reload' => settings()->get('twig_auto_reload', NULL),

Looks good to me, thanks @steveoliver!

+1 for RTBC

Feel free to bounce back to RTBC, but a quick question.

Is there a reason we didn't just change the code in CoreBundle.php to do things like this: 'auto_reload' => settings()->get('twig_auto_reload', NULL), and then simply call registerTwig from here?

This seems like an awful lot of duplicated code, and the part that's not duplicated seems like it would make sense for both CoreBundle.php and installer.

I'm actually surprised that settings() works here, but that's cool. :)

Is there a reason we didn't just change the code in CoreBundle.php to do things like this: 'auto_reload' => settings()->get('twig_auto_reload', NULL), and then simply call registerTwig from here?

Well, the method is currently protected. We would need to make it public and static?

The code being added to install.core.inc in #6 is entirely duplicated from CoreBundle. CoreBundle::registerTwig() has the settings() code as well, and this is what I brought up in #5, that Twig in the installer should behave the same way and respect settings added to settings.php.

Here's a patch that does #10. The patch works as expected locally – I'm able to install i.e. #1939082-3: Convert theme_status_messages() to Twig and #1885564-10: theme.maintenance.inc (authorize.php) - Convert theme_ functions to Twig, but am not able to without the patch. Let's see what testbot thinks.

Grepped through for registerTwig, and CoreBundle::build() provides the only reference.

MUCH BETTER. :D

Committed and pushed to 8.x. Thanks!

Oh, that works amazingly well. Another +1 for RTBC, much nicer.

x-post ...

Filed #1957042: Document why CoreBundle::registerTwig() is public and static. I think I had mute on when I tried to suggest this to @webchick on the phone. :)

Thanks @xjm, good call.