Feedback on 7.x-2.x

Sorry, I have just gotten back to the project that is running with this.

Something in the last 3 months has broken the 1.x code, but I haven't investigated and have gone straight to 2.x to push this forward.

/**
 * Implements hook_field_formatter_info_alter().
 */
function empty_fields_field_formatter_info_alter(&$info) {
  foreach ($info as $formatter_key => &$formatter) {
    $formatter['settings']['empty_fields_handler'] = '';
    foreach (empty_fields_plugins() as $class_name => $title) {
      $plugin = new $class_name();
      foreach ($plugin->form() as $field_id => $default) {
        $field_name = empty_fields_generate_field_name($field_id, $class_name);
        $formatter['settings'][$field_name] = $default;
      }
    }
  }
}

Doesn't this mean to be $plugin->defaults()?

The declaration refers to the old name.

- * Implments hook_empty_fields_info().
+ * Implements hook_empty_fields().

And though it doesn't matter in PHP, there is a case mis-match in the class name in this hook.

OK, more.

From the top.

1) -11,10 +11,12
I have added a class_exists() in empty_fields_field_formatter_info_alter() as an error here prevents the registry flush from completing, even after errors in the code have been fixed.

2) -61,11 +63,11
Case / description change

3) -96,6 +98,9
Always return summary text

4) -118,6 +123
Most handlers probably do not have any settings.

5) -144,6 +149,13
Many entities were being processed multiple times. This tries to negate this.

6) -163,6 +175,11
We need context to the actual field that we are handling here!

7) -193,27 +210,30
Provides a way for a Handler to bale out if it decides that there is actually nothing to return

+      // Do not render empty values.
+      if (!empty($markup) || (is_scalar($markup) && drupal_strlen($markup))) {

Just realised that we are assuming a string here, $plugin->alterOutput(), is available if something else is required.

8) EmptyFieldHandler
Many description changes, but slight type hinting changes and concrete methods as only the two are required imho.

I got a fatal error on the live multi-site installation when deployed, so had to implement the quickest fix to normalise all of the method signatures. Looking at the patch, maybe type hints are better?

-  public function form(&$form_state = NULL) {
+  public function form(&$form_state = array()) {

to

-  public function form(&$form_state = NULL) {
+  public function form(array &$form_state) {

etc.

9) Readme
This was fully out of date

Marked #1797494: No options if the field lacks display formatter settings as duplicate

From the testing code review - using core drupal string functions.

From point from 7, changed code to always assume a scaler value.

In this branch, when setting up display format, there is no choice for " Use field by default (if set) " as it was with the 7.x-1.x branch.

I think that RaulMuroc may be having the same issue as I had with Better Formats or another module that saves an empty value

OR

The default value was set after content was created / the field was added after content was created.

And these are never used.

abstract class EmptyFieldHandler {
-  public $title;
-  public $description;
  public $options;

This patch includes the comment from #1 [EmptyFieldHandler::form() to EmptyFieldHandler::defaults()] and removes $form_state from EmptyFieldText::form(). This argument is not passed in and is used incorrectly and appears redundant afaick.

Whats up in empty_fields_load_plugin()? Did you mean this?

-    foreach ($plugin->defaults() as $field_id => $field) {
+    foreach ($plugin->defaults() as $field_id => $default) {
      $field_name = empty_fields_generate_field_name($field_id, $plugin_type);
      $plugin->options[$field_id] = !empty($view_mode_settings[$field_name]) ? $view_mode_settings[$field_name] : $default;
    }

[edit]
Let me know if the changes are OK and I'll push these. With the exception of the above, I think we are ready for a rc1 or full release on the 2.x branch. :)

K, finally got back to this. Pushed it through with a couple coder review errors.

Changed API may require a notice

And a minor follow-up