Make WordPress Core

Opened 2 years ago

Closed 2 years ago

#56331 closed enhancement (fixed)

Confusing argument name in `wp_set_post_terms()`

Reported by: desrosj's profile desrosj Owned by: desrosj's profile desrosj
Milestone: 6.1 Priority: normal
Severity: normal Version: 2.8
Component: Posts, Post Types Keywords: good-first-bug has-patch commit
Focuses: Cc:

Description

The wp_set_post_terms() function updates the terms for a taxonomy assigned to a given post based on the arguments supplied.

The second argument is $tags. This name makes sense when using the default value of post_tag for the taxonomy argument, but feels weird when any other taxonomy is passed.

At the core, tags are just terms. Changing $tags to $terms would make the function more generic, interchangeable, and less confusing.

Attachments (4)

#56331.patch (1.4 KB) - added by hilayt24 2 years ago.
Argument name changed along with comment
#56331.2.patch (2.4 KB) - added by hilayt24 2 years ago.
$tags changed to $terms in the function
#56331.3.patch (2.4 KB) - added by hilayt24 2 years ago.
Every Instance of tags changed to tems
56331.4.patch (2.4 KB) - added by krishaweb 2 years ago.
Here is the updated patch per the @SergeyBiryukov suggestion.

Download all attachments as: .zip

Change History (14)

#1 @hilayt24
2 years ago

@desrosj , I agree with your suggestion here I am adding patch for the same here.

@hilayt24
2 years ago

Argument name changed along with comment

#2 @costdev
2 years ago

  • Keywords changes-requested added

Hi @hilayt24, thanks for the patch! You'll also need to change all occurrences of $tags inside the function to $terms.

Last edited 2 years ago by costdev (previous) (diff)

#3 @hilayt24
2 years ago

@costdev, Thank you for spotting that.

@hilayt24
2 years ago

$tags changed to $terms in the function

#4 @costdev
2 years ago

Thanks @hilayt24! There's also the "tag delimiter" string, which should be changed to "term delimiter"

#5 @hilayt24
2 years ago

@costdev, Ohh thanks !!

@hilayt24
2 years ago

Every Instance of tags changed to tems

#6 @costdev
2 years ago

  • Keywords has-patch added; changes-requested removed

LGTM! 👍

#7 @SergeyBiryukov
2 years ago

Thanks for the patch!

The context for _x( ',', 'tag delimiter' ); should not be changed here, as this string is used in other places of core, and changing the context will unnecessarily duplicate it.

@krishaweb
2 years ago

Here is the updated patch per the @SergeyBiryukov suggestion.

#8 @costdev
2 years ago

My mistake! 😅

#9 @desrosj
2 years ago

  • Keywords commit added

Thanks for the patches, everyone!

This is looking good to go. There is one more instance of $tags that needs to change near the end. But I can fix that one before committing to avoid having to create another patch.

#10 @desrosj
2 years ago

  • Owner set to desrosj
  • Resolution set to fixed
  • Status changed from new to closed

In 53825:

Posts, Post Types: Change variable name in wp_set_post_terms() for clarity.

This changes the $tags variable used within wp_set_post_terms() to $terms.

While the default for the $taxonomy argument is post_tag, the function can be used to assign terms to a post for any taxonomy. When a different taxonomy is passed, $tags is an inaccurate name and could be confusing.

Props hilayt24, costdev, SergeyBiryukov, krishaweb, desrosj.
Fixes #56331.

Note: See TracTickets for help on using tickets.