Clean-up entity build/view modes

+1 to 2) at least. I think some help text for each build mode would greatly assist intermediate developers with understanding how to take advantage of them.

I can't really comment on 1) or 3). build -> view does seem to make sense, in that we are not altering the structure or data of the entity so much as we deciding on how they "display".

Agreed on all points. Also 3) renaming "full" to "default" -- I was a bit confused when I saw that default argument handling for the first time and asked myself, why we hard-code one of the build modes. If "full" was named "default", then I wouldn't have asked myself this question.

I wonder whether we can even shorten $view_mode to just $mode...? Or perhaps even find a better name... I mean, we are all totally familiar with $build_mode, because we all know that it stemmed from the old $page and $teaser arguments... but honestly, if I'd look at Drupal for the first time, then I'd probably have to consult the handbooks or API documentation to understand and figure out what a "build_mode", "view_mode", or "mode" is..........

yay! The idea: That other patch renamed the argument to $display. And I think, this term is not only exactly what the argument is about ("display as ..."), but also nicely maps to the actual configuration page in the Field UI ("Manage display").

subscribe

A quick superficial review of #6. I am liking how the info hook looks. One general question would be on the choice of "label". In FAPI its 'title' and 'description', in Menu its 'title' and 'description'. In this case its is just going to be displayed as a label in most interfaces, but is there an argument against title?

Overall I like the view_mode to build_mode change. At least in terms of the comments and the code it seems to work. I especially noticed that the word "display" is deeply linked with this concept and "view" seems more synonymous with "display" than "build" does. There probably is a point to be made that not all "build modes" have to do with visual output, but I think this is a good compromise.

+++ includes/common.inc	21 Dec 2009 20:50:46 -0000
@@ -6388,6 +6389,29 @@ function entity_create_stub_entity($enti
 /**
+ * Returns informations about view modes for an entity type.
+ *
+ * @see hook_entity_view_mode_info()
+ * @see hook_entity_view_mode_info_alter()
+ *

"Returns informations" => "Returns information"

+++ modules/comment/comment.module	21 Dec 2009 20:13:21 -0000
@@ -832,16 +832,16 @@ function comment_view($comment, $node, $
  * Builds a structured array representing the comment's content.
  *
  * The content built for the comment (field values, comments, file attachments or
- * other comment components) will vary depending on the $build_mode parameter.
+ * other comment components) will vary depending on the $view_mode parameter.
  *

The word "built" might cause confusion for people already familiar with the old "build modes".

+++ modules/field/field.attach.inc	21 Dec 2009 20:14:00 -0000
@@ -143,7 +143,7 @@ define('FIELD_STORAGE_INSERT', 'insert')
  *   - The $form in the 'form' operation.
- *   - The value of $build_mode in the 'view' operation.
+ *   - The value of $view_mode in the 'view' operation.
  *   - Otherwise NULL.

Sentences like this seem to make more sense now :)

+++ modules/node/node.module	21 Dec 2009 20:18:13 -0000
@@ -222,27 +222,34 @@ function node_entity_info() {
-function node_field_build_modes($obj_type) {
-  $modes = array();
-  if ($obj_type == 'node') {
-    $modes = array(
-      'teaser' => t('Teaser'),
-      'full' => t('Full node'),
-      'rss' => t('RSS'),
+function node_entity_view_mode_info() {
+  $modes['node'] = array(
+    'full' => array(
+      'label' => t('Full node'),
+    ),
+    'teaser' => array(
+      'label' => t('Teaser'),
+    ),
+    'rss' => array(
+      'label' => t('RSS'),
+    ),

I like how this is looking. Seems more... Drupalish.

+++ modules/system/system.api.php	21 Dec 2009 22:53:31 -0000
@@ -149,6 +149,74 @@ function hook_entity_info_alter(&$entity
+ * View modes let entities be displayed differently depending on the display
+ * context. For instance, a node can be displayed differently on its own page

Double use of "display" seems less than ideal. Maybe "let entities be displayed differently depending on the output context"? I dunno, that's not great either.

+++ modules/system/system.api.php	21 Dec 2009 22:53:31 -0000
@@ -149,6 +149,74 @@ function hook_entity_info_alter(&$entity
+ * Modules defining entity types should use this hook to define the set of
+ * build modes that makes sense for their own display contexts. Third party
+ * modules can also use this hook to define additional build modes for existing
+ * entity types.

"build modes" => "view modes" :)

This review is powered by Dreditor.

+1! Let me quickly state how cool it is to have someone else look at these kind of patches. And how AWESOME it is that yched rolls them! :) Seems like we're almost done here!

Overall I like the view_mode to build_mode change. At least in terms of the comments and the code it seems to work. I especially noticed that the word "display" is deeply linked with this concept and "view" seems more synonymous with "display" than "build" does. There probably is a point to be made that not all "build modes" have to do with visual output, but I think this is a good compromise.

Does this suggest that 'display mode' is the better name?

@Dries
After looking at where these "build_mode" or "view_modes" live in the wild, I think it would hurt code readability to call it "display_mode", because the word display is used in many functions as a variable. I do think "display_mode" matches up nicely with the "manage display" tab, but there is no reason that tab could not be "Manage View Modes" or something like it. I think to make "display_mode" readable, some other changes to the code would have to be made.

OK, let's stick with 'view_mode' for now.

Patch applied. No obvious problems. I guess we are waiting on bot.

+++ modules/node/node.module	22 Dec 2009 21:31:42 -0000
@@ -222,27 +222,34 @@
- * Implements hook_field_build_modes().
+ * Implements hook_entity_view_mode_info().
...
-function node_field_build_modes($obj_type) {
...
+function node_entity_view_mode_info() {

Hm. That's a pretty long info hook name... Of course, it makes sense in the context of this patch...

But when taking a (large) step back, then I seriously have to wonder why this info is not simply contained in hook_entity_info() instead? Don't those view modes 100% depend on the entity, and, doesn't (almost) every entity require at least one view mode? (unless it defaults to a default/full view mode like all of the current core entities seem to do)

So, when taking this step back, I really have to wonder why this is a separate hook at all.

This may sound like scope-creep for this issue/patch, but then again, we seem to touch every single line that deals with build/view modes in core in here...

I know that we already have a lot of properties in hook_entity_info() and that adding view modes would increase that list - but from a technical standpoint, I think it really belongs together, and would also clarify the task of a larger API revamp in D8, since we'd know most/all all of the entity properties due to the possible registry information.

The hook currently seems to be in place so that we can support registering view modes for entities that are not registered by the same module (e.g. node/search). However, for such purposes, we already have hook_entity_info_alter().

Thoughts?

I'm on crack. Are you, too?

@sun

But when taking a (large) step back, then I seriously have to wonder why this info is not simply contained in hook_entity_info() instead?

Makes sense, but this is probably a little deeper than I'm willing to commit an opinion :)

1) I think this patch is an awesome improvement to D7!

2) Apologies for not chiming in earlier, but having read the above issue comments and patch #22, I'm wondering why not name it $display_context. If there's agreement that this is a good name, I'll be happy to roll the patch for it, using #22 as a starting point. I think this name is both clear and $display could still be retained throughout the code to refer to what it already refers to: the settings to use for a particular display context (or the name of the context itself, in a situation where the settings could be retrieved given that name). I also think this would flow nicely with yched's proposal in #3, where "display style" or $display_style could be used to refer to a name for which display settings can be assigned, and then there could be a mapping to say which $display_style to use for a $display_context.

Thanks, yched! This makes much more sense now!

For me, this patch looks RTBC. I'm not sold on the idea that "display_context" would be a better name... "A display_context of teaser" sounds a bit weird to me. While a display context of "print" would perhaps make slightly more sense, I think that "view mode" really maps best to the functionality's purpose - at least for D7.

So, again, for me this looks RTBC, but I'll leave that to someone else.

Ok, I'm sold on 'view mode' being a better name than 'display context'. I still like 'display context' more if we were only thinking from a field perspective, where we already do use the word 'display', and where the view mode very much is a context by any definition of the word (a node 'teaser' is a display context for the field). But, thinking about #24 and #25, I agree that we can't just take the field's perspective, especially since view modes are defined for the entity, and from the entity's perspective, 'display' is a new term that conveys no new information than the already accepted term, 'view', and 'mode' is more accurate than 'context'.

I read through the patch pretty thoroughly, and it looks great. Bot likes it (though perhaps, given the breadth of the patch, should be re-run prior to commit). So: RTBC.

Umph, this patch needs a quick reroll because of the other field API clean-up patch that just went in. Sorry about that!

Looks great now. Committed to CVS HEAD. Still running the tests locally though. Thanks!

Committed to CVS HEAD. Thanks yched.

What about update guide docs?

Yes, the hook changes should still be documented in the update guide.

I opened #671268: Add descriptions for view modes. to see about getting some description copy going.

Followup on #1476882: Add missing taxonomy term view mode.