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.

@#3 by chx:

If it's required, is it documented so?

Apparently not, but that is a separate issue.

In fact, none of the hook implementations listed on the Field Types API page state whether they're required or optional.

As noted by bjaspan in #517430-7 (see history above):

The Field Type API docs should specify explicitly which hooks are required and which are not.

This has not yet happened, and probably should be opened as a two-year-old documentation bug.