Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.This watchdog line in search.pages.inc (around line 28) is trying to invoke hook_search() and should maybe/probably be using hook_search_info now?
if (trim($keys)) {
// Log the search keys:
watchdog('search', '%keys (@type).', array('%keys' => $keys, '@type' => module_invoke($type, 'search', 'name')), WATCHDOG_NOTICE, l(t('results'), 'search/' . $type . '/' . $keys));
| Comment | File | Size | Author |
|---|---|---|---|
| #15 | 674988.patch | 1.09 KB | jhodgdon |
| #12 | 674988.patch | 1.09 KB | jhodgdon |
| #8 | searchpage.png | 7.57 KB | jhodgdon |
| #3 | nopatch.png | 9.73 KB | jhodgdon |
| #3 | withpatch.png | 9.18 KB | jhodgdon |
Comments
Comment #1
jhodgdonThis should fix that watchdog entry.
Comment #2
jhodgdonJust as a clarifying note, hook_search() no longer exists in Drupal 7. It was broken up into different hooks, and hook_search_info() is the one where a search-activated module tells Search what the name of its search is.
Comment #3
jhodgdonSee attached screen shots of watchdog log entry details without and with this patch. The format of the message is
keys (search type).
(I searched for "abc" in both cases).
Comment #4
Dries CreditAttribution: Dries commentedThe "(Content)" part is confusing/redundant to me. Can't it be removed?
Comment #5
jhodgdonThat is what the message did in D6 - it showed whether you were searching for content or users or whatever (e.g. some custom search module provided a search type). Are you sure you want it removed? Another option would be to format the message something like:
Searched for ____ in _____
Maybe that would be clearer?
Current message if you search in "Users" instead of "Content":
abc (Users).
Comment #6
jhodgdonAnd just as a note, all my patch did was restore the functionality that was already there, but which wasn't working because whoever updated all the search hooks missed this one call.
Comment #7
Dries CreditAttribution: Dries commentedMaybe the type should be 'user search' vs 'content search'?
Comment #8
jhodgdonThe type string (User or Content) is coming from what the module provides in hook_search_info(), and is the same as the tabs/links provided on the main search page. I am not sure just adding "search" after it would be appropriate for all search sub-modules, as it is never displayed that way on the main search page (see attached screen shot of search page).
Here are some variations I can think of (keep in mind that Content and User are the two core possibilities for display of the Type):
1. abc (Content)
2. Searched for abc in Content
3. Searched abc in Content
4. Search: abc in Content
5. Search: abc (in Content)
6. Searched Content for abc
7. Keys: abc (Type: Content)
8. Keys: abc (Search type: Content)
9. Keys: abc (in Content)
#1 is the current/D6 version. I think #6 is my current personal favorite. Any of those (or another variation) seem reasonable to you? Keep in mind that the log also shows "search" as the log entty type in the log, so putting the word "search" in the log entry may not be necessary... Still, I think most Drupal messages are a bit more verbose than #1.
Comment #9
Dries CreditAttribution: Dries commentedThe type string comes from
watchdog($type, $message, $variables = array(), $severity = WATCHDOG_NOTICE, $link = NULL). It might not be best to create more types though so let's just pick one of your suggestions.Comment #10
jhodgdonOK. I'll make a patch with message variation #6
later todaytomorrow, unless you have a preference for one of the others. Just about anything would be an improvement over the cryptic D6 message (variation #1).Comment #11
jhodgdonComment #12
jhodgdonHere's a patch that makes the message:
Searched Content for abc
Searched Users for abc
Comment #13
jhodgdonComment #14
Dries CreditAttribution: Dries commentedI think we should use %type instead of @type because otherwise the capitalization looks funny. Not?
Comment #15
jhodgdonAgreed. %type makes the message:
Searched Content for abc.
Definitely better.
Attached patch.
Comment #16
Dries CreditAttribution: Dries commentedGreat. Committed to CVS HEAD. Thanks.