Problem:

The theme registry names theme hooks with underscores, because that is what PHP will allow in a function name. But in creating template files for Drupal, we've been using dashes. We accomplish this with the core template by setting the 'file' in the registry hook to the proper name. Discovery, however, keys on the name of the hook, which contains underscores.

This patch modifies the discovery to convert underscores to dashes when discovering templates in the file system. ALL underscores will be converted.

I haven't had the time to test this yet.

CommentFileSizeAuthor
#8 dynamic_text.tar_.gz__1.txt10 KBmerlinofchaos
#7 template-underscores_5.patch1.74 KBmerlinofchaos
#5 template-underscores_4.patch1.43 KBdvessel
#1 template-underscores_3.patch1.41 KBmerlinofchaos
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
FileSize
template-underscores_3.patch1.41 KB

Here's the patch

dvessel’s picture

dvessel CreditAttribution: dvessel commented
Status: Needs review » Needs work

The patch had no effect. Tried with the new .tpl files. anything with an underscore registers. Dashes don't show up at all. Same as before.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

Crapzor!

dvessel’s picture

dvessel CreditAttribution: dvessel commented
Priority: Normal » Critical
dvessel’s picture

dvessel CreditAttribution: dvessel commented
Status: Needs work » Needs review
FileSize
template-underscores_4.patch1.43 KB

Here's a patch. Was due to how the variables were used.

dvessel’s picture

dvessel CreditAttribution: dvessel commented

To test this patch, place a template file from one of the modules directory into your theme. Make some small change so you know it's picking up the new file then rebuild the theme registry.

for example. copy modules/user/user-profile.tpl.php into garland. Reset the cache with drupal_rebuild_theme_registry(); by placing it inside template.php and observe.

Does it show your changes at example.com/user/1 ?

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
FileSize
template-underscores_5.patch1.74 KB

This patch wasn't properly translating patterns. This one should do that. Will provide test material in a followup.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented
FileSize
dynamic_text.tar_.gz__1.txt10 KB

This test includes a module and a theme; it's designed for testing the dynamic theme stuff. The theme contains overrides for a and c in the template, and d as a .tpl.php -- anything other than those should revert to the default.

The module contains the theme and a test page.

dvessel’s picture

dvessel CreditAttribution: dvessel commented
Status: Needs review » Reviewed & tested by the community

Thanks for the test files. It works now.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Reviewed & tested by the community » Needs review

Last thing to check before this gets committed is where does $info[pattern] come from and what could it contain?

+      $pattern = strtr($info['pattern'], '_', '-');
+      $matches = preg_grep('/^'. $pattern .'/', $patterns);

I mean, the underscore has no special meaning in preg expressions, but the dash can have special meaning, if used to define ranges (ie. [a_z] becomes [a-z] after the replacement, which modifies the meaning.

Although this sounds like a possibly minor problem, it would be nice to rule it out by knowing more about the pattern. Otherwise the patch looks sane to me.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

The pattern is set by hook_theme(). In general, I don't expect it to be a complex regex; the standard I've introduced (and documented) is to set patterns like this: 'views__' -- the only reason it's a regex, honestly, is in case developers want additional flexibility.

Since the possible characters in a function name is really narrow, i.e, alphanumeric plus underscore, it seems unlikely that we'll see ranges. I do understand your concern, though; I'm not sure how easily we can actually distinguish real dashes from dashes in a range. Perhaps that requires a regex. Ahh, the irony. Maybe I'll put a regex expert on this to cover that possibility.

merlinofchaos’s picture

merlinofchaos CreditAttribution: merlinofchaos commented

The more I think on it, the less likely that scenario seems, Goba; [ isn't a valid character in a function name, so it seems unlikely that we'll see some odd use of _ where translating into a - will break the pattern.

Gábor Hojtsy’s picture

Gábor Hojtsy
he/him
Primary language Hungarian
Location Hungary
CreditAttribution: Gábor Hojtsy commented
Status: Needs review » Fixed

Well, [] is only used to define the range and will not end up required in the string which it is matched with, it is just part of the range syntax. Anyway, I understand that this is a very small nitpick, and could agree that we will not meet this kind of misbehavior.

So: committed!

Anonymous’s picture

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