I realize that overriding the forum path AND theme in the module makes for what feels like a much more elegant module, but in doing so the forums module no longer behaves as expected. I'd suggest if the code is going to overide the forum path call that rather than calling theme('og_forum_display) it calls the regular forum_display and has a phptemplate_forum_display for the template.php.

This way, when someone (perhaps someone other than the person who did the install) is looking to style the forum theme, they will be able to easily find the code they need to modify in order to theme it.

Either that or please prominently announce that the code is overriding core drupal behavior on the module page and in the README.txt -- this will be very useful to developers!

Thanks for all your hard work on the og_forums module.

Comments

rconstantine’s picture

rconstantine commented

Are there any examples of what you're talking about out there that I could look at? I'm not totally sure what you mean, but will look into it.

ragaskar’s picture

ragaskar commented

Sorry, that was a little unclear. Here's my point: as things stand, without doing a bit of detective work, it isn't obvious how to theme the forums when the og_forums module is added.

A lot of other modules include template.php snippets and instructions to "add this to your template.php file" in cases where they are overriding core theme behaviors. In your case, you would provide a 'phptemplate_display_forums()' function that users could paste into their template.php.

Another path to making it obvious would be to add a prominent mention "This module is overriding the forums path and uses theme_og_display_forums rather than theme_display_forums for output".

Lastly I would recommend looking into the db_rewrite_sql hook -- i ended up rewriting group functionality because the og module did not meet my needs and I found that this hook is very useful for controlling output in places you can't reach with nodeapi.

rconstantine’s picture

rconstantine commented
Assigned: Unassigned » rconstantine
Status: Active » Fixed

Okay, I understand where you're coming from. For now, I'm updating the README. I may actually rewrite the README because it doesn't say much at all.

Anyway, technically speaking, I'm not overriding the theme functions. I'm overriding the forum menu item to point to og_forum_page instead of forum_page and then my theme functions simply belong to og_forum_page as they should. So I doubt I'll go the route of having users insert these functions into their template.php files since they don't actually belong to any forum module function.

As for the db_rewrite_sql hook, to what do you refer? There is an implementation of this function and some pretty clever SQL (if I do say so myself) for showing only those forums of groups to which a user belongs, plus all site-wide forums. Is there something else you think I could use this hook for? I'll be adding more to this function to allow forums to be marked as publicly viewable which will trick things out even more.

Anyway, thanks for the suggestions.

Anonymous’s picture

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