Make WordPress Core

Opened 8 years ago

Closed 4 years ago

Last modified 4 years ago

#36399 closed enhancement (fixed)

Change function signature of `wp_count_terms()` to be compliant with recent `get_terms()` changes

Reported by: flixos90's profile flixos90 Owned by: flixos90's profile flixos90
Milestone: 5.6 Priority: normal
Severity: normal Version: 4.6
Component: Taxonomy Keywords: has-patch has-unit-tests has-dev-note
Focuses: Cc:


In #35495 the $taxonomy parameter of get_terms() was made optional, thus changing the function signature to the first parameter being the $args array where the taxonomy (if needed) could then be added to.

This ticket is about changing the signature of wp_count_terms() in a similar manner. This will bring more consistency and it will also allow to count all terms entirely (if no taxonomy is provided).

Attachments (1)

36399.diff (2.4 KB) - added by flixos90 8 years ago.

Download all attachments as: .zip

Change History (12)

8 years ago

#1 @flixos90
8 years ago

  • Keywords has-patch added

36399.diff is a patch for this. Specifically we need to determine if the check for $do_legacy_args is safe enough here. As an alternative, we could create a new internal function like _get_terms_defaults() which would allow us to do the same kind of verification as in get_terms().

#2 @chriscct7
8 years ago

  • Version trunk deleted

#3 @ocean90
8 years ago

  • Version set to trunk

#4 @chriscct7
8 years ago

  • Summary changed from Change function signature of `wp_count_terms()` to be comliant with recent `get_terms()` changes to Change function signature of `wp_count_terms()` to be compliant with recent `get_terms()` changes

This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.

4 years ago

This ticket was mentioned in PR #450 on WordPress/wordpress-develop by felixarntz.

4 years ago

  • Keywords has-unit-tests added

#7 @flixos90
4 years ago

  • Milestone set to 5.6
  • Owner set to flixos90
  • Status changed from new to assigned

I've opened a pull-request with an updated version of this change. It also includes:

  • tests to cover both new and legacy usage of wp_count_terms()
  • all references in core code have been updated to use new signature

As originally planned, because this function is widely used and there is no actual drawback to use the old signature, legacy usage shouldn't trigger a deprecated notice (similar like with get_terms()). Yet, proceeding with this change makes sense for consistency and to make it obvious that wp_count_terms() can also count all terms, not only terms of certain taxonomies.

#8 @flixos90
4 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 48840:

Taxonomy: Allow for wp_count_terms( $args ) signature, making passing a taxonomy optional.

This brings wp_count_terms() in line with other taxonomy functions such as get_terms() which technically no longer require a taxonomy. Similar to the previously modified functions, no deprecation warning is triggered when using the legacy signature.

Fixes #36399.

#9 @SergeyBiryukov
4 years ago

In 48843:

Docs: Update the description for the $legacy parameter of wp_count_terms() for consistency with get_terms().

Follow-up to [48840].

See #36399.

#10 @desrosj
4 years ago

  • Keywords needs-dev-note added

Let's call this out in the miscellaneous dev note, noting the new, preferred signature.

Note: See TracTickets for help on using tickets.