I need to wrap "No pending revisions found." in a paragraph.

<p>No pending revisions found.</p>

I see two options

1. Change the code to
return '<p>'. t('No pending revisions found.') .'</p>';

2. Convert revision_moderation_pending_revisions_admin() into a theme function
function theme_revision_moderation_pending_revisions_admin()

2 seems like the right solution, but I'm not sure if it's overkill. What would be the best way to do this? I can create the patch.

Thanks

Comments

webchick’s picture

Category: support » bug

Oh, yuck. Yes, that should totally be a theme function. If you have time to whip up a patch, that would be awesome.

webchick’s picture

Oh, and let's wrap that t() in p tags while we're at it.

So the answer is: "1 and 2, please" ;)

jbjaaz’s picture

Status: Active » Needs review
StatusFileSize
new1.34 KB

Here is the patch.

Is it okay to call a theme function from a menu item or is it better to have the menu point to a function which then points to the theme function?

'callback' => 'theme',
'callback arguments' => array('revision_moderation_pending_revisions_admin'),

vs

'callback' => 'revision_moderation_pending_revisions_admin',
...

function revision_moderation_pending_revisions_admin() {
  return theme('revision_moderation_pending_revisions_admin');
}

function theme_revision_moderation_pending_revisions_admin() {
  ...
}
webchick’s picture

Status: Needs review » Needs work

In a quick grep through core, it looks like there's always an interim function. So yeah, let's do the second option you listed there.

jbjaaz’s picture

Got it.

I also made a quick tweak to the table column that prints out the node type. I updated it to display the human readable version of the node type instead of the internal name, eg. "Job posting" instead of "job_posting". I borrowed the code from node.module.

jbjaaz’s picture

Status: Needs work » Needs review

whoops, forgot to update the status

toemaz’s picture

I rerolled the patch against the latest dev version. Find new patch attached.
I reviewed it and it looks fine to me. Yet I'm not setting it on RTBC because I didn't test it yet.

add1sun’s picture

That last patch is missing the p tags around the t(). I added that locally (and capitalized the comment) and everything seems hunky dory. New patch attached. If someone else can test, we can go ahead and commit this.

add1sun’s picture

Status: Needs review » Fixed

committed to 5 and 6. 6 also needed a hook_theme for the theme function, which I added.

Status: Fixed » Closed (fixed)

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