Convert the Maintenance Theme variable to settings

No longer blocked.

This has to work when the database is down so it should be in settings rather than CMI.

Updating as $conf as we got $conf['system.theme']['default']

catch do you have a problem with leaving this in $conf? I'd kind of prefer it myself.

+++ b/sites/default/default.settings.phpundefined
@@ -543,11 +543,11 @@
+ * $conf['system.theme']['maintenance']. The template file should also be copied into the

Nitpick: this comment is over 80 characters

Reorganized comment to fit 80 character limit.

Reorganized comment to fit 80 character limit.

#7: 1829170-maintenance-theme-cmi-6.patch queued for re-testing.

Re-rolling...

#10: 1829170-maintenance-theme-cmi-10.patch queued for re-testing.

Removing trailing whitespace.

#12: 1829170-maintenance-theme-cmi-12.patch queued for re-testing.

Updating for new Twig template location.

Updated to Drupal::config() as per #2028149: The config() function should be deprecated in favour of Drupal::config()

Also took out my change in #14 as that is probably scope creep.

#15: 1829170-maintenance-theme-cmi-15.patch queued for re-testing.

The change in #14 is fair game since we are already changing that line. If we are changing the line we should fix it!

Thanks Cottser, never sure about ones like that :)

#18: 1829170-maintenance-theme-cmi-18.patch queued for re-testing.

#18: 1829170-maintenance-theme-cmi-18.patch queued for re-testing.

+++ b/core/includes/theme.maintenance.inc
@@ -34,7 +34,7 @@ function _drupal_maintenance_theme() {
+    $custom_theme = isset($conf['system.theme']['maintenance']) ?: 'seven';

I can be wrong but this will return a boolean in stead of the maintenance theme. The ternary shorthand with isset is dangerous :)

@aspilicious you are correct. An updated patch is attached that also includes a minor change to the text.

With the patch applied the logic is really long:

    if (isset($conf['system.theme']['maintenance'])) {
      $custom_theme = $conf['system.theme']['maintenance'];
    }
    elseif (Drupal::config('system.theme')->get('default')) {
      $custom_theme = Drupal::config('system.theme')->get('default');
    }
    else {
      $custom_theme = 'bartik';
    }

    if (!$custom_theme)  {
      $config = Drupal::config('system.theme');
      // A broken install might not return an object.
      if (is_object($config)) {
        $custom_theme = $config->get('default');
      }
    }
    if (!$custom_theme)  {
      $custom_theme = 'bartik';
    }

This seems to be really duplicated information

@dawehner: thanks for the review. I agree. Updated patch is attached.

This is indeed way easier :)

So I don't think we should be using CMI here at all we should be using the Settings object since this should only be configured in settings.php as it is used when the database is unavailable.

CMI could use a kind of caching and needs directories configured so let's use $settings. (#3 catch and #28 alexpott suggest that)

There's a bit of confusion because the same settings used for install-update but defaults to 'seven', probably this should be preserved for custom profile installers

Yeah the required logic is kind of odd.

Makes sense!

Back to rtbc. looks ok to me...

+1 for RTBC.

Rather than checking for $config being true and an object, should we be doing try/catch there instead?

Sounds like a "needs review"

Here is a patch using try/catch.

I've also reworked the comments and split the assignment inside the if into two separate lines, to help readability.

Looks much better to me.

Committed/pushed to 8.x, thanks!