Still on Drupal 7? Security support for Drupal 7 ended on 5 January 2025. Please visit our Drupal 7 End of Life resources page to review all of your options.API page: http://api.drupal.org/api/drupal/core%21modules%21field%21field.form.inc...
Enter a descriptive title (above) relating to field_default_form, then describe the problem you have found:
There are no params at all documented here!!!
| Comment | File | Size | Author |
|---|---|---|---|
| #14 | field-documentation-1681468-14.patch | 1.8 KB | eddie_c |
| #9 | field-documentation-1681468-9.patch | 1.8 KB | eddie_c |
| #6 | field-documentation-1681468-6.patch | 1.83 KB | eddie_c |
| #4 | field-documentation-1681468-4.patch | 1.51 KB | eddie_c |
| #2 | field-documentation-1681468-2.patch | 1.21 KB | eddie_c |
Comments
Comment #1
jhodgdonThanks, yes that function needs documentation!
Comment #2
eddie_c commentedHere is a patch containing documentation for the params
Comment #3
jhodgdonThanks -- great start!
As long as we're cleaning up this function's documentation, how about fixing the verb in the first line (should be Creates not Create)?
Also:
- I see the first comment in the code says that $entity can be left out -- that should probably be mentioned in the docs. [example call of this type is in field_ui_default_value_widget() if you want to look at it]
- It also looks like certain of the arguments are only used in certain circumstances -- for instance, it looks like $items would override usage of $instance? I think the parameters need more details...
Comment #4
eddie_c commentedHi jhodgdon, thanks for the encouragement! As you can probably tell I've just started trying to contribute to D8. I've followed your advice and I've read a bit more about the FAPI. I've managed to expand a bit on the parameter descriptions. However, still not sure what you mean about $items overriding $instance. As far as I could see they contain quite different types of data and are not mutually exclusive? If you have a minute to explain any further that would be great!
Anyway, I've attached an updated patch.
Comment #5
jhodgdonThanks -- this is better! I think it could use another round of improvements:
a) I'm wondering about the first line description. It seems like this function is actually creating a form element for editing just *one* field, not "a separate element for each" field? And it also is capable of creating a form element for setting default values, right? So let's try to get as much of that into the first line as possible, and if it's incomplete, add a blank line and a paragraph with a more complete description of what this function actually does.
b) Ignore what I said about $items. :)
c) Looking through the parameter descriptions in the current patch... they mostly look good! Some thoughts:
- in $entity: this function doesn't create a form (just an element), and "dummy" isn't really accurate (it's a very real form for setting default values I think? even though the code comment calls it a dummy form, that doesn't seem right to me).
- $field: In cases like this, where the array is a standard structure with information about some object/concept, we usually use phrasing like "An array representing...", so something like "An array representing the field whose editing element is being created."... or maybe you can phrase it even better.
- $instance: looks good! But I think it should be just "$field" not "the $field". Either use $field as a variable, or if you want to write prose, say "the field" without the $. Also here, perhaps "representing" would be good terminology?
- $langcode: the function also uses this variable to find default values if $items is empty, and it gets into the returned element too, so perhaps it would be better not to mention $items specifically?
- $form: I'm not sure what attaching a field to a form would mean? Maybe it would be more accurate to say it's the form the editing element will be added to?
d) All @param and @return descriptions need to end in ".". (I noticed the @return was missing the .).
Comment #6
eddie_c commentedThanks for being so thorough and patient with this. I'm learning a lot here! Here goes with another patch which I think addresses everything that you've raised...
Comment #7
eddie_c commentedComment #8
jhodgdonThanks, looks better! A few things still to fix:
a) The first line needs to be one sentence:
http://drupal.org/node/1354#functions
Also the first line really shouldn't refer to parameters by name, since it is often used in lists of functions on api.drupal.org and needs to stand alone.
b) In the next paragraph, i.e. needs to be followed by a comma.
c) On $instance... It says "a bundle of $entity_type", but that's not really accurate, because the field could have different $instance structures within the same entity type (for instance, different node bundles can have different field properties).
Comment #9
eddie_c commentedThanks - here's another take..
Comment #11
eddie_c commented#9: field-documentation-1681468-9.patch queued for re-testing.
Comment #13
jhodgdonWeird that the tests are failing -- it's definitely documentation-only... So it can't be this patch.
Anyway, the patch is looking pretty good! I did find two minor issues:
a)
it's => its
b) The blank line between the params and return value has some extra blank spaces at the end. If you're planning to continue submitting patches (and I hope you will!) you might want to set up your programming text editor so it either highlights or deletes trailing whitespace at the ends of lines.
Comment #14
eddie_c commentedOk, those things are fixed. Let's see if this passes...
Comment #15
jhodgdonExcellent! This is ready to commit.
Comment #16
jhodgdonThanks! Committed to 8.x and 7.x.