fields : do a pass of TODOs checking

I'll take this on and post what I find when I'm done.

I updated my view of HEAD a few minutes ago and there are 89 occurrences of "TODO" in the Field module. Now I need to know where the current TODO list is located so I can compare. Anyone? Thanks in advance!

bjaspan (over IRC) thought that this issues is more involved than a 2. He suggested I look through the TODO and fix what I can and post patches, so that will be my approach. As possible, I will look to comparing with the issue queue open issues at http://drupal.org/project/issues/drupal?status=Open&priorities=All&categ..., but as he suggested, that may be a bit much for me as a newcomer...

I'm not getting to this in a timely fashion, and many of the TODO comments require advanced knowledge of the Field API, who is working on what, etc... So I am returning this one to the larger group.

Not sure whether this is the proper issue, but here is one patch for one of the TODOs: the one about field name length.

In addition, fixing this made me wonder: currently, nothing prevents field_create_field from creating fields with names starting with a number instead of [a-z_]. Shouldn't this be prevented ?

Here is a revised version. It now checks the start character.

I've also added simpletest for illegal field names (non-alpha first char, and non-alnum other char).

I've also added a test for the field name length, however here I ran into a limitation of the simpletest mechanism (see patch for details) and had to remove that test. Not sure there is an actual solution, and it would likely have to be at the simpletest level anyway.

Rerolled with just a comment instead of the test.

It fails if we test it with a name that should be valid, because it is 31 characters long: in that case, the assertion succeeds and processing continues to create the field, triggering the mysql error because the table name is now 65 (minimum) characters long. This is not demonstrated by the test in its current state, though, only by the "positive" test which I wrote and removed before committing, which tests that creation succeeds below 32 and fails above, as it succeeds only below 30 or 29 depending on the number of previously created fields (number of digits in the field fid).

Here it is with the test reinserted and commenting style fixed.

Taken into account active/inactive field and instances.

Adding Fields in Core tag.

Committed to CVS HEAD. Setting back to 'code needs work' for the remaining TODOs. Thanks fgm.

We have a TODO at http://api.drupal.org/api/drupal/modules--field--modules--text--text.mod... (from http://drupal.stackexchange.com/questions/8829/how-to-change-the-length-...)

TODO's find in the fields module:

./field.form.inc:293: * TODO : convert to a template.
./field.crud.inc:478:  // TODO: Check that the widget type is known and can handle the field type ?
./field.crud.inc:479:  // TODO: Check that the formatters are known and can handle the field type ?
./field.crud.inc:480:  // TODO: Check that the display view modes are known for the entity type ?
./field.crud.inc:574:    // TODO: what if no 'default_widget' specified ?

The first one can be found on the function function theme_field_multiple_value_form($variables) { ... } on line 301. There TODO claims that the HTML part should be in a seperate HTML tpl. The HTML is as follows:

$output = '<div class="form-item">';
$output .= theme('table', array('header' => $header, 'rows' => $rows, 'attributes' => array('id' => $table_id, 'class' => array('field-multiple-table'))));
$output .= $element['#description'] ? '<div class="description">' . $element['#description'] . '</div>' : '';
$output .= '<div class="clearfix">' . drupal_render($add_more_button) . '</div>';
$output .= '</div>';

I'm not shure if these lines should be in a seperate html tpl. Wouldn't it be a little bit overkill?
http://api.drupal.org/api/drupal/modules!field!field.form.inc/function/theme_field_multiple_value_form/7
Suggestion: Remove the TODO from description.

The second, third and fourth one can be found in function field_create_instance($instance) { ... } on line 454 with a notice:

  // TODO: Check that the widget type is known and can handle the field type ?
  // TODO: Check that the formatters are known and can handle the field type ?
  // TODO: Check that the display view modes are known for the entity type ?
  // Those checks should probably happen in _field_write_instance() ?
  // Problem : this would mean that a UI module cannot update an instance with a disabled formatter.

I couldn't find any issues regarding these TODO's.
http://api.drupal.org/api/drupal/modules!field!field.crud.inc/function/field_create_instance/7

Last one can be found in _field_write_instance($instance, $update = FALSE) { ... } on line 554.
http://api.drupal.org/api/drupal/modules!field!field.crud.inc/function/_field_write_instance/7

@yched: All of them?