Field formatters as plugins

Two minor things in there

+++ b/core/modules/field/modules/text/lib/Drupal/text/Plugin/field/formatter/TextTrimmedFormatter.php
@@ -0,0 +1,82 @@
+ * Note: This class also contains the implementations usedby the

typo: used by

+++ b/core/modules/field/modules/text/lib/Drupal/text/Plugin/field/formatter/TextTrimmedFormatter.php
@@ -0,0 +1,82 @@
+   * Implements FormatterInterface::viewElements().

Possible inconsistence in the comments, in other formatters the whole class name is informed, e.g. Implements Drupal\field\Plugin\Type\Formatter\FormatterInterface::viewElements()

This looks great, here are some code style nitpicks that aren't urgent, but could be kept in mind going forward:

+++ b/core/modules/field/field.attach.incundefined
@@ -197,6 +197,145 @@ function field_invoke_method($method, Closure $target_closure, EntityInterface $
+ * @param $method
+ *   The name of the method to invoke.
+ * @param $target
+ *   A closure that receives an $instance object and returns the object on

Might as well add the type hints here and for newly added functions/methods

+++ b/core/modules/field/field.attach.incundefined
@@ -197,6 +197,145 @@ function field_invoke_method($method, Closure $target_closure, EntityInterface $
+ *   An array of entities, keyed by entity id.

ID, not id. (Here and other places)

+++ b/core/modules/field/field.attach.incundefined
@@ -197,6 +197,145 @@ function field_invoke_method($method, Closure $target_closure, EntityInterface $
+ * @param $a
+ *   A parameter for the invoked method.
+ * @param $b
+ *   A parameter for the invoked method.
+ * @param $options

These should mention their default values.

+++ b/core/modules/field/field.attach.incundefined
@@ -197,6 +197,145 @@ function field_invoke_method($method, Closure $target_closure, EntityInterface $
+ *  - 'field_name': The name of the field whose operation should be invoked. By
+ *    default, the operation is invoked on all the fields in the entity's
+ *    bundle. NOTE: This option is not compatible with the 'deleted' option; the
+ *    'field_id' option should be used instead.
+ *  - 'field_id': The id of the field whose operation should be invoked. By

The hyphens should be indented one more space, and there shouldn't be quotes around the key names

+++ b/core/modules/field/field.attach.incundefined
@@ -197,6 +197,145 @@ function field_invoke_method($method, Closure $target_closure, EntityInterface $
+function field_invoke_method_multiple($method, Closure $target, $entities, &$a = NULL, &$b = NULL, $options = array()) {

As I understood it, this either had to be use'd at the top of the file, or have an inline leading \ per #1614186: Coding standards for using "native" PHP classes like stdClass in namespaced code

+++ b/core/modules/field/modules/text/lib/Drupal/text/Plugin/field/formatter/TextDefaultFormatter.phpundefined
@@ -0,0 +1,44 @@
+    return $elements;
+  }
+}
diff --git a/core/modules/field/modules/text/lib/Drupal/text/Plugin/field/formatter/TextPlainFormatter.php b/core/modules/field/modules/text/lib/Drupal/text/Plugin/field/formatter/TextPlainFormatter.php

diff --git a/core/modules/field/modules/text/lib/Drupal/text/Plugin/field/formatter/TextPlainFormatter.php b/core/modules/field/modules/text/lib/Drupal/text/Plugin/field/formatter/TextPlainFormatter.php
new file mode 100644

Should have a blank line after the method (Here and other places)

If this patch is re-rolled, here are some additional documentation notes. Otherwise, the enhanced docblocks are much better than before!!

+++ b/core/modules/field/field.info.incundefined
@@ -496,52 +366,42 @@ function field_info_field_types($field_type = NULL) {
+ * @return array
+ *   Either a single formatter type description, as provided by class annotations,
+ *   or an array of all existing formatter types, keyed by formatter type name.
  */

This comment needs to reformated to wrap at or before column 80.

+++ b/core/modules/field/lib/Drupal/field/FieldInstance.phpundefined
@@ -0,0 +1,226 @@
+
+  public $definition;

Doesn't this also require a docblock?

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Formatter/FormatterInterface.phpundefined
@@ -0,0 +1,104 @@
+   * @param Drupal\Core\Entity\EntityInterface $entity
+   *   A single object of type $entity_type.

Not sure why $entity_type is referenced here... Is this correct?

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Formatter/FormatterPluginManager.phpundefined
@@ -0,0 +1,88 @@
+
+  protected $defaults = array(
+    'settings' => array(),
+  );
+
+  protected $cache_bin = 'field';
+  protected $cache_key = 'field_formatter_types';

All of these need docblocks explaining the variable type and type hint.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/LegacyDiscoveryDecorator.phpundefined
@@ -0,0 +1,67 @@
+
+  protected $hook;

Needs a docblock.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetBase.phpundefined
@@ -0,0 +1,447 @@
+  /**
+   * Constructs a WidgetBase object.
+   *

Other constructor methods in this patch do not have docblocks. I am unsure on what the documentation policy is, but we should be consistent throughout this patch.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetBase.phpundefined
@@ -0,0 +1,447 @@
+  /**
+   * Special handling to create form elements for multiple values.
+   *
+   * Handles generic features for multiple fields:
+   * - number of widgets
+   * - AHAH-'add more' button
+   * - table display and drag-n-drop value reordering
+   */
+  protected function formMultipleElements(EntityInterface $entity, array $items, $langcode, array &$form, array &$form_state) {

I believe all of these variables need @param directives with type hinting and explanations. Also needs @return for $elements variable.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetBase.phpundefined
@@ -0,0 +1,447 @@
+  protected function formSingleElement(EntityInterface $entity, array $items, $delta, $langcode, array $element, array &$form, array &$form_state) {

This needs @param and @return directives.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetLegacyDiscoveryDecorator.phpundefined
@@ -0,0 +1,38 @@
+
+  protected $hook = 'field_widget_info';
+
+  public function processDefinition(array &$definition) {

Missing docblocks....

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Widget/WidgetPluginManager.phpundefined
@@ -0,0 +1,90 @@
+  protected $defaults = array(
+    'settings' => array(),
+    'multiple_values' => FALSE,
+  );
+
+  protected $cache_bin = 'field';
+  protected $cache_key = 'field_widget_types';
+  protected $hook = 'field_widget_info';

Missing docblocks...

@yched... I am not sure what happened in #10, but patch is zero bytes.

@yched: I am not sure if I am in the right issue, but here is my review of your patch in #13.

+++ b/core/modules/field/field.api.phpundefined
@@ -2113,27 +1919,27 @@ function hook_field_info_max_weight($entity_type, $bundle, $context) {
+ * @param array $context
  *   An associative array containing:
  *   - entity_type: The entity type; e.g., 'node' or 'user'.
+ *   - bundle: The bundle: e.g., 'page' or 'article'.
  *   - field: The field being rendered.
  *   - instance: The instance being rendered.
- *   - entity: The entity being rendered.
  *   - view_mode: The view mode, e.g. 'full', 'teaser'...

I know that it is not part of our coding standards, but it would be super helpful as a programmer to know what was expected for each of these array elements.

Does it make sense to do something like - entity_type: (string) The entity type ...

Based on the current documentation, I am unsure of what is expected for 'instance' key.

+++ b/core/modules/field/field.attach.incundefined
@@ -197,6 +197,135 @@ function field_invoke_method($method, \Closure $target_closure, EntityInterface
+ *   An array of entities, keyed by entity ID.
+ * @param mixed $a
+ *   A parameter for the invoked method. Defaults to NULL.
+ * @param mixed $b
+ *   A parameter for the invoked method. Defaults to NULL.

Both of these declarations should start with '(optional)'. Actually same goes for the $options variable as well.

+++ b/core/modules/field/field.attach.incundefined
@@ -197,6 +197,135 @@ function field_invoke_method($method, \Closure $target_closure, EntityInterface
+ * @return
+ *   An array of returned values keyed by entity id.

Can we add a type hint here for consistency?

+++ b/core/modules/field/field.attach.incundefined
@@ -1187,7 +1336,8 @@ function field_attach_prepare_view($entity_type, $entities, $view_mode, $langcod
+  // @todo not sure about that...

Can you expand upon this comment? Very unclear what you mean...

+++ b/core/modules/field/field.info.incundefined
@@ -49,22 +49,10 @@ function field_info_cache_clear() {
- *     module, giving the module that exposes the formatter type.
  *   - 'storage types': Array of hook_field_storage_info() results, keyed by

I am pretty sure that keys of an array should not be surrounded by single quotes.

+++ b/core/modules/field/field.info.incundefined
@@ -471,25 +388,21 @@ function field_info_widget_types($widget_type = NULL) {
- * @param $formatter_type
+ * @param string $formatter_type
  *   (optional) A formatter type name. If omitted, all formatter types will be

This should reference what the default result is.

+++ b/core/modules/field/lib/Drupal/field/FieldInstance.phpundefined
@@ -27,6 +27,13 @@ class FieldInstance implements \ArrayAccess {
   /**
+   * The formatter objects used for this instance, keyed by view mode..

Small nit... ends in double periods, should be a single period.

+++ b/core/modules/field/lib/Drupal/field/FieldInstance.phpundefined
@@ -68,6 +75,89 @@ class FieldInstance implements \ArrayAccess {
+   *     @todo : from_field_view_field() : "If no display settings are found for
+   *     the view mode, the settings for the 'default' view mode will be used." ??
+   *   - An array of display properties, as accepted by setDisplayConfig().

This should be rewrapped for 80 character maximum.

+++ b/core/modules/field/lib/Drupal/field/FieldInstance.phpundefined
@@ -68,6 +75,89 @@ class FieldInstance implements \ArrayAccess {
+   *
+   * @return Drupal\field\Plugin\Type\Formatter\FormatterInterface

For consistency, this should be 'FormatterInterface|null'

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Formatter/FormatterBase.phpundefined
@@ -0,0 +1,147 @@
+
+use Drupal\Component\Plugin\Discovery\DiscoveryInterface;
+use Drupal\Core\Entity\EntityInterface;
+use Drupal\field\Plugin\PluginSettingsBase;
+use Drupal\field\FieldInstance;

Not sure, but should not all use declarations be in alphabetical sort order?

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Formatter/FormatterInterface.phpundefined
@@ -0,0 +1,104 @@
+   * @return string

Silly question from reading code comments... Can this summary string include HTML tags or is it expected to be plain text?

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Formatter/FormatterInterface.phpundefined
@@ -0,0 +1,104 @@
+   *   Array of entities being displayed, keyed by entity ID.
+   * @param string $langcode
+   *   The language the field values are to be shown in. If no language is
+   *   provided the current language is used.

Unclear what is meant by 'If no language is provided'. First, there should be a comma after provided. However, not clear what happens if for instance $langcode is set to NULL in call.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Formatter/FormatterPluginManager.phpundefined
@@ -2,27 +2,27 @@
 use Drupal\Component\Plugin\PluginManagerBase;
 use Drupal\Core\Plugin\Discovery\CacheDecorator;
 use Drupal\Core\Plugin\Discovery\AnnotatedClassDiscovery;
-use Drupal\field\Plugin\Type\LegacyDiscoveryDecorator;
+use Drupal\field\Plugin\Type\Formatter\FormatterLegacyDiscoveryDecorator;
+use Drupal\Component\Plugin\Factory\ReflectionFactory;

Same comment as above about ordering of multiple use statements. Alphabetical?

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/LegacyDiscoveryDecorator.phpundefined
@@ -11,14 +11,19 @@ use Drupal\Component\Plugin\Discovery\DiscoveryInterface;
- *

Should not this @todo remain?

@yched - when you're off again, let us know if there's anything we should focus on while you're gone so we can bring this forward

Note - have you got my mail re: how to arrange the train/flight stuff easily - man that sound stupids to type that here :)

Alright, I'll check the drush scripts I've already written, it already allows to create a content type with a random number of textfields, adding a taxonomy term field option should be easy, I'll try to update in an hour.

Done, added option to generate term fields/instances as well. It uses the tags vocabulary that comes default with core install. makes it easier to setup scenarios faster.

command: drush fact 9 1

http://drupal.org/sandbox/swentel/1781486 (note, you're maintainer too, so feel free to make adjustments)

I'll run some tests tomorrow, rest of the day is dedicated to do nothing :)

What's left before this issue can be moved to the core queue? Would be great to get a bot check plus increase visibility.

Sounds to me like this should be in the core queue then, so doing so. Please reset if I'm wrong though.

@yched when would you like a detailed review for possible doc / code style issues?

Per #22, this patch is nearly done, so I think a docs and code style review would be appropriate now, unless yched says otherwise. Also, this issue isn't blocking any other major or critical issues that I'm aware of, so incorporating that feedback should be done as part of this issue, not a follow up.

Did some basic performance tests, not being able todo all the use cases though, and no xhprof so far, I'll try todo one before wednesday, but then I'm afk after that until next monday as well. The cool thing is that we already profit from the _field_info_collate_patch() that got in :)

I have a public dump available if anyone in the meantime wants to have a go: http://dl.dropbox.com/u/15116672/dump-d8.zip
It contains 10 content types, each with 9 text fields and 1 term field. It has a lot of terms and 5000 nodes.

Single node view
before: Page execution time was 65.91 ms. Memory used at: devel_boot()=1.2 MB, devel_shutdown()=4.89 MB, PHP peak=5 MB.
after: Page execution time was 68.29 ms. Memory used at: devel_boot()=1.24 MB, devel_shutdown()=5.07 MB, PHP peak=5.25 MB.

Front page (I think it lists 7 different node types now)
before: page execution time was 141.57 ms. Memory used at: devel_boot()=1.24 MB, devel_shutdown()=7.55 MB, PHP peak=9.25 MB.
after: page execution time was 143.88 ms. Memory used at: devel_boot()=1.24 MB, devel_shutdown()=7.85 MB, PHP peak=9.5 MB.

So far, not really alarming if you tell me.

Thanks. 68.29 vs. 65.91 = 3.6%, which is enough to warrant digging into.

@yched - Happy to help. Let me know when! I really like your work here.

Note about the patch: type hinting with \Closure is a really bad idea. Closures were at the origin an implementation detail not supposed to be public, even if it may be now, it's still more flexible to type hint using callbable instead. Note that the callable type hint is not permited until 5.4 but at least you can document it as a callable and just not type hint it.

If callable is not available until 5.4, it is inappropriate to use while Drupal requires 5.3.x. In my mind, type hinting is a MUST for all new code being added to D8.

If you think Closure is wrong to use as a type hint, open up an issue to change it once 5.4 becomes a minimal requirement for Drupal. Until then, this should be type hinted as accurately as possible.

If callable is not available until 5.4, it is inappropriate to use while Drupal requires 5.3.x. In my mind, type hinting is a MUST for all new code being added to D8.

We technically can't type hint here, because it makes impossible to use any other form of callable. This can't be commited with a \Closure type hint.

PHP has technical limitations, and this is one <5.4 specific, we must live with it: you still don't type hint for strings and integers right? Then the same behavior for callables apply, don't type hint.

If we pass a callable we must not type hint else we loose great flexibility for no reason. We can replace the type hint by: if (!is_callable($my_param)) { throw new \InvalidArgumentException(); } in place to reproduce the fatal behavior of the wrong call.

Ready for a detailed code /doc review too?

Here is what I observed in #33. Please realize that is intended to be a nit picking code / documentation style review.

+++ b/core/modules/field/field.api.phpundefined
@@ -2145,27 +1952,27 @@ function hook_field_info_max_weight($entity_type, $bundle, $context) {
+ * @param array $context
  *   An associative array containing:
  *   - entity_type: The entity type; e.g., 'node' or 'user'.
+ *   - bundle: The bundle: e.g., 'page' or 'article'.
  *   - field: The field being rendered.
  *   - instance: The instance being rendered.
- *   - entity: The entity being rendered.

Just to be consistent, let's change 'entity type;' to 'entity type:'.

+++ b/core/modules/field/field.api.phpundefined
@@ -2180,23 +1987,23 @@ function hook_field_display_alter(&$display, $context) {
- * @param $display
+ * @param array $display_properties
  *   The display settings that will be used to display the field values, as

The variable used in the function call is $display. This needs to be adjusted.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Formatter/FormatterBase.phpundefined
@@ -0,0 +1,147 @@
+
+use Drupal\Component\Plugin\Discovery\DiscoveryInterface;
+use Drupal\Core\Entity\EntityInterface;
+use Drupal\field\Plugin\PluginSettingsBase;
+use Drupal\field\FieldInstance;
+
+/**

Are use statements suppose to be orders in alphabetical order?

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Formatter/FormatterBase.phpundefined
@@ -0,0 +1,147 @@
+   *
+   * @param array $plugin_id
+   *   The plugin_id for the formatter.
+   * @param Drupal\Component\Plugin\Discovery\DiscoveryInterface $discovery

Glad that this is type hinted. I would have guessed that plugin_id was a string otherwise.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Formatter/FormatterInterface.phpundefined
@@ -0,0 +1,108 @@
+  /**
+   * Return a short summary for the current formatter settings.

I think this needs to be 'Returns'.

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Formatter/FormatterInterface.phpundefined
@@ -0,0 +1,108 @@
+  /**
+   * Allow formatters to load information for field values being displayed.
+   *
+   * This should be used when a formatter needs to load additional information

I think this needs to be 'Allows'.

+++ b/core/modules/field/modules/number/lib/Drupal/number/Plugin/field/formatter/NumberDecimalFormatter.phpundefined
@@ -0,0 +1,73 @@
+
+use Drupal\Core\Annotation\Plugin;
+use Drupal\Core\Annotation\Translation;
+use Drupal\field\Plugin\Type\Formatter\FormatterBase;
+use Drupal\number\Plugin\field\formatter\DefaultNumberFormatter;
+use Drupal\Core\Entity\EntityInterface;

I think that these should be sorted into alphabetical order.

+++ b/core/modules/field/modules/number/lib/Drupal/number/Plugin/field/formatter/NumberIntegerFormatter.phpundefined
@@ -0,0 +1,45 @@
+
+use Drupal\Core\Annotation\Plugin;
+use Drupal\Core\Annotation\Translation;
+use Drupal\field\Plugin\Type\Formatter\FormatterBase;
+use Drupal\number\Plugin\field\formatter\DefaultNumberFormatter;
+use Drupal\Core\Entity\EntityInterface;

Ibid.

+++ b/core/modules/field/modules/number/lib/Drupal/number/Plugin/field/formatter/NumberUnformattedFormatter.phpundefined
@@ -0,0 +1,44 @@
+
+use Drupal\Core\Annotation\Plugin;
+use Drupal\Core\Annotation\Translation;
+use Drupal\field\Plugin\Type\Formatter\FormatterBase;
+use Drupal\Core\Entity\EntityInterface;

Ibid.

+++ b/core/modules/field/modules/text/lib/Drupal/text/Plugin/field/formatter/TextDefaultFormatter.phpundefined
@@ -0,0 +1,45 @@
+
+use Drupal\Core\Annotation\Plugin;
+use Drupal\Core\Annotation\Translation;
+use Drupal\field\Plugin\Type\Formatter\FormatterBase;
+use Drupal\Core\Entity\EntityInterface;

Ibid.

Just to be consistent, let's change 'entity type;' to 'entity type:'.

Actually, they should both be commas.

Are use statements suppose to be orders in alphabetical order?

I think that these should be sorted into alphabetical order.

Please stop posting this on issues, it's not in our coding standard. They can be in any order.

Thanks @yched. The docs look much good.

#42 patch still includes Closure type hint and documentation in some various points (in @return statement for example).

Another minor, but yet important detail: if you use callable as type hint (at least as document type for now) you should call it using call_user_func($var, $a1, $a2) instead of direct $var($a, $a2); Performance penalty will be almost insignificant and it will avoid PHP WSOD if you pass there an array($object, 'methodName') instead of a Closure object.

I'll reroll it today and enforce the suggested changes.

• Rerolled the patch
• changed
  

   * @return Closure
   *   A 'target closure' for field_invoke_method().
  

  into

   * @return callable $target_function
   *   A 'target function' for field_invoke_method().
  

• Changed the invocation to call_user_func array in field_invoke_method(). Please review if that's correct and sufficient
• Added the AlterDecorator to the FormatPluginManager.
• Removed the workaround in LegacyDiscoveryDecorator

Tried to create interdiff as well but then I needed to do the reroll again.

SideQuestion:
Are there already issues created to let widgets use the AlterDecorator too? maybe some other stuff too?

Great! @pounard - if you're ok with this one, can you push RTBC ? Or let us know where changes are needed ? Thanks!

This ?

Nice swentel, that's it!
@yched, thanks again!
Really hope it's ready to go now.

I won't RTBC it because I prefer to let people that know better the field API do it. Just know that callable type hint sounds fine now and I'm OK with the latest patch regarding that specific point.

@pounard fair enough, thanks for the OK on the remarks you had :)

Maybe we can create a change record modelled after http://drupal.org/node/1796000 in the mean time - we can safely assume this will go in (and I can remove the change record afterwards in case this won't make it.).

edit: change notice at http://drupal.org/node/1805846

The patch is well-tested in sandbox and manually so some clean-ups could be done in follow-ups. Let's get them commited

@yched because this change requires @catch feedback according http://drupal.org/node/1207020

"because the community was not able to come to a consensus and needs a top-down decision"

This is RTBC, that on its own means the committers will look at it.

@yched did you forget the patch?

@yched The commited #1778942-14: Discovery::getDefinition() / getDefinitions() : inconsistent return values for "no result" already have this hunk, so no need for re-roll

+++ b/core/modules/field/lib/Drupal/field/Plugin/Type/Formatter/FormatterPluginManager.phpundefined
@@ -0,0 +1,106 @@
+  /**
+   * Overrides Drupal\Component\Plugin\PluginManagerBase::getDefinition().
+   *
+   * @todo Remove when http://drupal.org/node/1778942 is fixed.
+   */
+  public function getDefinition($plugin_id) {
+    $definition = $this->discovery->getDefinition($plugin_id);
+    if (!empty($definition)) {
+      $this->processDefinition($definition, $plugin_id);
+      return $definition;;
+    }
+  }
+

Yup, missed this

Let's do that now :)

#67: 1785748-67.patch queued for re-testing.

Ok sorry for the delay on this folks; this took some time to go through.

In general this looks very similar to the widgets patch, so once again, great work on the clean-up there.

I committed and pushed to 8.x, but had some follow-up questions/observations. Also tagging for a change notice.

+++ b/core/modules/field/field.api.phpundefined
@@ -2145,27 +1952,27 @@ function hook_field_info_max_weight($entity_type, $bundle, $context) {
+ * @param array $context
...
- *   - entity: The entity being rendered.
...
-function hook_field_display_alter(&$display, $context) {
+function hook_field_display_alter(array &$display_properties, array $context) {

@@ -2180,23 +1987,23 @@ function hook_field_display_alter(&$display, $context) {
+ * @param array $context
...
- *   - entity: The entity being rendered.
...
-function hook_field_display_ENTITY_TYPE_alter(&$display, $context) {
+function hook_field_display_ENTITY_TYPE_alter(array &$display_properties, array $context) {

Why remove entity here? That seems like valuable context. I noticed 'module' was also removed from another place (can't recall where, sorry).

+++ b/core/modules/field/field.attach.incundefined
@@ -577,6 +711,26 @@ function _field_invoke_widget_target() {
+ * @param mixed $display

+++ b/core/modules/field/lib/Drupal/field/FieldInstance.phpundefined
@@ -68,6 +75,88 @@ public function getWidget() {
+   * @param mixed $display_properties

I am really not a fan of these "mixed" type variables. Would it make sense here to make view mode (or whatever) an explicit function parameter?

+++ b/core/modules/field/field.attach.incundefined
@@ -577,6 +711,26 @@ function _field_invoke_widget_target() {
+function _field_invoke_formatter_target($display) {
+  return function ($instance) use ($display) {
+    return $instance->getFormatter($display);
+  };
+}

woah. crazy syntax. didn't know you could do that. I can't quite parse what's happening there, so a comment would be great.

Change notice is at http://drupal.org/node/1805846, so this is normal again.

Also, @yched, should we just bundle the plugins and widgets conversions into one issue per module ?

Bug with field access: #1814418: FormatterBase::view() calls field_access() with the wrong order of arguments

Provided the patch to
- remove 'mixed' from @params in function header
- comments for the use of php closure

Removing tags as well