Move hardcoded call to output comments from node_show() to hook_nodeapi() [#87326]

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

Comment	File	Size	Author
#7	comment_41.patch	3 KB	neclimdul
#7
#3	comment_39.patch	15.7 KB	RobRoy
#3
#2	comment_38.patch	16.09 KB	RobRoy
#2
	comments_to_nodeapi.patch	1.87 KB	RobRoy

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

RobRoy CreditAttribution: RobRoy commented 4 October 2006 at 00:41

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

Comment #2

RobRoy CreditAttribution: RobRoy commented 17 October 2006 at 02:11

File	Size
comment_38.patch	16.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! :)

Comment #3

RobRoy CreditAttribution: RobRoy commented 17 October 2006 at 02:11

File	Size
comment_39.patch	15.7 KB

Now it has UNIX line endings.

Comment #4

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 17 October 2006 at 02:57

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.

Comment #5

RobRoy CreditAttribution: RobRoy commented 17 October 2006 at 04:07

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

Comment #6

RobRoy CreditAttribution: RobRoy commented 25 January 2007 at 21:34

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

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

Comment #7

neclimdul

He/Him

CreditAttribution: neclimdul commented 25 January 2007 at 22:39

Status:

Needs work

» Needs review

File	Size
comment_41.patch	3 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.