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.

CommentFileSizeAuthor
#1 content_admin_cleanup.patch21.91 KByched

Comments

yched’s picture

yched commented
Status: Active » Fixed
StatusFileSize
new content_admin_cleanup.patch21.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.

karens’s picture

karens commented
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.

yched’s picture

yched commented

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

karens’s picture

karens commented

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 :(

yched’s picture

yched commented

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...

yched’s picture

yched commented

I added a note in CHANGELOG for now.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.