It would be great if the theme function got more information than an array of links. For me, I want to import some of the other node information. This patch passes $r['docs'] to the theme function instead of $links. I know that could be an issue with people who already created a custom them expecting links and if anyone has any suggestions on how to handle that, I would appreciate it.

Comments

pwolanin’s picture

A 6.x patch would be appreciated, since we are focusing active development there.

hurleyit’s picture

I will try to do one, the issue is the site I'm using this on is Drupal 5 and I don't have a Drupal 6 site using apachesolr yet.

hurleyit’s picture

Ok, I think I have a patch for 6 that will work, but I don't have a drupal 6 site that I can use to test it on where I can get matching documents. Should I create a new bug report for that patch and is there a way I could hook up a drupal 6 test site up to the beta of acquia's hosted solr project to test fully?

janusman’s picture

Please post the patch so someone (me?) can look into it =)

If you have any ideas about other theming functions, please post =)

hurleyit’s picture

Version: 5.x-1.0-alpha5 » 6.x-1.x-dev
StatusFileSize
new1.12 KB

Here is the one against the 6-dev version. Like I mentioned, I haven't been able to test fully, but if there are any issues, I will try to fix them.

pwolanin’s picture

looks ok, though I'd prbably code:

count($r) > 0
as

!empty($r)
davidseth’s picture

This patch works great! Just what I needed, more powerful MLT blocks so I can do something more than just display a list. Please commit :)

Thanks.

davidseth’s picture

Status: Needs review » Postponed (maintainer needs more info)
Issue tags: +MLT
Anonymous’s picture

Status: Postponed (maintainer needs more info) » Needs work

Tested patch. It works with beta 5 in that the code is valid, but of course will deliver some errors with the current MLT block, namely:

recoverable fatal error: Object of class Apache_Solr_Document could not be converted to string in /var/www/html/mysite/includes/theme.inc on line 1490.

...which is to be expected.

I think this is the way to go. I know I need that NID to get my pictures and item pricing to appear without having to jump through hoops.

pwolanin’s picture

Status: Needs work » Needs review
StatusFileSize
new3.13 KB

Looks basically fine - please try to roll patches from the root of the project. Also, use the issue# in the patch file name for bonus points.

The patch fails, looks like there is a conflicting commit. Here's a new version to test.

pwolanin’s picture

Anyone had a chance to test this?

Anonymous’s picture

Not yet, but I will tomorrow afternoon.

Anonymous’s picture

Status: Needs review » Reviewed & tested by the community

Tested patch, works great. This is exactly what we needed.

I know that could be an issue with people who already created a custom them expecting links and if anyone has any suggestions on how to handle that, I would appreciate it.

This is a more flexible solution IMO.

pwolanin’s picture

Status: Reviewed & tested by the community » Fixed

looks ok, but will need work for multi-site.

committed to 6.x

pokadan’s picture

Good stuff. I was just about to request this feature/improvement..
Kudos to you guys..

Status: Fixed » Closed (fixed)
Issue tags: -MLT

Automatically closed -- issue fixed for 2 weeks with no activity.