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.
In the hook_link the comment_count is retrieved by calling function 'comment_num_all()':
function comment_link($type, $node = NULL, $teaser = FALSE) {
$links = array();
if ($type == 'node' && $node->comment) {
if ($teaser) {
// Main page: display the number of comments that have been posted.
if (user_access('access comments')) {
$all = comment_num_all($node->nid);
if ($all) {
function comment_num_all($nid) {
static $cache;
if (!isset($cache[$nid])) {
$cache[$nid] = db_result(db_query('SELECT comment_count FROM {node_comment_statistics} WHERE nid = %d', $nid));
}
return $cache[$nid];
}
However the comment_count is already retrieved in the nodeapi load which is always runned before hook_link.
function comment_nodeapi(&$node, $op, $arg = 0) {
switch ($op) {
case 'load':
if ($node->comment != COMMENT_NODE_DISABLED) {
return db_fetch_array(db_query("SELECT last_comment_timestamp, last_comment_name, comment_count FROM {node_comment_statistics} WHERE nid = %d", $node->nid));
}
break;
New code:
function comment_link($type, $node = NULL, $teaser = FALSE) {
$links = array();
if ($type == 'node' && $node->comment) {
if ($teaser) {
// Main page: display the number of comments that have been posted.
if (user_access('access comments')) {
if ($node->comment_count) {
$links['comment_comments'] = array(
'title' => format_plural($node->comment_count, '1 comment', '@count comments'),
'href' => "node/$node->nid",
'attributes' => array('title' => t('Jump to the first comment of this posting.')),
'fragment' => 'comments'
);
Comment | File | Size | Author |
---|---|---|---|
#12 | comment_hook_link_del_func_with_test_no_%s.patch | 3.86 KB | R.Muilwijk |
#10 | comment_hook_link_del_func_with_test.patch | 3.3 KB | R.Muilwijk |
#8 | comment_hook_link_del_func_with_test.patch | 3.34 KB | R.Muilwijk |
#5 | comment_hook_link.patch | 1.36 KB | R.Muilwijk |
#5 | comment_hook_link_del_func.patch | 1.83 KB | R.Muilwijk |
Comments
Comment #1
catchNot tested this yet but this looks sane to me. Since this is the only place that comment_num_all() is used I'm wondering if there's any need for it at all?
Comment #2
R.Muilwijk CreditAttribution: R.Muilwijk commentedI don't know whether or not comment_num_all() should stay. After a grep I saw only comment_link was using the function.
The patch added to this comment removes the function. I hope someone who is more familiar with the comment module can make the decision what patch to apply.
Comment #3
nielsvm CreditAttribution: nielsvm commentedThe patch looks sane to me as well, I've just ran simpletest on a CVS up from an hour ago, pre-patch and post patch:
"297 passes, 98 fails and 6 exceptions"
niels@gazprom:~/cvs/drupal$ patch -p0 < comment_hook_link_del_func.patch
patching file modules/comment/comment.module
Hunk #3 succeeded at 1076 (offset -2 lines).
"297 passes, 98 fails and 6 exceptions."
So the patch doesn't have any influence here, seems valid to me.
+1
Comment #4
chx CreditAttribution: chx commentedYay less code!
Comment #5
R.Muilwijk CreditAttribution: R.Muilwijk commentedThe comment module was cleaned up (http://drupal.org/node/258171). Patches were broken.
This are the fixed patches. Somebody still has to make a decision whether or not to delete 'comment_num_all()'
Comment #6
R.Muilwijk CreditAttribution: R.Muilwijk commentedI've been looking into the comment_num_all().
It was added Sun Dec 30 16:16:38 2001
http://cvs.drupal.org/viewvc.py/drupal/drupal/modules/comment/comment.mo...
Added "x new comments" feature. Requires a SQL update.
As of then it is used in core only for that. When nodeapi was introduced with it loading the comment_count it never got deleted. There for I think it is save to delete the function and use the comment_hook_link_del_func.patch
Looks RTBC to me.....
Comment #7
catchIn that case it looks like we can definitely delete it which means applying: http://drupal.org/files/issues/comment_hook_link_del_func_0.patch
Once that's in, we should add a note in the update modules instructions, just in case, since it's officially a public function.
Comment #8
R.Muilwijk CreditAttribution: R.Muilwijk commentedPatch applied with huge offsets. Patch now applies to correct offsets and adds a test for hook_link. I tested them and they pass after applying patch. Can someone confirm?
Comment #9
catchYep, this is fine.
Comment #10
R.Muilwijk CreditAttribution: R.Muilwijk commentedBecause of another commit to comment this patch only applied with huge offsets. New patch is up which has been tested and is still correct. (no functional changes)
Comment #11
Dries CreditAttribution: Dries commented+ $this->assertRaw('2 comments', t('Link to the 2 comments exist. %s'));
What is the %s for?
Comment #12
R.Muilwijk CreditAttribution: R.Muilwijk commented@Dries,
Normal output is like this:
%s adds this part:
Patch up without %s and with %s. Status back to reviewed and tested because it has no functional changes.
Comment #13
R.Muilwijk CreditAttribution: R.Muilwijk commentedComment #14
Dries CreditAttribution: Dries commentedThanks R. Committed to CVS HEAD.
Comment #15
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.