Remove cruft function_exists() from _field_filter_items() leftover from the (removed) function registry.

History of that line of code:

#517430 by quicksketch on July 13, 2009 at 3:37am

Currently the field_set_empty() function is blindly calling [module]_field_is_empty() without checking that it exists or pulling it in if necessary through drupal_function_exists(). This makes it impossible to put all the field-related hooks in a separate file such as [module].field.inc. This patch wraps the function call in drupal_function_exists().

#517430-7 by bjaspan on July 14, 2009 at 3:32am

Assuming hook_field_is_empty() is intentionally a required function to implement the Field Types API, the ideal behavior given a field type module that does not implement it is for the code to fail to compile. Since we can't get that in Drupal, I agree that fatal error is the next best thing. Thus, the current code is correct.

If we want to declare that hook_field_is_empty() is actually optional because there is a reasonable default behavior, that's fine too. I have no opinion about this particular function, yched is the one qualified to have an opinion on it.

I do note the following points:

1. hook_field_is_empty() does not seem to be documented in field.api.inc or http://api.drupal.org/api/group/field_types/7.
2. The Field Type API docs should specify explicitly which hooks are required and which are not.

#516138-74 by quicksketch on August 11, 2009 at 12:48am

--- modules/field/field.module 11 Aug 2009 14:59:40 -0000 1.22
+++ modules/field/field.module 12 Aug 2009 23:21:56 -0000
@@ -280,14 +280,15 @@ function field_associate_fields($module)
  */
function field_set_empty($field, $items) {
   // Filter out empty values.
-  $filtered = array();
   $function = $field['module'] . '_field_is_empty';
-  foreach ((array) $items as $delta => $item) {
-    if (!$function($item, $field)) {
-      $filtered[] = $item;
+  if (drupal_function_exists($function)) {
+    foreach ((array) $items as $delta => $item) {
+      if ($function($item, $field)) {
+        unset($items[$delta]);
+      }
     }
   }
-  return $filtered;
+  return $items;
}

/**

#516138-75 by yched on August 11, 2009 at 3:03am

drupal_function_exists() is needed, but I'd rather not fail silently if function doesn't exist. The field module is broken. + we should keep this in #517430: field_set_empty() missing drupal_function_exists()

#516138-94 by yched on August 14, 2009 at 1:22am

@@ -279,15 +305,15 @@
  * TODO D7: poorly named...
  */
function field_set_empty($field, $items) {
-  // Filter out empty values.
-  $filtered = array();
   $function = $field['module'] . '_field_is_empty';
+  // We ensure the function is loaded, but explicitly break if it is missing.
+  drupal_function_exists($function);
   foreach ((array) $items as $delta => $item) {
-    if (!$function($item, $field)) {
-      $filtered[] = $item;
+    if ($function($item, $field)) {
+      unset($items[$delta]);
     }
   }
-  return $filtered;
+  return $items;
}

/**

#516138-134 August 18, 2009 at 3:54am by quicksketch

@@ -279,15 +305,15 @@ function field_associate_fields($module)
  * TODO D7: poorly named...
  */
function field_set_empty($field, $items) {
-  // Filter out empty values.
-  $filtered = array();
   $function = $field['module'] . '_field_is_empty';
+  // We ensure the function is loaded, but explicitly break if it is missing.
+  drupal_function_exists($function);
   foreach ((array) $items as $delta => $item) {
-    if (!$function($item, $field)) {
-      $filtered[] = $item;
+    if ($function($item, $field)) {
+      unset($items[$delta]);
     }
   }
-  return $filtered;
+  return array_values($items);
}

/**

Wed, 19 Aug 2009 13:31:14 +0000 by Angie Byron

#516138 by yched, KarenS, quicksketch, bangpound, et al.: CC-FREAKING-K IN CORE! OH YEAH! :D

Mon, 24 Aug 2009 00:02:05 +0000 by Angie Byron

*** empty log message ***

Mon, 24 Aug 2009 00:10:46 +0000 by Angie Byron

Of all the patches to accidentally commit without a message. :( Rolling back registry rip. Let's try that again.

Mon, 24 Aug 2009 00:14:23 +0000 by Angie Byron

#497118 by chx, catch, pwolanin, JoshuaRogers, and Jacob Singh: Remove the function registry. While the hope was that this would result in improved performance for low-end hosts, it comes at the expense of critical development experience problems and less benefit than something like APC. Class registry remains intact to facilitate autoloading.

If it's required, is it documented so?

Thanks! Committed and pushed to 8.x, moving back to 7.x.

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