Help needed for some field_attach function docs

I am not a very good writer, but I think helping out with documentation is one way to learn how things work. I prefer programming but and would be glad to help out with docs. I am a native english speaker.

1. Field_attach_presave seems ok.
2. Field_attach_form. I documented the results array. I wasn’t sure how to format some of the values where it wasn’t obvious what variable they came from. Lines 238,242,246,248,250. Also the example had 3 field values but I only showed 1.
3. Field_attach_preprocess. I didn’t know where to start.
4. It seems like _field_invoke _field_invoke_default introduce something different about hooks worth mentioning. Below is the general idea I know we would never use my wording.
5. The _field_invoke and _field_invoke_default functions implement a new technique for creating hook function names. An additional place holder I’ll call hookop defines an operation name. e.g. hook_field_hookop where the hookop placeholder is the value of the $op argument in the call to the function _field_invoke . The hookop placeholder allows the creation of default field hooks without the module name placeholder e.g. field_default_hookop. The function _field_invoke_default uses this hookup technique without the module name placeholder to implement field default hooks. I did not include this in the patch.
6. In lines 163 – 164 file field.attach.inc is the word render used correctly?
7. The Doxygen comment starting at line 203 in field.attach.inc shows up in my doxygen doc as the description for field_attach_form. I’m using doxygen 1.5.8. Maybe I need the same doxygen config file that drupal uses.
8. Coder reported an error on line 182 field.attach.inc else if. It wanted elseif. I fixed it.

Thanks puredata. As Barry said, your help is really appreciated.

- suggestion : could you prefix your patches filenames with some meaningful works (like 'field_attach_doc-361389-5.patch"). Much easier to handle when you have 25 patches in a folder somewhere ;-)
- 1. : field_attach_presave works for me. While we're in there, we could remove the TODO in the function.
- 3. : don't bother with field_attach_preprocess() for now. I seriously hope that #455844: Allow more granular theming of drupal_render'ed elements will let us get rid of it.
- the existing "Field Attach API functions load, store, generate *Form API* structures..." is correct.
- in the same sentence, we could fix "and perform a *vareity* of other functions " (typo exists in the current code)
- 4., 5. : I don't think I understand the part about _field_invoke and 'hookop'. The bottomline here is that strictly speaking, those functions are not hooks (can be implemented by any module), but callbacks. That terminology issue is outside the scope of this patch, though.
- 6.: "In lines 163 – 164 file field.attach.inc is the word render used correctly". No it's not :-). I reworded thos comments in #392494: Fields API needs to allow searching of scalars, I'd say don't bother here.
- 7. Dunno about that... api.drupal.org does seem correct : http://api.drupal.org/api/function/field_attach_form/7

Nice job on the form structure doc. That part is not easy.
- indentation issue + space before the ? in

+ *      // Is it required?
+ *       '#required' => bool , 

- Even short, inline, comments should end with a period. Also, watch out for missing space in //Foobar.
- Fix "=>" in '#fields' = > array(
- The syntax in the $form structure is not consistent. Some places use the correct 'foo' => array(, some others use $field_name array(. See the existing doc for field_default_view (which, if we want to be consistent, should probably be moved to field_attach_view, BTW)
- I'm not sure we need to document every property (#prefix / #suffix...). What's important here IMO is the structure, not an exhaustive description.
- It would be nice to document the case of a 'multiple' widget (a single form element that holds multiple values itself, e.g a 'select' or 'radios' widget).
In this case , there is no nesting below a 'delta' key (the 0 => array( part in your example).
See the phpDoc for field_default_view(), which has a similar distinction between formatters that display a single value and those that display multiple values.

1. Having trouble trying to figure out what the syntax rules are for documenting an associative array in doxygen. I have been trying to decipher the syntax used to document the $object in field_default_view(). The syntax looks similar to PHP but not quite. For now rather than trying to explain it to me I’ll just refer to lines in my patch that I am not sure about, hopefully someone will tell me the corrections and I will fix the patch. Learning by trial and error is a good place for me to start. A related issue but not directly what I am talking about. Look at way the api.drupal.org formats function field_default_view().
2. These are line numbers in the patch file field_attach_doc-361389-8.patch
a. Lines 58, 66. The key is a field name. I used the example field name ‘field_foo’.
b. Lines 61,88,104,118,121and 141. The value is a field name. I used a string literal ‘field_name’.
c. Line 67. The value I found in the debugger when field_default_form () took a branch that executed field_multiple_value_form() So what is this? The name of a theme template?
d. Line 75,91, 107 124. Here I am looking help to better phrase these lines.

Reroll of file_attach_doc-361389-8.patch. Field_attach_doc-361389-10 is a more appropriate filename.

rerolled

Rerolled now works the same as field_attach_doc-361389-10.patch did.

$object->content['field_foo']['wrapper'] = array(. I am not seeing 'wrapper' in the body field's render array. Is that just a placeholder word?

Ahah is usually spelled AHAH.

After these, the issue is RTBC IMO.

I'd like to chat with yched or whomever knows about the render() arrays that field_attach_view() emits. i'd like to see if they can be simplified. or point me to an existing issue.

Great work, + thanks for sticking to this despite our lack of feedback, puredata.

Here are a couple comments:

- typo: orginial

-

+ *   An associative array that is combined the with original form. Note that the orginial $form
+ *   is also modified inside the call to field_attach_form. It is passed in by reference.
+     // The elements listed are the only ones either added of changed.
+ *   The $form array with a field named 'field_foo' will look like:

The // should be a *

- '#type' vs #title (unquoted). if we present a sample form structure, we should consistently quote keys.

- //Unless it is Ahah then TRUE.
Missing space after // (other lines have this too)
+ AHAH, as noted by moshe.

- Missing a line with a closing ) before //Unless it is Ahah then TRUE.

- 'field' => array(field array),
is not really clear. The value is the field definition array.

- There's an issue with stuff like:
#theme => 'field_multiple_value_form',
and
#title => 'label',
In the former, the string is the actual value, will never change. In the latter, it's actually depending on the field configuration.

For the two points above, I'd suggest stepping out of a 'realistic' PHP syntax and do stuff like:
'field' => the field definition structure,
#title => the label of the field instance
#multiple => the field cardinality
(side note: #497504: rename #multiple to #cardinality in field forms - not for this patch)
Thus, string literals would be understood as constants

- 2 => array(
This part is incomplete.
I'm thinking providing *one* fully developped example might be enough. Could be something like:

+ *       // one numbered sub-array for each field value.
+ *       // Only multiple form elements: one numbered sub-array 
+ *       // for each field value.
+ *       0 => array(
+ *         '#type' => widget type,
+ *         '#default_value' => 'value0',
+ *         // Whether or not field is required.
+ *         '#required' => whether the field is required.,
+ *         '#columns' => the array of field columns
+ *         '#title' => the label of the field instance,
+ *         '#delta' => 0,
+ *         '#field_name' => the field name
+ *         '#bundle' => the bundle name,
+ *       )
+ *       1 => array(
+ *         ...
+ *       )

Similar could then apply to the 'view' structure doc.

-

+ *     '#default_value' => array(
+ *       '#type' => 'widget_type',
+ *       '#default' =>  array(

Looks odd. Is this really the structure that gets generated ?

-

+ *          0 => array(
+ *            'value' => 'value0',
+ *          1 => array(
+ *            'value' => 'value1',

Missing closing )'s

- moving the PHPdoc for the 'view' structure to field_attach_view() is a good thing IMO, but then it should be removed from field_default_view()

re moshe #15:
"$object->content['field_foo']['wrapper'] = array(. I am not seeing 'wrapper' in the body field's render array. Is that just a placeholder word?"
Could it be that you're testing with #367215: Remove 'exclude from $content' display setting ? ;-)

I am going to monitor this issue daily until we get it finished. I am not sure what exactly you meant by “providing one fully developed example”. So I removed the reference to a multiple value but single theme data. Yes there was an error to the sub-array for multiple value single theme example. I removed that part of the example. Like I said I think that change is wrong but I am not sure what to do.

Rerolled patch. Was out of date.

This is a reroll. I use to have a windows wamp machine. It crashed and I lost everything. I now have a mamp macbook leopard. Version 5 and 6 work fine but version 7 says "page not found". The debugger says "Fatal error: Class 'PDO' not found in /Applications/MAMP/htdocs/drupal/includes/database/database.inc on line 184". I know this is a busy time for everyone. I might be out service while I fix this.

Thanks for the reroll. Let's get this over with. Those things are really painful to document.

Updated patch fixes a couple changes in the functions since the last reroll, makes some explanations a little clearer, fixes some 80 chars line wrap, and tries to be explicit about string literals ('#theme' => 'field_multiple_value_form' because that's the actual value that will be set), vs full text explanations ('#field_name' => the name of the field, for 'variable' values)

Meaning I can't set to RTBC myself now ;-)

Wow, awesome work, folks!

Committed to HEAD!

Awesome clean up work. Henry Miller was right. "Even in a pile of garbage can bloom a rose." For me, this was a little painful but very very rewarding.
Thanks