I'd like to theme the forms generated by CCK. This should be as easy as making a theme_my_cck_form function, right? No, because of the convention adopted of using a '-' in the content type name; - isn't a legal function name character. This patch changes the - to a _ so that theme functions for the forms can be written. There are probably lots of issues that I'm not aware of, so this definitely needs careful consideration.

CommentFileSizeAuthor
cck_content.patch3.18 KBrobertDouglass
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

KarenS’s picture

Changing the content type names will break all kinds of things, including any customized templates that are already in use. IMO a better way would be to grab the $form element in the content_form function and push it through a theme before returning it. That theme can be given whatever name is needed so it can be customized.

robertDouglass’s picture

Karen, that would be an acceptable hack, but I still consider it a bug that you can't theme normal Drupal forms of whatever sort. Yes, all sorts of things will break, but as far as I know, this module is still in beta and hasn't reached an official release. If we don't fix this now, we never will. Lots of support can be given in terms of upgrade scripts and I'll help out with that if we can agree to fix this. If we run all forms through a theme function that people can override, the override will have to start with a bunch of string comparisons to get the right form and then pawn the form off to new theme functions, while changing four '-' characters to '_' characters in the code avoids all this and makes Drupal's forms api work the way it was designed to.

One last point; if Adrian comes through and makes every theme function its own file, and further separates the data/presentation logic in the current forms api, we don't want CCK to miss out on all of that for its forms just because we chose to protect people from having to update their current websites and data.

yched’s picture

I guess Robert has a point...

This is not a proper patch review though, I need my data for now :-(

robertDouglass’s picture

The data is easy to update. For every CCK content type, you have to:

UPDATE node SET type = 'content_yourtype' WHERE type = 'content-yourtype';
UPDATE node_field_instance SET type_name = 'content_yourtype' WHERE type_name = 'content-yourtype';
UPDATE node_type SET type_name = 'content_yourtype' WHERE type_name = 'content-yourtype';

So it's not about losing data. Anybody who has built custom CCK themes will have to replace '-' with '_' in their themes.

pbarnett’s picture

As a Drupal newbie but a programmer for more years than I like to think about, Robert's solution seems to eliminate cruft, rather than generate more... or am I missing something?

Pete.

marcoBauli’s picture

ok, as a NON-programmer and Drupal user i think the themability of CCK submission forms is fundamental feature.

The updates i read above should be easy enough to perform by anyone who knows how to use the 'find and replace' function of a text editor and a tool like phpmyAdmin.

..or am i missing something too?

KarenS’s picture

I'm no expert on themeing, so if that's a solution that fits the themeing criteria better, I have no objections on that basis. I can't test the patch because I already have heavily patched versions of many cck modules and I can't afford to change the database structure without knowing for sure that the change is going to be committed, so I'm in a catch-22.

The patch may need to include changes to the install file to make the database updates automatic. That's the way previous structural changes have been handled.

If this is applied, there will also need to be changes made to the internal themeing documentation and to the documentation posted in the handbook on drupal.org. It may also require changes in some of the many patches in the patch queue.

robertDouglass’s picture

KarenS: yup. Lot's of work. JonBob, give it your blessing and I'll do everything Karen described.

KarenS’s picture

Just in case it wasn't clear, I wasn't trying to imply you didn't know what needed to be done :-) I was just trying to bring up the things that I knew of that would need adjustment while it was fresh in my mind.

robertDouglass’s picture

KarenS: No worries =)

I always try to read things in the friendliest of ways.

Zach Harkey’s picture

+1 This is the way it should be, and there will not ever be a better time to fix it.

Patrick Nelson’s picture

+1 from me too. CCK needs the ability to be able to theme each individual content type.

KarenS’s picture

Just bumping this issue to keep it close to the top of the list since there are going to be so many other things affected by this change. Better to get it done earlier than later I think :-)

JonBob’s picture

Status: Needs work » Fixed

This change has been applied. When the CCK project began, - was the mandated separator for multiple node types in the same module; this requirement seems to be no longer relevant.

Users of views.module may have to re-save their views after this update.

Users with custom templates for node types will have to rename those templates to match the new convention.

Anonymous’s picture

Status: Fixed » Closed (fixed)