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.
API page: http://api.drupal.org/api/drupal/modules--node--node.api.php/function/ho...
Describe the problem you have found:
Comments
Comment #1
jhodgdonNo description, so I'm assuming this wasn't meant to be filed. If it was meant to be filed, please reopen and describe the issue you are seeing.
Comment #2
chriscohen CreditAttribution: chriscohen commentedSorry, I'm not sure how, but the description is lost! (Maybe I should file a bug for that separately? ;)
The description's here again, anyway:
The documentation mentions $types, but does not explain how to specify what $types are. If a module is implementing this hook, how is it going to specify the $types that it wants to interact with, given that this is a hook, so the module can't control how it's invoked.
The documentation recommends reading nodeapi_example.module for an implementation, but the signature for the function in nodeapi_example_node_load() is actually ($nodes, $form), and not ($nodes, $types) at all, which is confusing.
Comment #3
jhodgdonRE: losing data:
#1112110: Preview resets information entered, if issue filing URL has query strings
It looks like two people had trouble with this in the last few days. I've bumped the issue, so maybe we can get it fixed sometime soon.
Anyway, thanks for reporting this! I think the fix should be:
a) Better doc for the $types parameter.
b) Add to the example so that it uses $types as described in the description of the hook (i.e., check types against the module's types and do an early return if it doesn't match a type of interest).
As a separate issue, nodeapi_example_node_load() in Drupal 7 has the wrong signature. I've filed this as an issue in the Examples project.
#1187560: nodeapi_example_node_load has wrong function signature
Comment #4
Mile23Looks like the nook_node_load() docs contain all the useful info... But here's a bit of an edit for clarity.
Will look at the Examples issue.
Comment #5
jhodgdonI don't like the idea of moving the narrative description down. And any description of particular parameters should go into the @param area for that parameter, not in the narrative.
Also, I forgot to move this to 8.x. The patch should be made against the 8.x branch, then ported (if necessary) to 7.x (most patches at this time apply to both).
Comment #6
Mile23OK, redone. The original issue here was: What does $types do? The @params should explain what it is, and the 'narrative' should explain how to use it.
Also added a reference to $types in the sample code, and did a little cleanup on hook_load() for clarity.
Comment #7
jhodgdonThis looks like pretty much the same patch as the last time. Again, can we leave the original narrative paragraph explaining what the patch does at the top, and if you want to have specific paragraphs about the other parameters, please put them underneath?
Also, just because someone requested adding more information about parameters doesn't mean it should go into the generic narrative. Please put the additional information about parameters into the @param sections.
Comment #8
Mile23Is there some reason this function description should begin with an unreadable paragraph about other functions?
Comment #9
jhodgdonIf it's unreadable, someone should revise it. But the information tells how this hook is invoked, and where in the node loading process.
Comment #10
Mile23Yes, exactly. There is no way to revise it to make it more readable without consuming a huge amount of space on the page, and that same paragraph is repeated within many other docblocks for other functions in that file. Therefore, I put something readable at the top, in order to convey useful information other than the loading cycle, which I judge to be very useful information but not absolutely vital to understanding the function. My initial readable paragraph explains the $types parameter, *in addition to* the @params explanation at the bottom.
There is no where else to go with this, in terms of brevity, readability and the information desired. You are welcome to ignore my patch if this isn't adequate.
Comment #11
jhodgdonI'm sorry that you don't understand that first paragraph. There is one error in there -- field_attach_node_revision() should be field_attach_load_revision(). But other than that, it's an accurate statement of the sequence of node loading. Those paragraphs were added to all of the node hooks in an earlier issue, and reviewed by the community. I'm not sure how to make them clearer without expanding them to include a lot of background information that is hopefully on http://api.drupal.org/api/drupal/modules--node--node.api.php/group/node_... or in the drupal.org documentation.
Comment #12
Mile23"I'm sorry that you don't understand that first paragraph."
No need to be snarky.
It's a bit of a waste of time to try to read that super-dense paragraph only to realize it doesn't contain the information one is seeking. Thus, I moved it to the end, where the user who actually is seeking its benefit can enjoy breezily scanning the preceding paragraphs to find the proverbial pot of gold at the end of the rainbow. Simultaneously, this leaves the joy of discovery available to the non-hook-sequence-seeking masses, with considerably reduced frustration.
If it is our job to kill rainbows, then by all means tell me why the ordering is important and I'll roll another patch. That's all I've wanted.
Comment #13
jhodgdonMy sincere apologies - I was not trying to be snarky, and I'm sorry it sounded that way.
So. I still don't like the order of information that you proposed in your patch, but I also see your point about the offending paragraph about when the hook is called.
How about this:
a) Move the paragraph that starts "This hook should only..." up to the top (after the first line), and reword it to explain what the hook should be used to do, like maybe adding a sentence at first that doesn't have the word "only" in it, or taking the "only" out, so the proper rather than the improper use is emphasized. I.e., make this paragraph into a summary of what the hook is and isn't used for.
b) Move the paragraph about when the hook is called to the end, and fix the wrong function in it [field_attach_node_revision() should be field_attach_load_revision().]
c) Move the information that is specific to specific parameters to the @param section for that parameter.
Comment #14
Albert Volkman CreditAttribution: Albert Volkman commentedHere ya go rainbow killer ;-) Appreciate your work, @jhodgdon!
Comment #15
jhodgdonThis looks good to me! I'll leave it at RTBC for a bit before committing it to see if any of the other participants here would like to comment. Thanks!
Comment #16
jhodgdonWell, I think this has been here long enough... but unfortunately the patch needs a reroll as it does not apply to D8 any more. :(
Comment #17
Albert Volkman CreditAttribution: Albert Volkman commentedRe-roll.
Comment #18
jhodgdonThis bit didn't make it into the re-rolled patch:
Other than that, looks good...
Comment #19
Albert Volkman CreditAttribution: Albert Volkman commentedSorry about that.
Comment #20
jhodgdonThanks, I'll get this committed shortly!
Comment #21
jhodgdonCommitted to 8.x -- thanks again! The patch doesn't apply to 7.x so it needs a port.
Comment #22
Albert Volkman CreditAttribution: Albert Volkman commentedBackported.
Comment #23
jhodgdonThanks! Committed to 7.x.