Custom #type machine_name 'exists' callbacks cannot access other form elements/values

[merlin@furor ctools]$ grep -R "'exists'" /var/www/d7p3/modules/
/var/www/d7p3/modules/filter/filter.admin.inc:      'exists' => 'filter_format_exists',
/var/www/d7p3/modules/system/system.admin.inc:      'exists' => 'system_get_date_types',
/var/www/d7p3/modules/node/content_types.inc:      'exists' => 'node_type_load',
/var/www/d7p3/modules/menu/menu.admin.inc:      'exists' => 'menu_edit_menu_name_exists',
/var/www/d7p3/modules/taxonomy/taxonomy.admin.inc:      'exists' => 'taxonomy_vocabulary_machine_name_load',

These functions all accept only a single argument, so this patch will not break anything as far as I can tell.

Oh, and I'm interested in this because the export-ui stuff in CTools could really benefit from being able to have a generic 'exists' function for exportable objects. It could extract the export name out of the $form_state and use a single function for this -- so this would be a nice win. It's not strictly necessary, we can work around it, it just makes things a little more complex.

Ok, this seems like a harmless enough change. The fact that nothing else in core needs to change for tests to pass is evidence of that.

I asked about changing the existing _exists() function declarations to indicate that this extra param is available, since currently we don't have a good way of documenting callbacks and the only way you know what arguments to pass in is by either grepping or reading form.inc. :P However, sun pointed out that these are not uniform; sometimes generic loader functions are used for this, sometimes specific functions, etc. I talked to jhodgdon about opening up a separate issue to discuss callback documentation since it's out of scope for this issue.

Committed to HEAD.

Oooo, fantastic. I just finished writing a FieldAPI wrapper for this element type so people can set up name/machine_name fields, and it's working great. This patch fixes the only outstanding issue, and will allow uniqueness to be enforced across fields, etc.

Thanks!

I know that it's too late for this to go into D7 given the lateness of things, but after some experimentation it's clear this approach is still broken for many uses, including FieldAPI. If multiple instances of a field occur in the same form, the callback has no way of determining which instance of the field is in fact responsible for the value. Passing in $value, $element, AND $form_state would almost certainly do the trick as the callback would be able to look at the specific field definition. Any additional information (like pseudo-parameters) that needed to be passed along to the callback could be embedded in the element definition.

The attached patch does just that, and like the above one shouldn't affect any existing code.

Hrm. :\ Yeah, if we don't get this in now, it's going to really screw people up since we'd break APIs later on. :(

Committed to HEAD.

My tiny little nerd heart is so happy right now.

if anyone else is searching for eaton's FieldAPI wrapper for name/machine_name fields, it is here: Safeword