search_results should use theme function instead of writing out search_results directly

There's a reason that theme('search_results') isn't used, and that's because it's not actually modifiable from the outside. Namely, it contains a bunch of elements that are hardcoded and unsuitable as a pane.

Surely we can find a better way to solve this issue than to hard-code the HTML for it. In the case of the site I'm currently working on, I need to change the results so that it is a div surrounding the results instead of an ordered list.

I have attached a patch that updates the search plugin to use render arrays when rendering the search results. This will allow theme developers to alter the render array later down the line, either in templates or preprocessors. If we use render arrays here, we won't have to use string manipulation functions just to change something as trivial as the wrapper around the search results.

In an idea situation, we would be using the same code as the search results page itself does, but this is at least a little better than hard coding some of the HTML around the search results.

Yes, you are right, we can!

But I think we can do it a bit more extendable by providing a theme for it. Please find attached a patch which provides a theme with a template file and preprocessor.

D

After speaking to japerry on IRC, I made some adjustments to the patch to keep logic fully in module code rather than calling the theme function from inside the template.

Here is a patch (+ a interdiff from #4).

I've reviewed the patch and it works great, at first, I think it would be better and simplier a call to drupal_render over $results array on line 88 of search_result.inc, but #5 solution allows more control and let users who only want to change the HTML element which warps the results doing it with the only need of a a preprocess function with one line of code.

And this allows page_manager_search_results to fit into Drupal best practices allowing themable output.

I have reviewd #5. This patch fixes issue, but on my opinion it is better to use render arrays rather than theme_ function.

I would agree with @anpolimus on using render arrays rather than theme functions. In my case, I just used something like hook_page_alter to change the render array around a bit before rendering the page.

It would be even nicer (and I'll create a patch with this implementation once I get a moment) if we created a render element, so then we get the best of both worlds. We can re-use the HTML by re-using the render element, and we can stick with render arrays.

So #2 is closer to what we should be doing -- making a render array.

It would be even nicer (and I'll create a patch with this implementation once I get a moment) if we created a render element, so then we get the best of both worlds. We can re-use the HTML by re-using the render element, and we can stick with render arrays.

Yes, this would be better. I'm marking 'needs work' I think until we get that part of the patch in.

Drupal 7 is no longer supported, closing.