drupal_placeholder() takes an array as an argument, which is weird. One would expect it to take a string as an argument. Furthermore, there seems to be no reason to use an array, since it seems only one element of the array is ever used. Furthermore, at every place the function is called, the array is specifically created for that function, and seems to serve no other purpose. I realize that this is rather late for an API change, but in a function like t() that may be called thousands of times on a page load, we cannot really afford to have inefficiencies. Plus, it'll make for a much cleaner API.

I'll have a patch for this tonight.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cwgordon7’s picture

Status: Active » Needs review
FileSize
6.88 KB

And here's the patch.

Status: Needs review » Needs work

The last submitted patch, drupal_placeholder_01.patch, failed testing.

cwgordon7’s picture

Status: Needs work » Needs review
FileSize
6.96 KB
chx’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +DrupalWTF, +API change

I bet this was just theme_placeholder ported without thinking.

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Certainly a major WTF. Committed to CVS HEAD.

rfay’s picture

I assume this needs to be announced. Or is it unlikely that anybody would use this directly?

Status: Fixed » Closed (fixed)
Issue tags: -DrupalWTF, -API change

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