add domain_theme_theme_get_setting()

Looks good, makes sense.

Revised patch. Needs review. My changes are:

* Adds a proper docblock
* Changes the function to domain_theme_get_setting, as theme_theme looks odd.
* Properly checks for existing settings per domain.
* Removes the safe but redundant setting of the $cache['domain_id'] array.
* Changes empty() checks to isset() checks, which are more proper.

Was not working for what I need, but fixed a parse error in the patch.

Settings in multidimensional arrays were not available.
Changed the function so all settings from the array are available.

The use of this class needs some documentation. new RecursiveIteratorIterator

I don't see how $cache is used here.

This modified patch does not use the class. The ability to get a single value from within a single setting, while useful in some circumstances, does not mirror the behavior of Core's original theme_get_setting() function.

More importantly, that methodology can pollute the returned settings quite easily.

Consider the following settings:

$settings = array(
  'some_key' => TRUE,
  'other_key' => array(
    'some_key' => FALSE,
    'other_key' => FALSE,
  ),
  'third_key' => array(
    'some_key' => FALSE,
    'other_key' => TRUE,
  ),
);

Iteratively flattening this array absolutely destroys the data through otherwise-safe naming conflicts:

$settings = array(
  'some_key' => FALSE, // from $settings['third_key']
  'other_key' => TRUE,  // from $settings['third_key']
  'third_key' => array(
    'some_key' => FALSE,
    'other_key' => TRUE,
  ),
);

Pardon any mistakes, but you get the idea. The original, accurate values can still be read from within their arrays (if they are not overwritten entirely!) but then there is no point in flattening it.

The patch also moves the "grunt work" into a domain_theme_get_settings() function, which is then called from domain_theme_get_setting() in order to retrieve single values. The _get_settings() result is stored with drupal_static() and cleared when appropriate.

I'll be honest, it's been years since I looked at this code.

This looks to be an API-level change insofar as it simply adds convenience functions for theme developers, but doesn't change the underlying behavior.

Is that accurate?

I understand -- thank you for ducking back into this for me.

Yes, that is accurate, it does not modify any functionality. It's not strictly for convenience... that said, most cases could probably be worked around, but I did not want this issue to end on a deeply flawed patch!

Worth noting, it seems to only be necessary when the values need to be accessed (very!) early in the theming process, or (potentially?) from within a module doing alterations involving domain_theme (haven't tested on that one, don't have a case involving it, but could probably dream one up...)

But my last patch is flawed! I noticed domain_theme_get_settings doesn't work on the primary domain. That helper wasn't really meant to be called directly, and the main function (domain_theme_get_setting) seems to work (so far) with its theme_get_setting fallback... but I would still like to handle that with more consistency and elegance "just in case."

I'll have an updated patch (and interdiff) within the next few days, hopefully...

Great. Thank you!

Drupal 7 in End of Life and no longer supported. We encourage you to upgrade to a supported version of Drupal. For more information, see https://www.drupal.org/upgrade.

	Thank you for your contribution! Your continued support of this project makes other volunteer contributions more sustainable.
There are multiple ways to show appreciation for the work contributed to this project, including: Triaging issues and adding more context to existing issues. Writing documentation or patches for this project.