Make WordPress Core

Opened 14 years ago

Closed 14 years ago

Last modified 14 years ago

#15407 closed defect (bug) (fixed)

get_the_category code duplication

Reported by: filosofo's profile filosofo Owned by: filosofo's profile filosofo
Milestone: 3.1 Priority: normal
Severity: normal Version: 3.1
Component: Taxonomy Keywords: has-patch
Focuses: Cc:

Description

get_the_category needlessly duplicates the functionality of get_the_terms, and it lacks a filter that the corresponding get_the_tags has.

Attachments (4)

get_the_category.dry.15407.diff (965 bytes) - added by filosofo 14 years ago.
deprecate-get_the_category.15407.diff (5.6 KB) - added by filosofo 14 years ago.
15407.diff (787 bytes) - added by nacin 14 years ago.
15407.fix-for-no-category-posts-fatal-error.patch (639 bytes) - added by Viper007Bond 14 years ago.

Download all attachments as: .zip

Change History (25)

#1 @scribu
14 years ago

Shouldn't the filter be named 'get_the_category' ?

#2 @filosofo
14 years ago

Well, my thought was to be consistent with get_the_tags, so that the general form would be get_the_[taxonomy]s

The function itself, get_the_category(), is rather poorly named.

#3 @scribu
14 years ago

How about we deprecate get_the_category() and add a get_the_categories() then?

#4 @filosofo
14 years ago

I'll make a patch.

#5 @filosofo
14 years ago

deprecate-get_the_category.15407.diff deprecates and replaces get_the_category().

#6 @automattor
14 years ago

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

(In [16332]) Replace get_the_category() with get_the_categories(). Props filosofo. Fixes #15407

#7 @nacin
14 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

For consistency, the_category() with the_categories()?

These are very commonly used template tags so I'm not sure deprecating them for a pluralization is worth it. We've been fine for 8+ major releases with them as is.

#8 @filosofo
14 years ago

I'm not wedded to the name change, but I think the code duplication fix of the original patch should stay.

I also think that get_the_categories as a filter makes more sense than get_the_category or the like, because it's filtering an array.

#9 @filosofo
14 years ago

And actually, maybe it wouldn't be a bad idea to go with the_categories(). It's easier to remember, and the Extend theme reviewers will love having the_category() deprecated so they can smite even more themes. :)

#10 @westi
14 years ago

  • Cc westi added
  • Keywords revert added

The name change needs to be reverted - we should and will not rename a function just for the sake of the name being more proper.

#11 @westi
14 years ago

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

(In [16334]) Revert most of [16332] - renaming and deprecating a commonly used function is silly. Improving it is good. Fixes #15407.

#12 follow-up: @Denis-de-Bernardy
14 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

The newly introduced filter's name is plural and inconsistent with the function's name. Suggesting we rename it accordingly.

#13 @filosofo
14 years ago

It makes more sense to name a filter according to the use of the filter rather than its containing function, especially when the function is poorly named.

#14 in reply to: ↑ 12 @westi
14 years ago

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

Replying to Denis-de-Bernardy:

The newly introduced filter's name is plural and inconsistent with the function's name. Suggesting we rename it accordingly.

This is intentional.

#15 @nacin
14 years ago

(In [16343]) Move cast to within array_keys to prevent warning when get_the_terms returns false. see #15407.

#16 @nacin
14 years ago

  • Keywords revert removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

After an IRC discussion with Viper007Bond, the issue here is that get_the_terms() may return false. I was trying to prevent issues there, but it may be cast awkwardly into an array(0=>null) which then causes some problems.

Shuffling things around here a bit.

@nacin
14 years ago

#17 @nacin
14 years ago

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

(In [16395]) Properly force variable to an array. props Viper007Bond, fixes #15407.

#18 @Viper007Bond
14 years ago

Whoops, I guess you saw my message in IRC before you timed out. :)

#19 @ryan
14 years ago

(In [16487]) Populate post term relationship cache in get_the_terms(). Restores caching lost when we moved away from get_the_category(). see #15407

#20 @gazouteast
14 years ago

I struggled to follow this one - are you saying that get_the_category() and the_category() are now deprecated?

#21 @nacin
14 years ago

No. They were deprecated, then undeprecated.

Note: See TracTickets for help on using tickets.