The current documentation could be improved. On the API page: https://api.drupal.org/api/drupal/core%21modules%21system%21system.api.p... the documentation says only:

base hook: A string declaring the base theme hook if this theme implementation is actually implementing a suggestion for another theme hook.

I was attempting to implement exactly that - a hook suggestion for theme_image() - and the single sentence above left many questions unanswered; for instance, did I need to fully copy the base hook's 'variables' => array(...) in my hook's info? The answer in this case is "yes." But the documentation is entirely unclear.

When theme($hook, $variables) is called, base hook can greatly change the outcome of theme().
These are the determining steps (for the following, assume base hook = 'image' and hook = 'image__example'):

  1. hook_theme_suggestions_HOOK() is invoked where HOOK is the base hook ('image' in our example).
  2. The original hook ('image__example' in our example) is added to the end of $suggestions
  3. hook_theme_suggestions_HOOK_alter() is invoked where HOOK is the base hook ("image").
  4. At this point, the original hook 'image__example' may or may not remain as the best (last) item in $suggestions.
  5. If the base hook ("image") has any $info['includes'] files, they are included.
  6. If the base hook has any $info['preprocess functions'], they clobber the original hook ("image__example")'s preprocess functions - the original hook's processors will not be called.
  7. The base hook's $info['preprocess functions'] are called against $variables, $hook, $info, where the original hook may still be overridden and prevented from rendering output.

As a result, the original hook's preprocess functions are possibly not called (if the base hook has preprocess functions), and the original hook's theme function/template may not be used (clobbered by something implementing the base hook). Developers should be aware of these possibilities, so that they may use base hook effectively to invoke the base hook's preprocessing and include files, and so that they are not surprised if/when their theme hook suggestion isn't called due to vagaries in the base hook's preprocessors or hook_theme_suggestions_HOOK_alter().

The procedure described above is the same in D7, with a few minor changes (D7 checks the base hook's process functions as well as preprocess functions, and D7 uses "theme_hook_suggestion" to store the original hook instead of appending it to the "theme_hook_suggestions" array n.k.a. $suggestions.

This is a quick issue built from a comment originally in #909954: Document 'base hook' in hook_theme().

Files: 
CommentFileSizeAuthor
#16 hook_theme_base_hook_documentation_improvement-2106635-16.patch6.1 KBjay.dansand
PASSED: [[SimpleTest]]: [MySQL] 59,476 pass(es).
[ View ]
#16 interdiff-2106635-13-16.txt1.77 KBjay.dansand
#14 hook_theme_base_hook_documentation_improvement-2106635-14.patch5.99 KBjay.dansand
PASSED: [[SimpleTest]]: [MySQL] 59,420 pass(es).
[ View ]
#13 base-hook-improvement-hopefully.patch5.83 KBjhodgdon
PASSED: [[SimpleTest]]: [MySQL] 59,434 pass(es).
[ View ]
#1 hook_theme_base_hook_documentation_improvement-2106635-1.patch1.32 KBjay.dansand
FAILED: [[SimpleTest]]: [MySQL] 59,366 pass(es), 0 fail(s), and 2 exception(s).
[ View ]

Comments

jay.dansand’s picture

StatusFileSize
new1.32 KB
FAILED: [[SimpleTest]]: [MySQL] 59,366 pass(es), 0 fail(s), and 2 exception(s).
[ View ]

Attached is a patch against 8.x to amend the documentation (new text in bold):

base hook: A string declaring the base theme hook if this theme implementation is actually implementing a suggestion for another theme hook. When theme() is called on this hook, it will become the first theme hook suggestion for the base hook. Any of the base hook's include files will be loaded. If the base hook has any preprocess functions, they will take the place of this hook's preprocess functions. If any hook_theme_suggestions_HOOK() (where HOOK is the base hook) implementations change the order of hook suggestions, this hook may not be invoked. If this hook remains as the best suggestion after hook_theme_suggestions_HOOK(), then its function or template will be used to generate the output of theme().

jhodgdon’s picture

Good start... I found it very difficult to read, however, because of the continued use of "this" and "its", which are kind of ambiguous in the context. Maybe another iteration would be useful?

jay.dansand’s picture

Totally agreed: on principle I hate pronouns. Unfortunately though, without diving into using example code, it is not obvious to me how to refer to "the hook that is the key in the $themes array being returned by the respective hook_theme() that specifies 'base hook'" without referring to itself as "this hook." I piggy-backed on the original documentation which self-references as "this theme implementation," although honestly that's also a bit confusing.

So, "this" is hard to avoid, but maybe it can be better qualified.

base hook: A string declaring the base theme hook if this theme implementation is actually implementing a suggestion for another theme hook. If base hook is specified, when theme("example_theme_suggestion", $variables) is called, instead of being invoked directly this theme hook ("example_theme_suggestion") will become the first theme hook suggestion for the base hook. Any of the base hook's include files will be loaded. If the base hook has any preprocess functions, they will take the place of this theme suggestion's preprocess functions. If any hook_theme_suggestions_HOOK() (where HOOK is the base hook) implementations change the order of hook suggestions, this theme suggestion ("example_theme_suggestion") may not be invoked. If however, this suggestion remains the top suggestion after hook_theme_suggestions_HOOK(), then this suggestion's function or template will be used to generate the output of theme().

I tried to qualify "this" with "this theme suggestion" language, since the first sentence of the existing documentation already establishes that by using "base hook" you are specifying that the theme implementation is a suggestion. I couldn't resist sprinkling a little "example_theme_suggestion" around to try to clarify the meaning; I hope that's okay (this is my first stab at core documentation).

If we can find consensus on verbiage, I'll gladly roll another patch - I just don't want to attach several files if the perfect wording takes a while to discover.

jhodgdon’s picture

Yeah, maybe the only way to clarify this is with an example. You could start with "For example, if the outermost key in your hook_theme() implementation is '(something appropriate)' and the 'base hook' value is '(something appropriate)', and theme(...) is called..." I think that would be good and would clarify things quite a bit?

jay.dansand’s picture

How about this?

base hook: A string declaring the base theme hook if this theme implementation is actually implementing a suggestion for another theme hook. If base hook is specified, when theme() is called on this implementation, instead of being invoked directly the base hook will be called with this implementation as the first theme hook suggestion.

For example, if the outermost key returned by hook_theme() is 'image__example' and its 'base hook' value is 'image', then:

  1. If the base hook ('image') has preprocess functions, they will take the place of 'image__example' preprocess functions, and any 'image__example' preprocess functions will not be executed.
  2. If any hook_theme_suggestions_image() implementations change the order or set of hook suggestions, 'image__example' may be overridden. If however, the 'image__example' remains as the top suggestion after hook_theme_suggestions_image(), then the 'image__example' theming function or template will be used to generate the output of theme().
jhodgdon’s picture

Let's step back a bit and consider this in context.

The docs for hook_theme() say that it is used to register theme implementations either for render elements or calls to theme(). Then the @return says the keys are the internal names of theme hooks and the values are the information about the hook.

Then when I get down to 'base hook' it tells me suddenly that I might be "implementing" a suggestion for another theme hook. First of all, hook_theme() doesn't implement anything itself -- it just gives information about theme hooks your module/theme is providing. Second, I think we need to mention this possibility up in the introduction to @return, and probably also in the first full paragraph of docs in hook_theme() -- this will provide context that will help the explanation. In the first full paragraph, just mention it as a possibility, and in the @return, say what the outermost array key would be in this case and say you need to use 'base hook'.

Then in the base hook section, you can refer to the outermost array key by the name you gave it above (probably "theme suggestion hook name" or something like that?). And then instead of the confusing "If theme() is called on this implementation", which makes zero sense to me (theme() is not called on an "implementation"), you can instead say what the theme call would actually be.

jay.dansand’s picture

I'm not entirely sure that it's accurate to say "theme() is not called on an 'implementation'", at least not based on what I'm finding in the docs (should we create issues to clean those up, as well?) Here are some counter-examples I found.

theme() docs (both D7, D8) refer to theme() invoking some implementation:

Modules ... provide a default implementation via a function named theme_HOOK() (e.g., theme_taxonomy_term())

They also make explicit mention of the returns from hook_theme() being implementations:

Themes can override a default implementation by implementing a function named THEME_HOOK() (for example, the 'bartik' theme overrides the default implementation of the 'menu_tree' theme hook by implementing a bartik_menu_tree() function)

The hook_theme() docs themselves also say that the return from hook_theme are discrete implementations (invoked by theme()):

The implementations declared by this hook have two purposes: either they specify how a particular render array is to be rendered as HTML (this is usually the case if the theme function is assigned to the render array's #theme property), or they return the HTML that should be returned by an invocation of theme().

The remainder of the hook_theme() docs continue to use "implementation" throughout, and many parts of the @return are documented as "this implementation" or "this theme implementation" specifically. Some examples:

template: If specified, this theme implementation is a template...

base hook: A string declaring the base theme hook if this theme implementation...

pattern: A regular expression pattern to be used to allow this theme implementation...

function: If specified, this will be the function name to invoke for this implementation...

file: The file the implementation resides in...

From all of that, "implementation" seems to be the standardized terminology, and "theme hook" the outlier. Regardless of which is which, the rest of the docs need to be tidied up to reflect a consistent interpretation.

If we settle on "implementation," perhaps instead of rewriting the docs to change "this implementation" language (which is already abundantly used), we could get away with better defining the terminology in @return so it's less confusing. Looking at the top of @return, it refers to the returned outer keys as "hook names" (even though the @return is the first time in the docs that "hook" is used in this way), and then suddenly it changes terminology and says "Use 'variables' if your theme implementation..."; that's really a confusing change and should be introduced better and earlier.

Here's an attempt at ditching "hook" in favor of "implementation" (which is what the rest of the text refers to; a "theme hook" is an anomaly in @return: even the summary of hook_theme() is "Register a module (or theme's) theme implementations."):

array An associative array of theme hook information implementations. The keys on the outer array are the internal names of the hook implementations, and the values are information arrays containing information about the hook each implementation. Each information array must contain either a 'variables' element or a 'render element' element, but not both. Use 'render element' if you are the implementation is theming a single element or element tree composed of elements, such as a form array, a page array, or a single checkbox element. Use 'variables' if your the theme implementation is intended to be called directly through theme() and has multiple arguments for the data and style; in this case, the variables not supplied by the calling function will be given default values and passed to the template or theme function. The returned theme information array can contain the following key/value pairs (note: for the rest of this section, "the/this implementation" refers to the outer key of the return array and its associated information array):

jhodgdon’s picture

Um. Modules "provide default implementations of theme hooks" that they define, and themes "override the default implementations of theme hooks" -- in both cases, the "implementations" are either functions or template files. But you don't "call theme() on an implementation".

If hook_theme() says that the return values are implementations, then it needs to be fixed. The array elements in the return value of hook_theme() are information about theme hooks (including telling the theme system where to find the implementation). The return values are not implementations of the theme hooks, and the text saying "this implementation" should probably be changed to say "the implementation of this theme hook".

jhodgdon’s picture

I don't know if you edited your previous comment -- please do not do that: it makes the discussion difficult to follow.

But yes, "theme hook" is the right term to use.

jay.dansand’s picture

The array elements in the return value of hook_theme() are information about theme hooks (including telling the theme system where to find the implementation).

It looks like you've fallen into the same trap - "where to find the implementation" is certainly not the same as "where to find the hook" if I'm returning a new implementation of "image," the original implementation for which exists in theme.inc.

I think it's so confusing because hook_theme() provides dual functionality. Either you're returning new theme hooks and their specifications (which according to the docs requires a default implementation, so implicitly you are also specifying your implementation with hook_theme()), or you are registering your own implementation of an existing hook.

The extant docs for theme() and hook_theme() lean toward the more generic case (returning implementations), but Default theme implementations says that hook_theme() defines hooks. Clearly the docs themselves can't make up their minds.

Anyway, arguing semantics is moot and beyond the intended scope of this issue. If we need to overhaul several doc pages to standardize nomenclature, that's fine, but it needs its own set of issues and definitely more people looking at it than just the two of us. I am merely trying to provide an incremental improvement to the existing docs, written in a consistent manner with the surrounding text. A larger issue requires... well, a larger issue ticket.

jhodgdon’s picture

Sorry, but I disagree about the scope. I do not see a way to make sense of the base hook section of this documentation without fixing up the rest.

jay.dansand’s picture

Well, we either proceed with edits to hook_theme() to bring the whole page inline with itself (simpler), or tear up everything across theme(), hook_theme(), Default theme implementations, etc. (harder). To accomplish the latter, a new issue should be created and this one postponed until that other issue is resolved - it doesn't make sense to co-opt an issue which is only tangentially related to the nomenclature problem.

For what it's worth, I agree more with the current hook_theme() and theme() documentation (and comments in code) interpretation: what you are defining in hook_theme() are implementations of a hook. That's how the rest of Drupal's hook system works: you define implementations of hook_init() (and you put "Implements hook_init()." as a comment right above the relevant function). Likewise, you implement theme hooks. That makes sense to me (and apparently to the original authors), so I'd argue for standardizing in that direction, not the reverse.

Expanding on that thought, it makes very little sense to say "file - here's where the hook's file lives." Hooks are specifications, not implementations - if anything, they "exist" in something.api.php. The actual implementation has a file where it lives. But, to argue against myself, hook_theme()'s returns also delineate "variables" which arguably should be part of the specification, not the implementation. So we're back to the dual-use of hook_theme() and its corresponding confusion; in the end, neither interpretation is decidedly "wrong." That's a unilateral decision with little to recommend it.

jhodgdon’s picture

Status:Active» Needs review
StatusFileSize
new5.83 KB
PASSED: [[SimpleTest]]: [MySQL] 59,434 pass(es).
[ View ]

OK ok ok... You are all making good sense.

How about this patch?

jay.dansand’s picture

StatusFileSize
new5.99 KB
PASSED: [[SimpleTest]]: [MySQL] 59,420 pass(es).
[ View ]

I like it! But the "base hook" piece needs tweaking for accuracy. How about this?

Here's what I changed:

@@ -86,8 +86,10 @@
+ *   - base hook: Used for theme() suggestions only: the base theme hook name.
-+ *     This suggestion will become the first suggestion to be searched for in
-+ *     a call to theme(), unless an implementation of
-+ *     hook_theme_suggestions_HOOK() changes the order. If this suggestion is
-+ *     chosen, its include files will be loaded and its preprocess functions
-+ *     will be called in place of the base hook's preprocess functions, and then
-+ *     its function or template will be used to generate the output for theme().
++ *     Any of the base hook's files will be included and the base hook's
++ *     preprocess functions will be called in place of any preprocess functions
++ *     for the suggestion. If an implementation of hook_theme_suggestions_HOOK()
++ *     (where HOOK is the base hook) changes the suggestion order, the
++ *     suggestion being defined here may not be used to generate the output for
++ *     theme(). If the order of suggestions is not changed, then this
++ *     suggestion's function or template will be used to generate the output for
++ *     theme(), but any of its preprocessors still will not be called.
jhodgdon’s picture

Status:Needs review» Needs work

Please make an interdiff file when you upload a new patch. Thanks!

Anyway, I think this new wording is really confusing. Can you try again? read it and see if it makes sense if you don't know anything about base hooks and how they are supposed to work. I cannot figure out which functions will be called and which files will be included, and you took out the part about this becoming the first suggestion, so I don't know what order the suggestions would be in?

jay.dansand’s picture

StatusFileSize
new1.77 KB
new6.1 KB
PASSED: [[SimpleTest]]: [MySQL] 59,476 pass(es).
[ View ]

The thing is (and this was the not-so-obvious thing that lead me to want to improve the docs in the first place), the base hook does not become a suggestion - it basically becomes the hook that is invoked up until the very last stage (where the template file or function is used). The base hook's files are always included, its preprocessors are always run (instead of any other suggestion's preprocessors), and that happens before any suggestions are searched. The only place the suggestions come into play is in the very last step, when the suggestion's function or template file is used to generate the output.

I've taken another stab at it; how's this? Sorry for the lack of interdiff previously - I had never stumbled on https://drupal.org/documentation/git/interdiff before!

jhodgdon’s picture

Status:Needs work» Reviewed & tested by the community

Ah, OK. Now I get it! I think that the changes you made here are excellent! Thanks for taking the time to work on this and for caring that we get our documentation correct.

I will tentatively mark this RTBC, since presumably you have reviewed the changes I made, and I've reviewed the changes you made. But I will suggest we don't commit it for a few days, to give other participants time to respond. Also, marking it RTBC will make the test bot run, just in case.

And no problem on the interdiff. Learning++ :)

jhodgdon’s picture

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Thanks again! Committed to 8.x. This should probably be ported to 7.x (and may need some changes there? or maybe not?).