This patch for HEAD moves the archaic call to append comments to the node body in node_show(). It should be way easier than it currently is to move where the comments show up in your node.tpl.php and this should help do that.

Probably need to add a $comments = TRUE param to node_view() so functions can choose to omit comments (like Views showing full nodes). But I want some input as to where this should be implemented. Would passing that $comments param through to node_build_content() then to hook_nodeapi() be the best way? Or, would it be better to let Views and other modules deal with that in hook_nodeapi('alter')? I think the latter is the best approach.

I'd like some input! :P

CommentFileSizeAuthor
#7 comment_41.patch3 KBneclimdul
#3 comment_39.patch15.7 KBRobRoy
#2 comment_38.patch16.09 KBRobRoy
comments_to_nodeapi.patch1.87 KBRobRoy

Comments

RobRoy’s picture

RobRoy commented

Also, that cid arg part is bad. How should we pull that?

RobRoy’s picture

RobRoy commented
StatusFileSize
new comment_38.patch16.09 KB

Cool, thanks to hunmonk I got a better direction on this. It looks like a biggie, but it's really not. I just removed the 2nd arg from comment_render and un-indented a huge chunk of code because AFAIK it isn't being used anymore.

This patch pretty much just moves comments to use the new content array and cleans up some super outdated comment/node code that is related to that, therefore it's in the same patch. This is important for modifying where comments are rendered.

Trivial demo, just set the weight to -100 in nodeapi 'alter' to show comments on top of a node now! Before, you'd have had to modify core or do some other nastiness to do so.

Pretty please review my patch! :)

RobRoy’s picture

RobRoy commented
StatusFileSize
new comment_39.patch15.7 KB

Now it has UNIX line endings.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs review » Postponed

I'd love to see this in core, but unfortunately it's an API change which we can't have this late in the release cycle. :( Marking postponed.

RobRoy’s picture

RobRoy commented

Bless your heart. I'll make sure to get this in for...ugh, 6.0?

RobRoy’s picture

RobRoy commented
Version: x.y.z » 6.x-dev
Status: Postponed » Needs work

Updating status. This patch is old, so don't hold it against me!

neclimdul’s picture

neclimdul
He/Him
commented
Status: Needs work » Needs review
StatusFileSize
new comment_41.patch3 KB

Simplified patch. This patch retains the same comment_render function however that may need to happen.

Problems caused by this patch that need to be addressed:

  • Node links(Add new comment) show up below the comments. This is a product of the way the links are rendered.
  • node/nid/cid view a single comment functionality is broken. This is why comment_render *could* be cleaned of the single comment rendering code.

A walk through of the rendering process:

node_show()
 |-> node_view()
       |-> node_build_content() - comments are rendered into the body
       |     |-> node_invoke_nodeapi($node, 'view', $teaser, $page);
       |           |-> comment_nodeapi() - adds comments
       |-> theme('node',...) adds links <em>after</em> the body

Since the body is rendered in node_build_content() before the theme function is called comments are always added before the links.

Assuming this patch where to be commited(which I believe it should) either links should be handled differently of the theme function should be changed to handle this case. Currently I'd lean towards handling the links different but not heavily and would like some feedback.

RobRoy’s picture

RobRoy commented
Status: Needs review » Closed (duplicate)

Efforts should be focused on Eaton's new node rendering patch at http://drupal.org/node/134478.

RobRoy’s picture

RobRoy commented

Efforts should be focused on Eaton's new node rendering patch at http://drupal.org/node/134478.