"Add new comment" text appears in search result snippet

Not only "Add new comment" link but you will also get the read count prefixed in the excerpt if you are counting page reads.
The patch disables sorting of $build during drupal_render so that if there are links added by other modules they will only show up (if they have to) after the body. And also, it disallows comment module to add the "add new comment" link if view mode is 'search result'. Not sure if page read count be allowed in the excerpt.

The attached patch seems to resolve the issue - thanks rayasa - but I'm wondering whether it's the best way/place to implement it...
When I encountered the same problem as the OP, I expected (but failed) to fix the "Add Comment" link being displayed in search snippets by visiting the "Content Type" configuration page at admin/structure/types/manage/page/display.

On the "Manage Display" tab here, you can define custom display settings for how the labels and formats of fields should appear in the "Search index" and "Search result".
However, on the "Comment Display" tab, you don't seem to get equivalent options for how comments (or, the "Add new comment" link) appear in Custom Display modes. This would seem to be a natural place for a checkbox to allow users to choose how comment-related links should be styled in search results and indexes, making them consistent with the handling of other fields attached to nodes

Just my thoughts, anyway!

Actually, scrap that last comment - I was thinking about how the comment body itself is included in search indexing/results, which is a separate issue.
I can't think of any reason why you'd ever want to display the "Add Comment" link in a search result snippet or index it!

So, I'm all for approving the patch in #1. I'll let someone more experienced than me determine if it should be RTBC or not though. And should this be in the Comment issue queue?

This is related to an earlier issue that I can't find now, where this line was put in:

-    elseif ($view_mode != 'search_index') {

It seems perfectly reasonable to me to amend it like this patch does:

+    elseif ($view_mode != 'search_index' && $view_mode != 'search_result') {

However, the next hunk of this patch is missing a period at the end of the comment:

-      // indexing.
+      // indexing or constructing a search result excerpt

And what is this doing -- is this bit left over from another patch?

--- modules/node/node.module	2010-09-14 13:15:15.000000000 +0530
+++ modules/node/node.module	2010-09-14 13:17:04.000000000 +0530
@@ -1583,6 +1583,7 @@ function node_search_execute($keys = NUL
     $node = node_load($item->sid);
     $build = node_view($node, 'search_result');
     unset($build['#theme']);
+    $build['#sorted'] = TRUE;

???

Here's the earlier issue that removed "add new comment" from search indexing, if anyone's interested...
#721374: "Add new comment" is put into the search index along with each node

Also we should add a test to this patch, so it doesn't break again.

I don't see how this issue is related to the one in #6. Here, the string "add new comment" is never added to the search index ( i.e. The fix for the issue in #6 is not broken yet. ). The issue here is with the construction of the search excerpt and this patch removes "add new comment" string from getting into the search result snippet.

The second hunk of the patch ($build['#sorted'] = TRUE;) is well intended and not by mistake. I see no reason why we should allow drupal_render to sort the build meant for 'search result' view. The body of the result should always be put before the links and these get switched if we sort.

Resubmitting the patch against head...

That sorting is a separate issue -- please file it as a separate issue rather than combining it here with this unrelated problem.

And this patch still needs a test.

Thank you... will file a separate issue. Test coming up (my first one).

Here's a patch containing just a test, which fails. I will also attach a patch with the fix, in the next comment.

Here's the above fix from #8 (without the unrelated issue), and the test from #14, which should now pass (at least, it does for me).

This is a D6 regression. I think it should be fixed before release.

It might be better to flip the logic and only add the link if we are in a normal/full display.

It might be better -- I'm not sure. Here's the current function:
http://api.drupal.org/api/drupal/modules--comment--comment.module/functi...

It's going through several special-case view modes, to decide whether to print out the "add new comment" and "N comments so far" links:
- RSS
- teaser
- default -- which we patched in #721374: "Add new comment" is put into the search index along with each node to be "everything but search indexing", and the proposed fix here is to make it "everything but search indexing and search snippeting".

I'm not the maintainer of the node or comment modules, and I'm not sure what other "view modes" there might be -- should the default for unknown view modes be that they act like the default view mode (which is to say, print out the links), or should the default be to omit the links? I'm inclined to say the default should be to print out the links (as is done in this patch), but I'm not sure....

I'd tend to think we should go for a no-link default for unknown modes.

I'm OK with that idea, but I think we should get a philosophical OK from webchick/Dries before we bother with a patch to make that change at this point in D7.

pwolanin and I investigated this in IRC today. We discovered that in D6 the add new comment link is added by comment_link(), i.e. hook_link(), which is invoked independent of build mode from node_view().

In D6, however, search indexing and search results do not use node_view(), which is why they don't show the add new comment link (hook_link is not invoked for either one, and taxonomy and comments are invoked in a special way, and there are some special nodeapi hooks as well).

So in D7 this is a regression that it is showing, but only for the case of search results, and we should probably leave the default as it was in D6 -- show comment links generically, and make exceptions for search results/indexing.

We took care of indexing previously, and this patch takes care of results... so I think it's OK as it is?

Yes, seems like this is the most reasonable fix at this point.

Committed to HEAD. Thanks!