Opened 10 years ago
Closed 8 years ago
#30415 closed enhancement (duplicate)
Add alternative to get_category_parents() that will work for any taxonomy
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | |
Component: | Taxonomy | Keywords: | needs-unit-tests needs-patch |
Focuses: | Cc: |
Description
The get_category_parents()
function is, as the name suggests, limited to only fetching the parents for the category
taxonomy. It would be great if we had the same function available for any specified taxonomy to use.
My suggested patch (attached) adds a new function, get_term_parents()
, which is a direct copy of the existing function, but with an added $taxonomy
parameter so you can use it for any given taxonomy. I have then updated the original get_category_parents()
to simply be a wrapper for this new function, so it is fully backwards compatible.
The only modification to the original function (aside from some coding standards updates) is the addition of an is_taxonomy_hierarchical()
check, which now becomes necessary.
Attachments (3)
Change History (18)
#1
@
10 years ago
- Component changed from General to Taxonomy
- Keywords has-patch needs-unit-tests added
#2
follow-up:
↓ 3
@
10 years ago
I'm just thinking, the category-template file contains mostly category specific functions. It would probably be better to move this functionality into wp-includes/taxonomy.php and call the new function from get_category_parents() to eventually deprecate it.
#3
in reply to:
↑ 2
;
follow-up:
↓ 4
@
10 years ago
Replying to jeffikus:
I'm just thinking, the category-template file contains mostly category specific functions. It would probably be better to move this functionality into wp-includes/taxonomy.php and call the new function from get_category_parents() to eventually deprecate it.
That's true and I considered that, but there are other general term functions in that file, so thought I'd stick it there for now until there's some official go ahead to move it to a different file. I don't want to mess with the file structure too much without knowing if it's the best move or not.
#4
in reply to:
↑ 3
@
10 years ago
Replying to hlashbrooke:
Replying to jeffikus:
I'm just thinking, the category-template file contains mostly category specific functions. It would probably be better to move this functionality into wp-includes/taxonomy.php and call the new function from get_category_parents() to eventually deprecate it.
That's true and I considered that, but there are other general term functions in that file, so thought I'd stick it there for now until there's some official go ahead to move it to a different file. I don't want to mess with the file structure too much without knowing if it's the best move or not.
I'll take a stab at it over the weekend, maybe tonight.
This ticket was mentioned in Slack in #core by hlashbrooke. View the logs.
10 years ago
#6
follow-up:
↓ 7
@
10 years ago
On your current patch: 30415.diff
L1399
@return string|WP_Error A list of category parents on success, WP_Error on failure.
why then return false;
? :-) Wheres the new WP_Error
?
L1421
$chain .= get_category_parents( $parent->parent, $link, $separator, $nicename, $visited );
shouldn't you call get_term_parents()
?
and a note from my side: Wouldn't it better to implement a function ( get_term_parent_tree()
) which returns the a tree (array) with parent => child ? Instead we should implement a function called get_term_parent_list_html()
.
#7
in reply to:
↑ 6
@
10 years ago
Replying to ChriCo:
On your current patch: 30415.diff
L1399
@return string|WP_Error A list of category parents on success, WP_Error on failure.
why then
return false;
? :-) Wheres thenew WP_Error
?
L1421
$chain .= get_category_parents( $parent->parent, $link, $separator, $nicename, $visited );
shouldn't you call
get_term_parents()
?
and a note from my side: Wouldn't it better to implement a function (
get_term_parent_tree()
) which returns the a tree (array) with parent => child ? Instead we should implement a function calledget_term_parent_list_html()
.
Good point about the boolean return there - will generate a WP_Error in that case. Will fix up the other incorrect category references too - thanks for pointing that out :)
In terms of generating an array instead of the markup - that would be solid I think. For this patch I was simply improving the existing function, but you're right it would actually be better to have the functionality more split like that - I guess that's in the scope of this patch too, so will work on that, but will fix the other issues in the mean time first.
#8
@
10 years ago
I've modified the solution to include two new functions instead - both included in taxonomy.php
:
get_term_parents()
- returns an array of parent term IDsget_term_parents_html()
- return an HTML string of term parents in hierarchical order
get_category_parents()
is now a wrapper for get_term_parents_html()
, which in turn calls get_term_parents()
to fetch the array from which to generate the HTML. This means that everything is still backwards compatible.
I named get_term_parents()
as such in order to mimic the naming convention already in place with get_term_children()
.
#9
@
10 years ago
- Summary changed from Allow get_category_parents() to be used for any taxonomy to Add alternative to get_category_parents() that will work for any taxonomy
I've realised my folly of describing the patch in the ticket description - most of the original description is no longer relevant and I can't edit it now :(
#12
follow-up:
↓ 13
@
10 years ago
Thanks for the patches so far.
I understand the motivation behind breaking get_term_parents()
into a separate function from get_term_parents_html()
. But this creates a disparity between get_category_parents()
, which returns a string, and get_term_parents()
, which returns an array of parents.
If we're going to have a general tool for getting ancestors of a term, I'd rather have it be a new parameter for get_terms()
, and it would be more efficient for it to work by fetching the cached hierarchy with _get_term_hierarchy()
instead of crawling backward with a while
loop. (Notwithstanding what get_category_parents()
already does :) )
#13
in reply to:
↑ 12
@
10 years ago
Replying to boonebgorges:
Thanks for the patches so far.
I understand the motivation behind breaking
get_term_parents()
into a separate function fromget_term_parents_html()
. But this creates a disparity betweenget_category_parents()
, which returns a string, andget_term_parents()
, which returns an array of parents.
That's a good point - it's frustrating when there's a disparity like that.
If we're going to have a general tool for getting ancestors of a term, I'd rather have it be a new parameter for
get_terms()
, and it would be more efficient for it to work by fetching the cached hierarchy with_get_term_hierarchy()
instead of crawling backward with awhile
loop. (Notwithstanding whatget_category_parents()
already does :) )
So in that case get_terms()
would be able to return an array of ancestors and then the new get_term_parents()
simply uses that to return an HTML string identical to get_category_parents()
. I think that makes a lot of sense - I'll rework my patch here and upload a new one once it's done.
Patch as described in ticket body.