GHOP #45: Document all theme functions as themeable

Looks like there's still Windows line endings in the patch. Those need to be removed. :-) Otherwise it applies cleanly, albeit with a few offsets.

For theme_button(), theme_submit(), and theme_image_button(), I'd suggest saying "Theme a form submit button" (etc.). Make it clear that it's a form element being themed.

You added a docblock to theme_progress_bar(), but it's missing @ingroup themeable.

I don't know if there is a formal standard for this, but I tend to put @ingroup directives above the parameter and return directives, without an extra newline.

If those functions cannot be overridden by themers, then I'd say they shouldn't be in the themeable group.

Marking needs review again.

There was one failure when applying the patch. Specifically in theme.inc, for theme_maintenance_page() and theme_install_page(). I believe another patch just went in that updated those, so this patch will need to be updated ("rerolled" as the locals say). Unfortunately large patches tend to do that sort of thing. :-/

Kudos for tackling such a big patch -- Drupal 6 is a bit of a moving target since it is still in active development, and as Crell mentioned, there are new patches going in all of the time. One way to quickly check the latest CVS commits for Drupal is to check out this page: http://drupal.org/project/cvs/3060

And taking a quick look at the patch, I noticed that you're patching against version number 1.397 of theme.inc, and it has changed a few times as you can see here:
http://cvs.drupal.org/viewvc.py/drupal/drupal/includes/theme.inc?view=lo...

They're up to version 1.400 now.

Anyway, like Crell said, the patch needs to be "re-rolled" using the most recent changes.

I will test this tonight if someone doesn't beat me to it. (Feel free to do so.)

#6 still had Windows line feed characters. I don't know how to roll a patch in Windows that doesn't have them, unfortunately, but it's a good habit to get into to avoid them.

theme_task_list() in includes/theme.maintenance.inc is missing an @ingroup tag, and is not marked as unthemeable. I suspect it was just added when the maintenance themeability patch landed. It should either be marked @ingroup themeable or noted that it is unthemeable, as appropriate.

theme_color_scheme_form() in color.module is missing an @ingroup tag.

There are a whole bunch of theme functions in comment.module that are missing @ingroup tags.

There's more, too. :-( Try searching all files for the string "function theme_". That's probably the easiest way to find candidate themeable functions. You can do it file-at-a-time, but I'd recommend using an editor that can search all of Drupal at once. It will be a lot easier.

Or you can grep from a terminal window. But it looks like you're on windows, and so it might be a bit different.
But if you can get a unix window, then this grep should also work:
grep "function theme_" -r * > x.txt;
That'll output the results to a file named x.txt.

Awesome, thanks for the follow-through, fberci. :) Keep on rockin'!

Thanks, fberci! Yeah, looks like it was .module files that got missed.

However, I'd say that theme_syslog_format() should still be flagged as themeable. It might be rather foolish of someone to try overriding it, but it's still possible so it should be flagged accordingly. It also needs a description in its docblock either way.

Applies with a few offsets, no Windows line endings. (yay!)

theme_get_registry(), theme_get_settings(), theme_get_setting(), theme_render_template() in theme.inc are actually not themeable. They're part of the theme API internals. Yes, that's a very confusing name. :-( (One of the reasons for the @ingroup themeable is to differentiate which functions really are themeable.)

I've rerolled the patch to just remove those 3, so this is now RTBC. No sense holding it up for something that trivial. Thanks for keeping on it!

(Committers: Please don't credit me.)

Looks good, thanks. Committed!