As discussed in #422362: convert form.inc to use new static caching API with pwolanin and chx, let's open a separate issue to deal with the $reset parameter in form_options_flatten().

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JamesAn’s picture

Thinking about it some more, wouldn't it pretty straight-forward to remove the $reset param using a helper function?

Original code:

function form_options_flatten($array, $reset = TRUE) {
  static $return;

  if ($reset) {
    $return = array();
  }

  foreach ($array as $key => $value) {
    if (is_object($value)) {
      form_options_flatten($value->option, FALSE);
    }
    else if (is_array($value)) {
      form_options_flatten($value, FALSE);
    }
    else {
      $return[$key] = 1;
    }
  }

  return $return;
}

Modified code:

function form_options_flatten($array) {
  // Always reset static var when first entering the recursion
  drupal_static_reset('_form_options_flatten');
  return _form_options_flatten($array);
}

// New helper function
function _form_options_flatten($array) {
  $return = &drupal_static(__FUNCTION__);

  foreach ($array as $key => $value) {
    if (is_object($value)) {
      _form_options_flatten($value->option);
    }
    else if (is_array($value)) {
      _form_options_flatten($value);
    }
    else {
      $return[$key] = 1;
    }
  }

  return $return;
}

We'd need some documentation to make this obvious in plain language.

JamesAn’s picture

Status: Active » Needs review
FileSize
1.09 KB

My webdev box runs out of memory while running the tests, but everything passes so far. So here it goes.

Dries’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me!

Status: Reviewed & tested by the community » Needs work

The last submitted patch failed testing.

JamesAn’s picture

Status: Needs work » Needs review
FileSize
1.22 KB

Rerolled patch against the comment added in #422362: convert form.inc to use new static caching API.

catch’s picture

+  // Always reset static var when first entering the recursion

missing period.

JamesAn’s picture

FileSize
1.22 KB

Thanks for catching that!

Status: Needs review » Needs work

The last submitted patch failed testing.

lilou’s picture

Status: Needs work » Needs review

retest.

Status: Needs review » Needs work

The last submitted patch failed testing.

JamesAn’s picture

Status: Needs work » Needs review
FileSize
1.22 KB

Rerolled.

Re-test of 437018-11.patch from comment #11 was requested by Arancaytar.

cburschka’s picture

Status: Needs review » Reviewed & tested by the community

Pending a passing test with the new HEAD, this patch looks good.

cburschka’s picture

[Duplicate post, sorry.]

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

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