driven_diff_summary_table inside driven_diff should be a theme function registered through the normal means and accessed using theme(). That way, others can access the variables using MODULE_preprocess_HOOK() before they are fully themed.

This is necessary for me as I am trying to modify how the table at the top of comments (showing the diff) is rendered.

This is a problem throughout driven and comment_driven, but I don't have other specific instances that I can mention at the moment.

Thanks for your hard work on this important module! :)

CommentFileSizeAuthor
#2 874238_summary_table_theme.patch1.91 KBAnonymous (not verified)

Comments

arhak’s picture

arhak commented

would you propose a patch?

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new 874238_summary_table_theme.patch1.91 KB

Here is my attempt at a patch. (It's against CVS).

Basically, it adds a theme function between the $rows and the summary table. This allows other functions to register themselves as preprocess functions. And it makes it easier for themers to add functions or templates.

This shouldn't interfere with any existing code.

arhak’s picture

arhak commented

did you test it?
'arguments' => array('variables' => array('rows' => array())), <<-- this looks like D7

Anonymous’s picture

Anonymous (not verified) commented

I did test it. I agree it looks like D7, but I was following the form that you established in the existing D6 code. If you download the dev copy of D6, it has the same format for all the theme declarations. Maybe that is another patch that needs to be submitted if you want that to fit the standard D6 array format.

arhak’s picture

arhak commented
Status: Active » Reviewed & tested by the community

yes, it should work in D6 anyway
and you're right, I was using D7 params, but I was also using function driven_diff_output($theme, $variables) as a D6 proxy...

nevermind, your patch is valid

Anonymous’s picture

Anonymous (not verified) commented

Great, thanks. Do you know when it will be available as part of a stable release? I need to re-architecture my module once this is available as part of a stable release.

arhak’s picture

arhak commented
Component: Code » Theming

not sure about when
I have 4 issues landed, perhaps soon

just curious: why would you have to "re-architecture" your module then?

Anonymous’s picture

Anonymous (not verified) commented

"re-architecture" might have been a little exagerated. But basically I have to do a ton of hacks deep into the theme system to be able to grab $rows before it is output. My module needs to grab $rows and could do it with far fewer hacks if this theme function existed.

Thanks. I'll keep an eye out for it to be committed. Thanks.

eiland’s picture

eiland commented

is this basically the same issue as #1472904: Altering the reply form??