[wp-trac] [WordPress Trac] #50521: comments_pre_query doesn't align with other pre_query filters

[WordPress Trac]

#50521: comments_pre_query doesn't align with other pre_query filters
--------------------------+------------------------------
 Reporter:  dinhtungdu    |       Owner:  (none)
     Type:  defect (bug)  |      Status:  new
 Priority:  normal        |   Milestone:  Awaiting Review
Component:  Comments      |     Version:
 Severity:  normal        |  Resolution:
 Keywords:  has-patch     |     Focuses:
--------------------------+------------------------------

Comment (by dinhtungdu):

 @adamsilverstein Thanks for the reply!

 > The reasoning at the time (which I am happy to revisit) is that the
 function enables passing `count=true` and in this case we would be setting
 $this->comments to the count, not the comments. So if we do assign the
 returned value we need more logic about what/how to assign.


 Agreed. And we have the naming problem exists for the method name as well,
 with `count` set to `true`, `WP_Comment_Query::get_comments()` return the
 count, not the comments.


 > If you use `WP_Comment_Query` in your filter callback, does that mean
 you are using a WP db lookup anyway?
 >
 > Can you give some wider context for your use case, how you plan to use
 the `pre_` filter in your code?

 I didn't mean using `WP_Comment_Query` in the filter callback.

 I mean when we use `WP_Comment_Query` to query comments, the class runs
 the `get_comments` method if arguments passed are not empty. If any plugin
 hooks to `comments_pre_query` to manipulate the result, then we need to
 run the `get_comments` method again to retrieve comments.

 {{{#!php
 <?php
 // When we initialize WP_Comment_Query, get_comments method will
 // be called if the $args passed is not empty.
 $comment_query = new WP_Comment_Query( $args ); // At this point,
 $comment_query->comments is null because of short-circuiting.

 // To get comments, we need to call the get_comments method again
 $comments = $comment_query->get_comments();
 }}}

 > Do these other `pre` filters include the ability to return a count?

 Other objects like User, Term, Post assign the data back to their
 properties. The count is handled differently in each object.

 > Can you please open a separate ticket with a patch to correct that
 (assuming this is inline doc you are talking about)?

 Of course, I will do it.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/50521#comment:7>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform