Formatters refactoring [#213279]

(As discussed with Karen on IRC)
The following patch does two things :

* Refactor formatters API to be more tightly tied with themes and drupal_render, as initially suggested by dopry.
- formatters are basically theme functions (and thus overridable), operating on a #-array $element.
- field rendering on node_view relies completely on drupal_render cascading rendering
- field data sanitization (XSS prevention, etc) is moved out of formatters (we don't want to let that in the hands of theme overrides) and into a separate step, invoked before rendering (currently hook_field('sanitize'), but that might be revised)
- content_format() API function, used for views output and encouraged for custom themeing, keeps the same behavior

* Allow formatters to handle multiple values
- new 'multiple values' key in hook_field_formatter_info, is either CONTENT_HANDLE_MODULE or CONTENT_HANDLE_CORE (similar to what we now have for widgets)
- content_format() has to be fed with an "$items" array ($node->field_foo) when used with a 'multiple-values' formatter,
and with an $item array ($node->field_foo[n]) when used with a 'single' (regular) formatter

The patch is not quite ready yet, but I'm posting it here for early reviews.
It takes care of text and number fields for now, and leaves userref and noderef fields display broken for now.
It implements a dummy 'foo' formatter for text fields, illustrating what 'multiple values' formatters look like

It still has a few TODOS / open questions, notably :
- best way to implement the 'sanitize' pass : hook_field($op = 'sanitize'), or hook_field_sanitize (similar to hook_field_set_empty) - that's mainly a code-style point.
- the structure of the render array could possibly be unified a bit further between 'single' and 'multiple-values' formatters. I still need to check that.
- not sure of what to do with the $items array passed to template files when using a 'multiple-values' formatter :
we have several 'items', but only one output value. The current patch raises "undefined index $item['view']" warnings.

Comment	File	Size	Author
#6	cck_formatters_3.patch	32.79 KB	fwalch
#5	cck_formatters.patch	31.66 KB	yched
#3	cck_formatters.patch	31.66 KB	yched
#2	cck_formatters.patch	30.08 KB	yched
	cck_formatters.patch	21.28 KB	yched

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

yched CreditAttribution: yched commented 23 January 2008 at 12:22

Forgot to add : the patch does not take care of fieldgroups yet.
In order to test, please comment out the content of fieldgroup_nodeapi.

Comment #2

yched CreditAttribution: yched commented 26 January 2008 at 04:23

File	Size
cck_formatters.patch	30.08 KB

Updated patch, should be complete AFAICT.

- all field modules are ported to the new formatters api
(BTW, the corresponding theme functions are pure logic and no markup, which hardly anyone would be willing to override - the 'formatters as theme' feature is precious for less basic contrib field types, though)

- the method used to hide empty fieldgroups didn't work with the new approach, so fieldgroups theming needed a refresh. While I was at it, I turned the 'simple fieldgroup' style into a template.

- I left the 'sanitize' step as a hook_field op. It makes 2-3 awkward lines in content_format, but I thought it was better than requiring yet another different hook from field modules.

- I left sample render #-arrays for 'single value formatter' and 'multiple values formatters' in code comments, to help visualizing the structures.
It should be noted that these #-arrays embed a full $node for each rendered field.
It does not have any impact on memory usage, because the $node we receive is a reference, and thus we don't create actual new copies, but it makes a reading through a dsm($node->content); much more painful.

- the patch still includes a dummy 'multiple values' formatter ('text_formatter_foo'), that will of course not be actually committed.

I'll wait a couple days for reviews or comments, but I think we're good to go (this was our last API-breaking task before an alpha release)

Comment #3

yched CreditAttribution: yched commented 26 January 2008 at 04:53

File	Size
cck_formatters.patch	31.66 KB

Previous patch didn't include userreference.

Comment #4

eaton CreditAttribution: eaton commented 1 February 2008 at 07:10

Subscribing. A quick smoke-test indicates things are working nicely, but I'll be testing it more thoroughly over the weekend. Very, very cool.

Comment #5

yched CreditAttribution: yched commented 1 February 2008 at 13:15

File	Size
cck_formatters.patch	31.66 KB

New patch fixes a typo in nodereference 'full node / teaser' formatters - reported by Moshe.

Comment #6

fwalch CreditAttribution: fwalch commented 1 February 2008 at 23:43

File	Size
cck_formatters_3.patch	32.79 KB

Great work, yched! I was already searching for some functionality like this for my GHOP task to port cck_map to Drupal 6. I had a quick look at the code and noticed two things:

In content_handle(entity='formatter'), a variable $formatter is used, which doesn't exist. The attached patch fixes this and changes the $field parameter to $object.
in text.module, there's a formatter called 'foo' with the description 'Test multiple'. This is not changed by the patch.

Comment #7

yched CreditAttribution: yched commented 2 February 2008 at 00:03

Yes Beeradb pinged me on IRC about your GHOP task. Placing points on a map is the typical use case for 'multiple values' formatter.

- You're quite right about content_handle params. I tested the 'multiple formatter' part in a previous iteration where I didn't use content_format yet, and then obviously stopped in the middle of the content_handle overhaul. Thanks for the patch.
- The 'foo' formatter is just there to illustrate how 'multiple' formatters work, it will be removed before we commit this :-)

Comment #8

yched CreditAttribution: yched commented 7 February 2008 at 18:45

Status:

Needs review

» Fixed

The patch didn't apply anymore. We might find we can still further unify the structure of #-render-arrays for 'single' and 'multiple' formatters, but I committed the current state of the code so that we can move on.