Available theme hook suggestions are only exposed when there are preprocess functions defined

Thanks for reminding of this problem. I hope that effulgentsia can shed some light on the details.

Assigning to myself for now, just to ensure that this is tackled. Feel free to re-assign, if you want to work on it.

subscribe... just am running across this

I hope that effulgentsia can shed some light on the details.

I will. Soon.

I ran into this today (or at least some version of it). Is this two issues in one, or are they the same issue?

The followups at #241570: Theme preprocess functions do not get retained when using patterns seem to be discussing the issue that when theme('foo__bar') is called, MYMODULE_preprocess_foo__bar() isn't.

But I think Jacine's second point above (and the issue I just encountered) is that even if you implement MODULE_preprocess_foo() itself, you cannot know if it was theme('foo'), theme('foo__bar'), or theme('foo__something_else') that was called; you don't have that context in $variables['theme_hook_suggestions'] or anywhere else.

I talked this over with @effulgentsia a bit and he pointed out that in many cases you can get the context from some other variable that was passed in. However, we then realized that's not always the case. An example would be this function:
http://api.drupal.org/api/drupal/modules--node--node.module/function/nod...

This results in a call to theme('item_list__node') but there is nothing it provides that would (cleanly) allow preprocess functions to distinguish it from all other calls to theme('item_list').

The root of the problem seems to occur early in theme(), with this code:

    while ($pos = strrpos($hook, '__')) {
      $hook = substr($hook, 0, $pos);
      if (isset($hooks[$hook])) {
        break;
      }
    }

Which basically destroys all memory of what the passed-in $hook originally was, as it strips parts of it off. If we retained some record of the original suggestions in some variable that would eventually get passed to the preprocess and process functions as part of the $variables array, that might be a quick and dirty solution for at least part of this issue.

Yes, two in one. That other issue was unknown at the time of creation.

We can provide that original hook name info as theme variable, using the 'theme_' namespace, since that namespace is owned by the theme system. $variables['theme_hook'] would make sense, I guess.

~

Marking this as critical for before-release-candidate attention. There are situations where this cannot be worked around. Maybe fixing it is not considered an API change? But still better pre-RC.

Let there be patch!

debug() is probably just too late from templates. It sets a drupal message.

Not like any of us want to add more complexity to theme(), but this *is* a problem as others on this issue have eloquently explained. We'll want to revisit the render and theme layers in D8 to see if we can simplify things, because the implementation of building and using the registry really is getting out of hand.

As David explains in #5, my goal in #678714: Unify use of theme hook / template suggestions, fix clobbering problems, and improve suggestion discovery performance and other issues has been to make the following as similar as possible with respect to everything that happens later in the pipeline:

theme(array('links__node__comment', 'links__node', 'links'), ...);

theme('links__node__comment', ...);

theme('links', ...);
...
function template_preprocess_links() {
  if (...) {
    $variables['theme_hook_suggestions'][] = 'links__node';
  }
  if (...) {
    $variables['theme_hook_suggestions'][] = 'links__node__comment';
  }
}

Basically, for various legacy reasons, we have these 3 techniques for doing essentially the same thing. And each one is employed somewhere (see template_preprocess_field() for an example of the last one, and the way drupal_prepare_form() sets $form['#theme'] for an example of the first one). While there are subtle reasons for why one technique is employed in one place, and another one is employed in another place, I think it would be best for them to work identically with respect to themeing.

This issue discovered one area in which they needlessly differ, and that is that only in the last case do hook_preprocess_*() implementations have access to a $variables['theme_hook_suggestions'] variable containing all of the suggestions requested up until that point. This patch fixes that. It needs a test added though. Any volunteers?

At first I was surprised by this. I thought of $variables['theme_hook_suggestions'] as a 1-way variable for preprocess functions to add to, and theme() to use, not for later preprocess functions to branch off of. After all, in the case of something like 'node__article', a later preprocess function doesn't need to check $variables['theme_hook_suggestions']. It can check $variables['node']->type. But as David points out in #5, via some short sightedness on our part, we don't always invoke theme functions with all necessary context information that might be useful. In the case of 'item_list__node', $variables['theme_hook_suggestions'] is the only variable available containing this useful context information, so I think this patch is the correct approach, despite the added implementation ugliness.

The latest work on #241570: Theme preprocess functions do not get retained when using patterns is a separate matter, and something I'm resistant to, but haven't taken the time to articulate why. I'll do so in that issue though. It shouldn't hold up this one.

This doesn't look critical to me. I don't understand why this couldn't be fixed after RC1, or even in 7.2 for that matter.

Also, if we're screwing with theme() we need benchmarks.

I didn't say it won't get fixed in D7? It's not like we stop fixing bugs once Drupal 7.0 ships, and there's nothing in here at all that breaks APIs.

But this isn't done until benchmarks and tests are done.

prety pretty pretty please we really need this one in core.

It an old problem for themers that we dont have a clue of whats actually given to us. So kinda walk around in the darkness, until by accident someone finds the right template or theme function (which can take days in a larger drupal site)

subscribe

subscribe

Love to see this go in, its a pretty big wtf and pita and a bunch of other unmentionable acronyms... sorry I cannot help with tests, wouldn't know where to start I'm afraid.

Subscribing.

I'm quite confused on why the passed $hook is needed inside 'theme_hook_suggestions'. theme() finds the closest registered hook available. I don't see the point in carrying all the misses into the suggestions since they don't exist to begin with.

"theme_hook_suggestions" should be one way. Variable processors should only add to them and let theme() handle the rest.

And some of the code within theme() is just too much. A lot can be handled while building the theme registry. This could include the HOOK_foo__bar__baz() from a module (as Jacine originally mentioned) even if nothing explicitly defines the patterned hook.

Here's how it could possible happen.

Collect all functions with get_defined_functions() and filter it down checking based on existing hooks that hold a 'base hook'. drupal_find_theme_functions() does this but it's specific to theme functions.

If the patterned hook is not explicitly defined, take all the info from the base hook and carry it onto this new patterned hook and register it.

There would still be something very iffy about 'theme_hook_suggestions'. Defining it from a variable processor is a little late. If one of the suggestions catch on, it will not have its own specific var process functions invoked because it's in the middle of var process to begin with.

On a side note, In drupal 8 we should to get rid of the idea of setting 'theme_hook_suggestions' from a variable process function. Variable process functions should only be for processing variables. Suggesting alternate hooks should be its own thing due to what I mentioned above and it simply doesn't make sense when we talk about a function for processing variables.

Going to try tacking this.

I'm relearning how to apply a core patch so I hope this is correct.

The patch does the following:

• Hook suggestions have a full set of variable processors. It no longer does the weird switching of hooks in the middle of a theme() call just to have the base hook processors from running. Since each hook suggestion has a full set, that means 'name_preprocess_hook' *and* 'name_preprocess_hook__suggestion' will be invoked.
• If a module or a theme supplies a variable processor named as a pattern and that pattern was never explicitly registered, it will automatically register a hook based on the pattern and inherit all the attributes of the base hook. For example, a module or theme supplying 'foo_preprocess_bar__suggest' for the variable process function will register 'bar__suggest' as a new hook inheriting everything from the 'bar' hook. Of course, if 'bar' doesn't exist then it will do nothing.
• A few performance optimizations for theme(). It's more straight forward removing some blocks of code and handling it in the registry build.

Overall, the performance is about the same. Using Apache bench, it showed no improvement which I find confusing because when profiling with xdebug and webgrind, theme() is consistently faster by a small margin.

Here's a diff, --before and ++after. Done with xdebug disabled. I'm no pro at performing benchmarks but here are the results.

1000 calls, 5 concurrent.
Page caching off
Block caching on
css/js aggregation on
Opcode cache cleared before each test then primed with 50 calls before the full 1000 calls on the front page.

--- /Users/joon/Desktop/before_patch.txt	2011-05-17 01:37:15.000000000 -0400
+++ /Users/joon/Desktop/after_patch.txt	2011-05-17 01:37:20.000000000 -0400
@@ -21,35 +21,35 @@
 Server Port:            80
 
 Document Path:          /drupal_7/
-Document Length:        36327 bytes
+Document Length:        36331 bytes
 
 Concurrency Level:      5
-Time taken for tests:   41.704958 seconds
+Time taken for tests:   42.148533 seconds
 Complete requests:      1000
 Failed requests:        0
 Write errors:           0
-Total transferred:      36739703 bytes
-HTML transferred:       36363327 bytes
-Requests per second:    23.98 [#/sec] (mean)
-Time per request:       208.525 [ms] (mean)
-Time per request:       41.705 [ms] (mean, across all concurrent requests)
-Transfer rate:          860.28 [Kbytes/sec] received
+Total transferred:      36707000 bytes
+HTML transferred:       36331000 bytes
+Requests per second:    23.73 [#/sec] (mean)
+Time per request:       210.743 [ms] (mean)
+Time per request:       42.149 [ms] (mean, across all concurrent requests)
+Transfer rate:          850.47 [Kbytes/sec] received
 
 Connection Times (ms)
               min  mean[+/-sd] median   max
-Connect:        0    0   0.3      0       6
-Processing:    82  207  69.1    195     451
-Waiting:       74  188  66.8    174     444
-Total:         82  207  69.1    195     451
+Connect:        0    0   0.0      0       1
+Processing:    84  209  59.4    199     419
+Waiting:       74  192  57.9    183     413
+Total:         84  209  59.4    199     419
 
 Percentage of the requests served within a certain time (ms)
-  50%    195
-  66%    218
-  75%    234
-  80%    249
-  90%    314
-  95%    354
-  98%    392
-  99%    422
- 100%    451 (longest request)
+  50%    199
+  66%    220
+  75%    237
+  80%    248
+  90%    297
+  95%    344
+  98%    370
+  99%    381
+ 100%    419 (longest request)

I also attached screen shots of webgrind.

I haven't been explaining myself clearly and how it relates to the issue at hand.

From the original post by Jacine:

Even though some module uses theme suggestions in the form of foo__bar__baz, a module cannot simply implement HOOK_foo__bar__baz().

My patch doesn't address that directly but it does allow a module to use HOOK_preprocess_foo__bar__baz() since it is focused on variable process functions.

Allowing the theme *function* of HOOK_foo__bar__baz() to work from a module doesn't fit conceptually for me since all modules are equal and they don't stack on top of each other like themes do and theme functions are the end of the line. There can only be one active theme function for a given hook.

Variable process functions on the other hand do have a pattern of stacking or layering on top of each other.

Example:

$output = theme('foo__bar__baz');

/**
 * foo theme function. This can also be implemented as a template. Doesn't
 * make a difference.
 */
function theme_foo() {

}

/**
 * foo pattern preprocessor based on foo.
 *
 * Since the module supplies this function, 'foo__bar__baz' must
 * be registered for it to be available and the patch does it
 * for you automatically if it doesn't exist already.
 *
 * This can also be done from a theme. Makes no difference.
 */
function module_name_preprocess_foo__bar__baz(&$vars, $hook) {

}

/**
 * foo hook preprocess function.
 */
function theme_name_preprocess_foo(&$vars, $hook) {
  // theme('foo__bar__baz') will result in $hook being 'foo__bar__baz'.
  // This is only because 'module_name_preprocess_foo__bar__baz()' was
  // provided by the module forcing the hook to be registered and
  // 'foo__bar__baz' inherits all the base hook variable processors.
  // This can also be your context.
  print var_dump($hook);
  // theme('foo') will result in $hook being 'foo' as expected.
}

Under the hood, when 'foo__bar__baz' is never registered but a variable process function based on that pattern is discovered, it finds the base hook and copies all the meta into the new hook.

If the 'foo' base hook is a template, we'll have a foo.tpl.php file but the same idea will apply. That preprocess function based on the pattern will create the new hook and use that same foo.tpl.php file. When foo--bar--baz.tpl.php is created, it's seen as an explicit override and use that instead.

Suggestions that are not applied via preprocess functions do not appear in $variables['theme_hook_suggestions'] so themes have no way of telling what suggestions are available.

I might have missed something but I'll repeat my understanding on this.

Let's say a theme call of theme('root__derivative') is invoked. The 'root' is a known hook but 'root_derivative' was never registered. If that's passed into $variables['theme_hook_suggestions'], what can be done with it? If it was never registered, it's dead from the start.

The only thing you would miss are the less specific hooks that might actually exist but I don't see the why anyone would want that.

Ah, yea. I must be loosing touch. You're looking at it in terms of understanding what your options are as a theme developer then acting on that information. I was more concerned about having a complete theme registry so it acts on the passed hooks more consistently. I kinda jacked your thread, didn't I. I'll open this in a new issue.

Concerning the approach taken before I got here.. It should not be passed to 'theme_hook_suggestions' since it wouldn't do anything useful. Since it's more about context for the themer, just pass it into $variables. Using theme_hook_suggestions would incur a tiny performance hit since theme() does all that checking with suggestions.

I moved the patch here: http://drupal.org/node/939462#comment-4491226

http://drupal.org/node/956520#comment-3860490 seems to be the current state of affairs.

tagging for learnability

Marked as duplicate of #939462-109: Specific preprocess functions for theme hook suggestions are not invoked.

tagging

Opened follow-up for the non-duplicated part of this issue:

#1836142: Provide suggestions to (pre)process functions

This issue has been side-tracked too much for it to be still useful.

This is actually a discrete bug from that. It is only about exposure, not invocation. Slightly adjusting title to clarify.

#16 holds a simple solution attempt. I didn't quite understand why #30 started to revamp the entire theme registry processing, and it's perfectly possible that these patches and discussions are indeed duplicating #939462: Specific preprocess functions for theme hook suggestions are not invoked.

Okay, I reduced the scope in the issue summary and marked freshly created #1836142: Provide suggestions to (pre)process functions as duplicate.

Working here then and start with #16.

Any progress, Fabian? Just trying to get theme work going again.

#16 worked fine locally for me, but probably needs some more thought of what this will output.

Postponing for now. I think this is going to end up being a duplicate of #2004872: [meta] Theme system architecture changes.

I think this is officially now a dupe of #1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter()

Updated to reduce scope to #16