Wrong variable name in filter documentation

[david.binda]

In r52974 some $pieces variables were changed to $clauses including the usage in docblocks. However, some of the instances were reverted back to $pieces in r53376 but the docblocks were intact creating possibly mis-leading docblocks.

I'd like to propse to change the docblock back to the original state prior r52974

[SergeyBiryukov]

Thanks for the ticket!

IIRC, this part of the DocBlock was not reverted back to $pieces in [53376] specifically to avoid confusion. My thinking was that $clauses represents compact( $pieces ), which gets passed to the filters, not $pieces itself.

So reverting this to $pieces would be more confusing for me, but I'm curious to see what others think.

Another thing to consider is that these variable names are often copied when writing new functions attached to the hooks, and $clauses seems like a more fitting variable name.

[ironprogrammer]

I agree that $clauses more accurately describes the associate array expected here after compact( $pieces ), but it does look funny for the comment to not allude to the obvious use of $pieces in context.

The DocBlock at https://core.trac.wordpress.org/browser/trunk/src/wp-includes/class-wp-query.php#L2934 is verbose in describing what $clauses represents. Would it help for the other filter calls to include these details to avoid ambiguity?

[hellofromTonya]

Interesting. Looking at the history:

• [52974] changed $pieces to $clauses.
• [53375] for WP_Query, reverted part of the change for WP_Query -> the individual query pieces array gets assigned to $pieces and compact( $pieces ) represents the $clauses.
• [53376] did the same treatment in WP_Comment_Query, WP_Network_Query, WP_Site_Query, and WP_Term_Query.

Each of these queries are now consistent, which is good.

IIRC, this part of the DocBlock was not reverted back to $pieces in [53376] specifically to avoid confusion. My thinking was that $clauses represents compact( $pieces ), which gets passed to the filters, not $pieces itself.

So reverting this to $pieces would be more confusing for me, but I'm curious to see what others think.

I agree with @SergeyBiryukov and @ironprogrammer. Pieces and clauses are 2 different things. The naming is consistent and aligns well IMO to these 2 different purposes.

The DocBlock at https://core.trac.wordpress.org/browser/trunk/src/wp-includes/class-wp-query.php#L2934 is verbose in describing what $clauses represents. Would it help for the other filter calls to include these details to avoid ambiguity?

I agree with @ironprogrammer. The filter's DocBlock for the others should also receive the more verbose version for the $clauses. Doing so keeps them consistent, helps with understanding, and should avoid confusion.

For WP_Comment_Query, WP_Network_Query, and WP_Site_Query, expands the $clauses array structure explanation in filter DocBlock.

Benefits:

• Consistency with WP_Query and WP_Term_Query.
• Helps to improve understanding.
• Should help to avoid confusion of the purpose for pieces and clauses.

Follow-up to r52974, r53375, and r53376.

Trac ticket: https://core.trac.wordpress.org/ticket/61307

[hellofromTonya]

Patch: ​https://github.com/WordPress/wordpress-develop/pull/6869

Now available for review. What do you think?

[hellofromTonya]

Patch: ​https://github.com/WordPress/wordpress-develop/pull/6869

With an approval on the PR (thank you @johnbillion), marking for commit. Reassigning to me, as I'm preparing the commit.

[hellofromTonya]

In 58454:

Docs: Document $clauses associative array in *-clauses filters.

Document each element within the filter parameter $clauses associative array structure for the 'comments_clauses', 'networks_clauses', and 'sites_clauses' filters.

This change:

• Brings consistency amongst the WP_[*_]Query filters.
• Helps to improve understanding.
• Helps to avoid confusion of the purpose for pieces and clauses.

Follow-up to [53376], [53375], [52974].

Props david.binda, audrasjb, hellofromTonya, ironprogrammer, johnbillion, oglekler, SergeyBiryukov.
Fixes #61307.

Committed via https://core.trac.wordpress.org/changeset/58454.

[hellofromTonya]

Hello @davidbinda,

Do you agree with discussion and [58454]?

If no, please feel free to re-open this ticket to continue the discussion.

If yes, woohoo.

Thanks :)