taxonomy_get_tree().

In the foreach loop the following statement is being executed during every run. It should be placed right after $max_depth is being calculated.
if ($max_depth > $depth) {

CommentFileSizeAuthor
#7 taxonomy_get_tree-D6.patch1.49 KBandypost
#4 taxonomy_get_tree.patch1.48 KBJeremyFrench
#2 taxonomy_get_tree.patch1.48 KBJeremyFrench
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Xano’s picture

Xano
he/they
Primary language English
Location Southampton
CreditAttribution: Xano commented

Kick.

JeremyFrench’s picture

JeremyFrench CreditAttribution: JeremyFrench commented
Assigned: Unassigned » JeremyFrench
Status: Active » Needs review
FileSize
taxonomy_get_tree.patch1.48 KB

Changed this and also put the two ifs outside the loop into one patch uploaded

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented
Status: Needs review » Needs work

Oh my, yes! ;)

Thanks for the patch.

A minor point:

+  if ($max_depth > $depth and !empty($children[$vid][$parent])) {

By convention, we use && instead of and, and we encourage "to always use curly braces even in situations where they are technically optional, as it increases readability and decreases the likelihood of logic errors".

JeremyFrench’s picture

JeremyFrench CreditAttribution: JeremyFrench commented
Status: Needs work » Needs review
FileSize
taxonomy_get_tree.patch1.48 KB

I have changed the "and" to "&&" but I am not sure where I have missed out a curly brace, should I split the if into to statements?

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs review » Reviewed & tested by the community

Let's get this one in.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Version: 7.x-dev » 6.x-dev
Status: Fixed » Patch (to be ported)
FileSize
taxonomy_get_tree-D6.patch1.49 KB

Suppose this one should be ported

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Issue tags: +Quick fix

Just a tag

brianV’s picture

brianV CreditAttribution: brianV commented
Status: Patch (to be ported) » Reviewed & tested by the community

Port looks good. Setting to RTBC to be committed.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Reviewed & tested by the community » Fixed

Committed to Drupal 6, thanks.

Status: Fixed » Closed (fixed)
Issue tags: -Quick fix

Automatically closed -- issue fixed for 2 weeks with no activity.

Issue tags: +Quick fix

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.