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.The documentation for hook_search at http://api.drupal.org/api/function/hook_search/6 is missing documentation of the 'admin' operation. The example provided (which comes from node_search) includes it.
Suggest adding, in the 'op' parameter definition section:
'admin': The hook should return a form array, containing any fieldsets the module wants to add to the Search settings page at admin/settings/search.
Also, it should be made clear in the "Return Value" section that this return value only applies to the 'search' operation, by adding a short note.
Note that this is an issue in Drupal 5.x, 6.x, and 7.x. So all three should be patched.
| Comment | File | Size | Author |
|---|---|---|---|
| #20 | jamesan_411388-20-D5.patch | 4.52 KB | JamesAn |
| #20 | jamesan_411388-20-D6.patch | 4.52 KB | JamesAn |
| #17 | jamesan_411388-16-D5.patch | 4.59 KB | JamesAn |
| #17 | jamesan_411388-16-D6.patch | 4.6 KB | JamesAn |
| #11 | issue-411388.patch | 4.81 KB | lilou |
Comments
Comment #1
jhodgdonThis would be a good "Novice" project.
There is some instruction at http://drupal.org/node/144223 (Updating API Documentation) about how to modify API documentation. Note that in Drupal 7, the hook_search API doc page is generated from a page in the main Drupal source code tree, but in earlier versions, it is in the "contrib" tree (see referenced page on Updating API Documentation for more information).
Comment #2
JamesAn CreditAttribution: JamesAn commentedThanks for the bite-size task. ^_^
Here's the patch. I took the liberty of fleshing out the return value in more detail, as I've been frustrated before reading incomplete documentation. I've documented the expected return value for all the operations.
I've gotta go, but Drupal 5 and 6 versions are coming up. I'll finish them tonight.
Comment #3
JamesAn CreditAttribution: JamesAn commentedHere are the D5 and D6 patches.
Comment #4
JamesAn CreditAttribution: JamesAn commentedGah. Always forgetting to flip the status to needs review. Here we go!
Comment #5
mr.baileysProbably needs some fine-tuning: capitalization and periods at the end of sentences are a bit inconsistent. Sometimes colons are followed by a capitalized letter, sometimes they're not. Some sentences end with a period, others do not.
Comment #6
JamesAn CreditAttribution: JamesAn commentedComment #7
JamesAn CreditAttribution: JamesAn commentedBlah. Forgot to flip the switch.
Comment #8
JamesAn CreditAttribution: JamesAn commentedComment #9
mcrittenden CreditAttribution: mcrittenden commentedCouple of really picky things: lines 38, 53, 56, and 63 go over the recommended 80 characters. There's a period in the middle of the sentence "... indexing bookkeeping so that it. starts from scratch ...". Line 32 should have a comma after "the module name" (common mistake when dealing with items in a series...see http://www.chompchomp.com/terms/iteminaseries.htm).
Sorry to be so picky, somebody's gotta do it :).
Comment #10
mcrittenden CreditAttribution: mcrittenden commentedForgot to switch status.
Comment #11
lilou CreditAttribution: lilou commentedReroll according to #9
Comment #12
lilou CreditAttribution: lilou commented...
Comment #13
Dries CreditAttribution: Dries commentedI've committed this to CVS HEAD because it is an improvement. Feel free to add follow-up patches with refinements to this issue, if necessary. I don't think it is. Thanks!
Comment #14
jhodgdonNeeds port to D6, and possibly to D5 (are we still patching D5?).
Comment #15
JamesAn CreditAttribution: JamesAn commented#6 contains the D6 and D5 patches. Can someone review this? Thanks.
Comment #16
jhodgdonSee #9 - it looks like the D5 and D6 patches in #6 suffer from the same problems.
Comment #17
JamesAn CreditAttribution: JamesAn commentedOy vey! Apologies for not reading everything... I assumed everything was fixed with I saw the D7 commit message.
Here's the D6 and D5 patches fixed according to #9.
Comment #18
jhodgdonPickiness:
+ * hook_update_index() should update their indexing bookkeeping so that it.
* starts from scratch the next time hook_update_index() is called.
This sentence has a period in the middle of it.
Otherwise, it looks great!
Comment #19
jhodgdonJamesan - do you have Contrib access to commit those patches (with the minor period change)? If so, I say go ahead and commit them.
Comment #20
JamesAn CreditAttribution: JamesAn commentedNice catch. Silly me and inserting periods at the wrong places. All fixed.
I just got my CVS account a couple days ago for GSoC. Still figuring out how to do what and where. Let me figure out how to commit this patch as a test run. ^_^
Comment #21
jhodgdonSounds good, JamesAn. If you need any guidance, please ask for help.
Comment #22
JamesAn CreditAttribution: JamesAn commentedMethinks I did it!
Question about the commit message:
The doc (and most people) format the message with "#12345 by username: Brief description of changes.", but Dries consistently commits with "- Patch #12345 by username: Brief description of changes.". I know its minor, but does the difference matter?
Comment #23
jhodgdonThe important thing is that someone looking at the commit message can find what issue it came from, see who contributed to the changes, what you changed, and why. So I don't think the difference is significant.