Kick comment rendering out of node module

looks good on a first pass, subscribing.

Visually looks great, but i can't say about functionality. Let's wait for test results and then see what happens.

The exception you got in testing is further evidence that the poll module is mostly around to cause trouble, and I mean in a good way that shows us what's broken. Also, subscribing and interested in seeing this land in core.

Let's fix that exception. :-)

@moshe I moved the code out of node.module that reads the max number of comments on nodes, which is used for ranking search results. The node module should ideally be completely oblivious to the whole comment system.

The search ranking changes are causing the fail.

I'll try to babysit this patch over the weekend.

@moshe I'll take a look tomorrow at work.

Running tests now...

I ran all tests twice, and I got failure both times on "First record inserted as expected."

Code looks good, functionality works, all tests passed for catch. It's good!

All the code looks good - there's one db_result which needs converting to dbtng but it's a copy/paste from elsewhere so will get caught at some point. All tests pass too (can't reproduce the failures reported earlier with or without the patch). Lots and lots of cruft removed :)

Cross-posted, I agree this is RTBC.

Untested but should do the trick (re: dbtng conversion)

Oops sorry, not unidiff

This was committed to CVS HEAD.

w00t! thanks Dries!

   $variables['title']     = check_plain($node->title);
+  $variables['page']     = (bool)menu_get_object();
...
   // Render all remaining node links.
   $variables['links']     = !empty($node->content['links']) ? drupal_render($node->content['links']) : '';
+  
+  // Render any comments.
+  $variables['comments']     = !empty($node->content['comments']) ? drupal_render($node->content['comments']) : '';
...
+    
...
-}
+}
\ No newline at end of file

Spacing/newline errors.

@@ -392,22 +390,21 @@ function hook_nodeapi_validate($node, $f
...
 * will be called after hook_view(). The structure of $node->content is a renderable 

Line exceeds 80 chars.

+++ themes/pushbutton/node.tpl.php	31 Dec 2008 00:43:06 -0000
     <?php if ($links): ?>
     <div class="links">&raquo; <?php print $links ?></div>
+    
+    <?php print $comments; ?>
+      
     <?php endif; ?>

$comments are only output if $links are output for pushbutton.

Committed to CVS HEAD. Thanks.

Catch was so kind to enlighten and point me to this issue from #410674: Remove $page parameter from hook_node_view() implementations. This patch introduced magic we don't really want.

  <tha_sun|pissed>	It seems like the $page argument was removed in D7 (good), but replacing it with menu_get_object() === bad.
tha_sun|pissed: not everything that isn't a teaser is a page.

  <tha_sun|pissed>	Why was $page removed then?
tha_sun|pissed: actually, page isn't removed - it's still available in templates.
tha_sun|pissed: but menu_get_object() is a standard way to know if you're on the official page for the thing you're working with.
tha_sun|pissed: $page was just a special case version of that for nodes.

  <tha_sun|pissed>	So, nodes can't ever be displayed in full on an arbitrary path anymore.
tha_sun|pissed: erm, why not?

  <tha_sun|pissed>	Because each and every module manually checks menu_get_object()
  <tha_sun|pissed>	You've a Panels page on my/foo/path/that/is/NOT/node, tada.
  <tha_sun|pissed>	You want to expose a full node page view to an external service, on cron runs, impossible.
tha_sun|pissed: ah I see, set $page to true manually, can't do that any more.

Interesting, I just opened #445902: (bool) menu_get_object() is *not* an equivalent of $page. Let's move the discussion there.

I believe that the good place for $page is somewhere in an alter hook, probably not in hook_page_alter(), as I suggested there, but on a special alter hook to be added at the bottom of node_show().