Change field admin form and function names [#304828]

Function names in content.admin.inc are completely out of the usual conventions (menu callbacks starting with _, form functions not ending with _form...). We've been carrying this since 4.7...

Attached patch fixes this.
A little late, but I'd much rather fix this now rather than in a subsequent release or carrying this through the D6 cycle.

Comment	File	Size	Author
#1	content_admin_cleanup.patch	21.91 KB	yched

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

yched CreditAttribution: yched commented 6 September 2008 at 15:54

Status:

Active

» Fixed

File	Size
content_admin_cleanup.patch	21.91 KB

Oops, patch missing, I hit 'Submit' instead of 'attach'.

So, here's the patch, I committed it after checking all forms + content copy work ok, and tests are still green.

Comment #2

KarenS CreditAttribution: KarenS commented 6 September 2008 at 19:42

Title:

Cleanup function names in content.admin.inc

» Change field admin form and function names

Not a bad idea, but all the modules using drupal_execute() on the admin form to create fields (I think there may be a lot of them) will be broken, so we should announce this to developers or at least make it more obvious that this has changed.

Comment #3

yched CreditAttribution: yched commented 6 September 2008 at 20:35

Shouldn't they be advised to use CRUD functions instead ?

Comment #4

KarenS CreditAttribution: KarenS commented 6 September 2008 at 20:43

Sure, but people love drupal_execute(), both module developers and individuals writing their own custom PHP snippets -- look at all the drupal_execute-related bugs I found in the issue queue, and many of them were in the D6 version.

I continually get drupal_execute bugs in Date and Calendar and when I say "don't use drupal_execute" they point to all kinds of articles that have written all over the place that recommend it as the "most reliable method" for doing things like creating fields and importing data. It's hard to overcome that.

We should write up some documentation on the use of the API, but who has time :(

Comment #5

yched CreditAttribution: yched commented 6 September 2008 at 21:42

Hmm - well, people are going to be even more surprised if/when #234774: Use Webform's UI for field addition lands and removes the old 'add field' / 'add group' forms :-/
Creating fields through drupal_execute() in D6 is sort of an heresy, which I'd happily declare 'unsupported', but it's true that the CRUD api kind of lacks a user doc.

Not only that, but every field module / widget would need to document their own specific properties in the field and widget definition arrays, dos and don'ts...

Can we really say more than 'play with the result of content_field_instance_read(),and start trial-and-error from there' ?
The output of content_field_instance_read() gives an example of a $field definition array to be fed to content_field_instance_create(), problem is it also comes with internal stuff ('db_storage', 'active', 'columns'...), that are confusing, and that content_field_instance_[create|update]() take care of filling anyway.
Maybe we need a wrapper content_field_instance_get() or something, that would remove the internal cruft ?

Well, I'm drifting away, this probably deserves a separate tasK...