The DocBlock comments on comment_reply()
document the return value thus:
/**
* @return
* The rendered parent node or comment plus the new comment form.
*/
That's not correct. What's actually returned is an array containing a renderable array for the node or parent comment and the comment form as an array. I propose the comment be changed to read something like this:
/**
* @return array
* An associative array containing:
* - An array for rendering the node or parent comment.
* - comment_node: If the comment is a reply to the node.
* - comment_parent: If the comment is a reply to another comment.
* - comment_form: The comment form as a renderable array.
*/
When there's consensus on the change I'll roll a patch.
Comment | File | Size | Author |
---|---|---|---|
#7 | comment-comment_reply-1532682-7.patch | 800 bytes | TravisCarden |
#4 | comment-comment_reply-1532682-4.patch | 1.25 KB | TravisCarden |
#3 | drupal-1532682-3.patch | 1.21 KB | TravisCarden |
Comments
Comment #1
TravisCarden CreditAttribution: TravisCarden commentedTagging.
Comment #2
lotyrin CreditAttribution: lotyrin commentedI like this, but I'm not sure about the established conventions for describing the presence of things like comment_node/comment_parent. In fact, I wonder why the key here needs to change at all.
Please go ahead and make patches in situations like this, that way the status could have been "Needs review" already and can attract the appropriate attention. Tags help too.
Comment #3
TravisCarden CreditAttribution: TravisCarden commentedCan do! Thanks.
Comment #4
TravisCarden CreditAttribution: TravisCarden commentedRe-rolled.
Comment #5
dastagg CreditAttribution: dastagg commentedThe patch updates the code as per the summary.
Comment #6
Dries CreditAttribution: Dries commentedCommitted to 8.x. Moving to 7.x. for back porting.
Comment #7
TravisCarden CreditAttribution: TravisCarden commentedGreat! Here's the same thing for d7. (Can I put it right into RTBC?)
Comment #8
David_Rothstein CreditAttribution: David_Rothstein commentedI don't see this in D8 yet.. anyone else?
Let's also move it to the documentation queue - @jhodgdon will likely snag reviewing/committing it there before any of the rest of us gets around to it :)
Comment #9
David_Rothstein CreditAttribution: David_Rothstein commentedUm, I forgot to actually move it, apparently.
Comment #10
Dries CreditAttribution: Dries commentedCommitted it to 8.x. For real now. Moving back to 7.x.
Comment #11
TravisCarden CreditAttribution: TravisCarden commentedI still don't see the commit. It should be in the repository pretty much immediately, right?
Comment #12
David_Rothstein CreditAttribution: David_Rothstein commentedYou're right, maybe it was committed but never pushed? I'm actually seeing the same thing for a few other issues too, so I will bump them back to Drupal 8 also.
#4 is still RTBC for Drupal 8. Hopefully third time's the charm :)
Comment #13
Dries CreditAttribution: Dries commentedForgot to push my changes. It is committed now: http://drupalcode.org/project/drupal.git/commit/1ed9d79.
Comment #14
David_Rothstein CreditAttribution: David_Rothstein commentedI see you learned my trick - if you paste the commit link into the issue, that guarantees you'll never forget to push :)
#7 looks RTBC for Drupal 7 to me.
Comment #15
jhodgdonThe patch in #7 has been committed (and pushed :) ) to Drupal 7.x. Thanks!