Generalize API and Integrate with core field types

Are there plans to make this generic? Of the 75 contrib modules we have installed in a current project, none actually implement hook_diff()!

So before contrib gets started, have you considered making this much more generic?

A compare entity hook could compare entities using ENTITY_diff() and loop through the fields after calling this to generate any field diffs. This idea is shown below, (but node_hook() still does the second part), using a generic fallback field diff value callback.

/**
 * Implements hook_diff() for node.module.
 */
function node_diff($old_node, $new_node) {

  $result = array();
  $type = node_type_get_type($new_node);
  $result['title'] = array(
    '#name' => $type->title_label,
    '#old' => array($old_node->title),
    '#new' => array($new_node->title),
    '#weight' => -5,
    '#format' => array(
      'show_header' => FALSE,
    )
  );

  $instances = field_info_instances('node', field_extract_bundle('node', $type));
  foreach ($instances as $instance) {
    $field_name = $instance['field_name'];
    $field = field_info_field($field_name);
    $langcode = field_language('node', $new_node, $field_name);

    $func = $field['module'] . '_field_diff_view';
    if (!function_exists($func)) {
      $func = 'diff_field_diff_view';
    }

    $old_values = array();
    if (!empty($old_node->{$field_name}[$langcode])) {
      $old_items = $old_node->{$field_name}[$langcode];
      $old_values = $func('node', $old_node, $field, $instance, $langcode, $old_items);
    }

    $new_values = array();
    if (!empty($new_node->{$field_name}[$langcode])) {
      $new_items = $new_node->{$field_name}[$langcode];
      $new_values = $func('node', $new_node, $field, $instance, $langcode, $new_items);
    }

    $max = max(array(count($old_values), count($new_values)));
    for ($delta = 0; $delta < $max; $delta++) {
      $result["{$field_name}_{$delta}"] = array(
        '#name' => $instance['label'],
        '#old' => isset($old_values[$delta]) ? $old_values[$delta] : '',
        '#new' => isset($new_values[$delta]) ? $new_values[$delta] : '',
      );
    }
  }
  return $result;
}

/**
 * Implements hook_field_diff_view().
 *
 * An generic fallback to support any field that provides a value column. This
 * is not a real hook, rather than a default callback for field modules that do
 * not provide an implementation of hook_field_diff_view().
 *
 * @param $entity_type
 *   The type of $entity.
 * @param $entity
 *   The entity being displayed.
 * @param $field
 *   The field structure.
 * @param $instance
 *   The field instance.
 * @param $langcode
 *   The language associated with $items.
 * @param $items
 *   Array of values for this field.
 *
 * @return
 *   An array of rendered string values for the $items, keyed by numeric indexes
 *   starting from 0.
 */
function diff_field_diff_view($entity_type, $entity, $field, $instance, $langcode, $items) {
  $diff_items = array();
  foreach ($items as $delta => $item) {
    if (isset($item['value'])) {
      if (is_scalar($item['value'])) {
        $diff_items[$delta] =  (string)$item['value'];
      }
      else {
        $diff_items[$delta] =  implode("\n", $item['value']);
      }
    }
  }
  return $diff_items;
}

So contrib can very easily extend this out for the fields that they handle.

For example, the Name Field module:

/**
 * Implements hook_field_diff_view().
 */
function name_field_diff_view($entity_type, $entity, $field, $instance, $langcode, $items) {
  $diff_items = array();
  foreach ($items as $delta => $item) {
    unset($item['safe']);
    $diff_items[$delta] = implode("~", $item);
  }
  return $diff_items;
}

Any plans or interest for this?

Subscribing!

It's a pity diff doesn't work for fields :(

Somewhat related: #248778: Taxonomy diff

OK, refactored patch.

I've got some spare time (sort of) to try and push the field related issues.

I am setting up a test environment that has one of every core field types. Once that is done, it should be a fairly quick job implementing all of the core fields.

OK the results of the initial tests of the core fields.

The field set up

And the results from NULL (or 0 for single boolean checkbox) to a value

The new patch adds the loading mechanism for field modules.

In includes/list.inc, (not in patch), lists are functional using:

function list_field_diff_view($items, $context) {
  $diff_items = array();
  $allowed_values = list_allowed_values($context['field'], $context['instance'], $context['entity_type'], $context['entity']);
  foreach ($items as $delta => $item) {
    if (isset($allowed_values[$item['value']])) {
      $diff_items[$delta] = $allowed_values[$item['value']];
    }
    else {
      // If no match was found in allowed values, fall back to the key.
      $diff_items[$delta] = $item['value'];
    }
  }

  return $diff_items;
}

Since a field admin is unlikely to have duplicate list labels, I have excluded the list keys.

I knew these were going to be fast.

Term references

function taxonomy_field_diff_view($items, $context) {
  $diff_items = array();
  foreach ($items as $delta => $item) {
    if (!empty($item['tid'])) {
      if ($term = taxonomy_term_load($item['tid'])) {
        $diff_items[$delta] = $term->name . ' [tid:' . $term->tid . ']';
      }
      else {
        $diff_items[$delta] = t('Invalid term reference');
      }
    }
  }
  return $diff_items;
}

Better, bulk load in the custom prepare hook:

function taxonomy_field_diff_view_prepare(&$old_items, &$new_items, $context) {
  $tids = array();
  foreach (array_merge_recursive($old_items, $new_items) as $info) {
    $tids[$info['tid']] = $info['tid'];
  }
  $terms = taxonomy_term_load_multiple($tids);
  foreach ($old_items as $delta => $info) {
    $old_items[$delta]['term'] = isset($terms[$info['tid']]) ? $terms[$info['tid']] : NULL;
  }
  foreach ($new_items as $delta => $info) {
    $new_items[$delta]['term'] = isset($terms[$info['tid']]) ? $terms[$info['tid']] : NULL;
  }
}

function taxonomy_field_diff_view($items, $context) {
  $diff_items = array();
  foreach ($items as $delta => $item) {
    if (empty($item['term'])) {
      $diff_items[$delta] = t('Invalid term reference');
    }
    else {
      $diff_items[$delta] = $item['term']->name . ' [tid:' . $item['term']->tid . ']';
    }
  }
  return $diff_items;
}

I've pumping out all of the components of the text field items passed through check_markup(). While this accurately displays the differences of the final rendering, it could make it difficult to see the real changes within the content. Also, some people may get confused with the Text format display.

Maybe we need to look at making this more configurable?

Full rendered output example (body field)

It is late, so File / Image will have to wait till tomorrow.

This is biggish patch, testers needed.

11 files changed, 620 insertions(+), 99 deletions(-)

You will need to run update.php (note: there are no updates) or flush your caches to register the new hooks.

It depreciates hook_diff() and introduces hook_entity_diff().

Very minimal changes related to this.

Implements hook_hook_info()

For cleaner handling of contrib. code, allow maintainers to use "MODULE_NAME.diff.inc" for the hooks.

Complete core field coverage

From the various threads in the queues, it adds support to all core field types, namely list, file, image, taxonomy.

Initial steps for a generic entity solution

If you have devel installed, you can see this in action using the Execute PHP block / page:

# Assumes users have at least 1 field
$a = user_load(171);
$b = user_load(1);
$user_diffs = module_invoke_all('entity_diff', $a, $b, 'user');
dpm($user_diffs);

Some typo fixes in the comments.

Wow, interesting!

Would love to test this, but it doesn't apply to (git) 7.x-2.x or 7.x-2.0. The patch also says, Subject: [PATCH 1/4] =Issue 1365750 by Alan D., et al: Incomplete field handling and no entity support. So, I'm really confused. Are there 3 more patches to apply? What's the order?

I'm probably overlooking something. Could you please make a reroll or give instructions?

Thanks in advance.

It should apply as a singular patch (#13), it contains 4 individual commits to my local version using "git format-patch".

I don't often apply via the command line, try "patch -p1". I usually use Eclipse, and that is Team > Apply Patch and select p1. I'm actually wondering if I should sandbox these ideas to speed up the development and tracking, I will see how things go this weekend...

Sorry, I'm not a dev, and hence not so familiar with git messages. But I did try both git apply and patch -p1, so a sandbox would be terrific; or if you only have a few min, then post a tar file here for now.

Lazy approach, a zip.

I'm just starting to play with the field diff options, so when you edit the content type, you get two new options in the diff tab:

a) Preview view mode
This sets the view mode when displaying the current revision - or to hide this completely.

b) Enable per instance diff settings for this content type
This does nothing at the moment :)

It will enable configurable diff settings on individual fields to control field comparison and rendering options. i.e. to strip HTML from fields etc. So once checked, every field instance (the top part of the field edit page) will get diff options. If unchecked (the default), it will default to the standard settings provided currently.

It seems to work in the ways described, except unlike #6, my field name didn't show above the the value changed. Perhaps it's because I added the field after I already had a couple of revisions.

Will do more extensive reviews too soon. Could you please provide a testing rubric and perhaps default settings/content that shows it without me needing to much to see it all running on my dev?

The module needs a cache flush that is not built in. After that, all fields should show up in the comparison.

#17: these sound like great improvements.

#18: you can pretty much disregard that. This isn't a module with complex configuration tasks, more like flip a switch and *bam, awesome functionality.. Very different than the other stuff I've been testing lately.

#19: The problem was that the field name I used was too short to recognize and that the field name text above the diff line is a bit smaller than I had expected.

Usability folk: What are your thoughts on that screenshot in #11? Are there any easy / obvious / big win changes that can make this more user friendly? Unfortunately it isn't using a standard theme.. but #17 works if you want do dive in.

One particular question I have is, does anyone know of examples of diffs on texts with fields? I don't know any other examples off hand, just mediawiki and code diffs. I would suspect ruby, django, plone, etc might have something similar.

Thank you so much for this awesomeness, Alan D.!!

Sorry for jumping all over the place, but the task I'm doing affects so multiple issues...

In relation to #17, I threw up some ideas in #1418760: Optional setting to honour the display settings, but following #1347316: Selectable view mode for inline diffs and "Current revision" display view mode. the view mode method appears to be the best option. This zip contains initial work going in that direction, the preview and inline block should be fully functional, but all the standard functionality does is to follow the hidden or visible flags of the view mode diff_standard. It defaults to ignoring this and to compare all fields.

I'll be having a look at ways to cleanly integrate things like #372957: HTML Strip for Diff, WYSIWYG Friendly soon. While that works, a JQuery toggle of this on the main diff page would probably be much more user friendly :)

OK, re-roll of the zip, this one introduces an update script to ensure that the selected view mode in the standard preview doesn't change when doing the update. I have changed the variable names from "diff_view_mode_preview_" to "diff_view_mode_preview_node_" so to follow the pattern "diff_view_mode_preview_ENTITY-TYPE_BUNDLE-NAME"

Also has a resolution for:
#1280892: Diff should track the variables that it defines
#1304658: Remove "Add new comment" section from revisions view (But I haven't checked the inline block)

Disclaimer, contains code written at 2.30am and is a work in progress...

Attached is a git diff between 7.x-1.x and #21. If there aren't effective reviews coming for the patch as it stands, then let's try to break it up and split it between the mentioned issues.

Getting all this code reviewed is also a big task, so I issued project_issue an assistance request, see #1673248: [Meta] Change audit for retained dependencies in major version upgrade.

Removed usability request, since that'd be better in a different issue.

Use the 7.x-2.x branch. There are far too many changes between the branches to effectively compare what is already a massive patch :)

Oh

lol, you have to love windows mac and unix for not deciding on the same line ending...

I can convert these to \n only and post a patch tonight

Clean diff patch

Ah! Much better.

Alan D. could you please update this now that #372957 is commited? I won't apply any more patches until we can get this in.

This is big solo effect at the moment, and I do not think that it right for committing into dev just yet, but I will re-roll soon. I've parsed all of the D7 issues, and I think that we have nailed a lot of these if I get this right, along with the other couple of commits just sent through.

My plans were to race into the display settings to see how far I can take it using the view modes, this is still a bit of an unknown in terms of both functionality and usability. If all went to plan, I was going to break this back into independent patches with dependencies on other patches and to introduce tests, but a single patch is by far easier (albeit riskier).

And is it worth trying to get someone from the core Drupal maintainers team to review first? This is an important module on drupal.org.

In relation to #372957: HTML Strip for Diff, WYSIWYG Friendly, did anyone try to implement an inline version of this? i.e. rather than another page, both forms, with HTML, without HTML diffs would be rendered into the same area and a toggle implemented to switch between the different views. I was going to play with this idea and to potentially include the rendered content too. Thinking something like:

Clicking the letters on the left would toggle the display to show the alternative view.

Here is a track of the minor patches / features. Once the selected ones of these are through, I'll roll the grand-daddy patch :)

Diff 7.x-2.x
o #888680 by Deciphered, Alan D.: Allow modules to interact via drupal_alter()
o #1280892 by Alan D., crea: Diff should track the variables that it defines
o #1304658 by Alan D.: Remove "Add new comment" section from revisions view
o #1122206 by RdeBoer, binford2k, Alan D.: Notices are thrown by DiffEngine::process_chunk()
o #1673856 by Alan D.: Use hook_form_BASE_FORM_ID_alter() rather than hook_form_alter()
o #1175064 by zilverdistel, Alan D.: Provide variables for leading and trailing context
o #1673864 by Alan D.: Allow users to skip the administration theme when viewing revisions
o #1673876 by Alan D.: Use core class autoloading rather than manually including the classes.

#30: Maybe create a sandbox with these commits applied separately? That way, there'd be a log of each one, and realityloop could still make a big merge with everyone still able to track the changes down to each issue.

If you're more so interested in continuing to develop features, then perhaps someone else could assist you with just doing the traceback and split for now. I wouldn't want to ask you to hold up from whatever you find more beneficial and interesting. I'm also trying to reach out the the d.o maintainers to see if they can help with these tasks.

#29: That looks like it'll be a very cool interface, but I'm not sure what to review yet. Since, the other issue already went through and is pretty old, could you open a new one? I think the usability team would also have an interest in that issue.

Cheers!

I will try and track things though #1630062: Developmental Roadmap that will coordinate the sandbox Diff developmental version and main project. The hard yards have been done now to break this out into an independent developmental project, but this is more error prone jumping between the two. But I'll see how it goes.

lol, "beneficial and interesting", would be implementing a plugin engine and enhanced UI. These would need to wait till the core project is refactored and a plugin system built for this!

Love some feedback on the UI for treating Text fields with either Text processing or Summaries enabled.

The settings that I'm considering are to optionally include summary and text format.

A long text field (Format drops to a new line at the end)

Default

All options (display summary and format)

For a field, Long text with summary:

Default

All options (display summary and format)

This patch has been created from about 8 commits ahead of Diff 2.x branch, but has the main workings of the change. Refer to the Sandbox for an accurate diif between the two.

How do you handle diff on multiple fields when values are reordered ?
How do you handle the particular case of a taxonomy term field configured to input tags ? In this case, the delta is determined by the core and may not be a valuable information for the user making a diff.

How do you handle diff on multiple fields when values are reordered ?

In the refactored version, the ordering is based on the delta value.

How do you handle the particular case of a taxonomy term field configured to input tags ?

Order is important for some users...

  // For the options form.
  $options_form['sort'] = array(
    '#type' => 'radios',
    '#title' => t('Sort'),
    '#options' => array(
      '0' => t('No sort'),
      '1' => t('Sort'),
      '-1' => t('Only sort free tagging fields'), # default
    ),
    '#default_value' => $settings['sort'],
  );

  // For the diff
  if (!empty($settings['sort']) && !empty($diff_items)) {
    if ($settings['sort'] == 1 || $instance['widget']['type'] == 'taxonomy_autocomplete') {
      usort($diff_items, 'uasort_taxonomy_field_diff_terms');
    }
  }

I've implemented a "Missing term" for the term diff, so the sort is a bit more complex:

function uasort_taxonomy_field_diff_terms($a, $b) {
  // We need to use t() to test for string overrides.
  $missing_text = t('Missing term reference');
  $a_missing = strpos($a, $missing_text) === 0;
  $b_missing = strpos($b, $missing_text) === 0;
  if ($a_missing && $b_missing) {
    return strnatcmp($a, $b);
  }
  elseif ($a_missing xor $b_missing) {
    return $a_missing ? 100 : -100;
  }
  return strnatcmp($a, $b);
}

I don't understand your answer so I will rephrase.

1/ How do you handle diff on multiple fields when values are reordered ?
What does diff will show if the only differences between two revisions is the order of two elements in a multiple field ?

Example :
Field X revision A
- value 1
- value 2
Field X revision B
- value 2
- value 1

Will diff show that the two items of the field X has been edited, or will it show none has been or will the user have the choice betwwen these two modes (and how ?)

2/ How do you handle the particular case of a taxonomy term field configured to input tags ?
By definition, order is never important in the case of tags so if two tags are in a différent order between two revisions I think diff may just ignore them. What do you [plan to] provide for this ?

3/ Thank you for your hard work :)

1/. If sorting is enabled, no differences. If sorting is not enabled, then it would probably show every term as different

a, b, c to c, b, a

With sorting, the comparison is
a = a
b = b
c = c

So no differences, and it will not show.

Without sorting
a != c
b = b
c != c

So the display is something like

-a +c
b b
-c +a

2/. Options
The more I do here, the more likely that this will become a new branch...

The comparison page uses either the standard diff comparison of raw fields OR a configurable view mode called "Diff standard". For this view mode, you can control which fields to show or hide. If the module Field formatter options is enabled, you can configure additional settings per content type in the display settings (this is painful but highly customizable). The screen shots show this in action.

By default, the globals are used.

But otherwise:

This has been completed.

I have configured it to use custom sorting for tagging, so that the tags are sorted, but the terms are not. However, most term widgets have a fixed order in the UI so these too are effectively sorted :)

I'm working on an administration page to also handle global defaults that are set using system variables.

The UI will be similar / same form under the "Enable custom Diff settings" in the screenshots above.

[Edit] I just saw the UI bug above. It should read "Sort if free tagging field"!

Good job !
The idea to use a dedicated view mode is really great.
Thank you for your answers.

This will be a new branch. There is so many changes, bug fixes, et al.

Right, the reason why I started this mission. +/- 20min coding has given me two random nodes in a diff. Click through to see the full page diff.

This is going into an independent project that will initially look at comparing any two specified entities. I think I can make this full generic using the Entity API.

The only node specific code below is the Title and dates, the rest is driven off the diff entity comparison api.

The styling is from wikimedia - will need to check issues with including that. The module allows you to optionally load the ugly classic css, this one or none.

Good !

Last question. :)
Did you manage to integrate with Entity API ?

A lot of modules use hook_entity_property_info() to define their entities' metadata which are really close to fields. This seems to be planned to be included in D8 so Core entities will certainly use it in the future.

At the moment there is no non-node UI planned, purely node support. However the API is open and I''m creating a new sandbox for generic comparisons based on this API. There is really no reason why you couldn't do User to Term comparisons. Both have label fields, both are fieldable. (But why you would want to? Who knows!)

I want to try and abstract out as much as possible, without adding any dependencies on Entity API to Diff itself.

So I've refactored 25% of the module, and another 15% (to remove the node specific functionality from core templates) to go before trying to get the maintainers to open a new branch.

This should be resolved with the new branch, 7.x-3.0-alpha1 or 7.x-3.0-dev.