hook_search_page not appearing on api.drupal.org

Good grief... The docs for this have been broken since *4.7*. Some of the docs are missing as well, gimme a few to write them..

Unassigning this again the moment. I got caught up on other things.

Changed the component to reflect the new component categorization. See http://drupal.org/node/301443
-nielsbom

#185469: hook_search_page() marked dupe.

How about this patch ?

Note sure whence doc patches should be rolled from, though.

This is an API doc issue. As such, it belongs in the Drupal issue queue, not in the Documentation issue queue.

I would like to see this patch make it clear that this hook is for use along with hook_search(). You have to use hook_search() to define a search type for your module, and then you can override what the Search module would normally do to display search results, for your module's search type, by implementing hook_search_page(). I don't think this proposed patch makes that clear.

And I don't like that it seems to imply that rendering is "expected" -- overriding rendering is what this hook does. You might change the first line to say something like "Overrides all theme rendering of search results", to make that even more clear, though the following line does describe it pretty well.

Also, the patch seems to have the @return cut off (it says "An HTML string repres" and stops there). It should probably say "An HTML string containing the formatted search results".

Do you want to redo the patch, fgm? If not, I can.

We probably need patches for D5, D6, and D7.

This is still an issue in 7.x. Probably should be fixed there first and then back-ported.

This hook is called from the search_data() function, in 5.x through 7.x (just to save others the trouble of searching through the code for it).

One other comment on the patch. At least in D6 and D7, it looks to me as though the reference implementation should call theme( 'search_result', $entry, $type ) rather than theme( 'search_item' ). See template_preprocess_search_results() for example, and search_theme() for a list of themeables in the search module.

I haven't checked D5 to see what the theme call should be there.

Attached is a patch for Drupal 7 which documents this hook. It fixes issues mentioned in #7 and #8.

Forgot to set status to needs review.

This needs some more work. It says:

*
* Modules may implement this hook in order to override the default function
* theme_search_page() to display search results for its own custom search.   
* 
* As a result of this, the module must also implement hook_search().

A few issues:
- I don't really think it's a "result of this" that they must implement hook_search(). What's really the case is that it's a module whose main purpose is to implement a custom search via hook_search(), and implementing hook_search_page() is one additional thing that module can do.
- They are not overriding theme_search_page() but theme( 'search_results' ). See http://api.drupal.org/api/function/search_data/7

We should also probably add a note into the hook_search() documentation about this hook.

How about this patch instead?

Wouldn't it be useful to mention that theme('search_result') by default creates DT/DD pairs ? Without this information, it makes the DL look rather bizarre.

I'm requesting a re-test, because this doc-only patch should not have caused the "Admin Overview" tests to fail.

Here's a new patch that mentions the DL/DT business. Also marking "novice" since this is a doc-only patch.

Shouldn't this get refactored to take advantage of drupal_render()?

I'm not sure if it can easily be done. Currently, theme('search_results') and theme('search_result') work together to create a definition list, and this hook takes the place of theme('search_results'). Theme functions are always supposed to return fully-rendered HTML, but it is only the combination of these two functions that would specify a full DL object that could then be rendered.

I agree it's awkward...

Does drupal_render handle DL lists anyway? Not sure...

It doesn't : the <li> is hardcoded within it, so no dd/dt.

Actually, maybe it would be an interesting addition to theme_item_list() to support definition lists, but that seems to be another topic

Anyway, even if theme_item_list() would handle DL lists, we'd have to totally restructure how search results are rendered in order to use it, because the input would be an un-rendered DL structure, and right now theme('search_result') would be rendering one DL item, and theme('search_results') would be putting in the wrapper.

User profiles use DL pattern as well and they manage to honor the render() array paradigm. I would be happy if we came up with a theme function that render a DL and then use it here and in user profiles. user profiles are a bit awkward about this.

Looks like the test bot was having issues yesterday, so submitting for retest (this was a doc-only patch).

So can we get this patch in as-is? I think it's too late now ofr a theme/render API change.

answering moshe in #22, I rerolled the patch in #54898: Add a description-list.html.twig template (ex. definition list), so maybe now could be related

RE #26: I don't think that is going to happen in Drupal 7.

Committed to CVS HEAD.

What about backport?

Good idea. In 6.x the hook docs are in the Documentation project git repository.

Committed the above patch to Drupal 6 docs.