Non-phptemplate template files are not discovered

Well, you took the existing substr($template, 0, strpos($template, '.')); and wrapped it inside an if (strpos($template, '.') !== FALSE). How does this solve the problem? Who reviewed this?

It solves the problem perfectly. substr($template, 0, strpos($template, '.')); will return an empty string if there is no '.' in $template..

Now why would a template not have an extension?

djnz's patch appears correct to me. Allow me to 'splain.

PHPTemplate uses a tiered extension of .tpl.php -- however, the method we use to list files chops off the .php extension. This means we're left with a filename of 'foo.tpl' which won't match anything. To match, we have to chop off that .tpl.

djnz's patch is fixing the case where there is no extra extension at all. In his case, it will be 'foo.smarty'.

I believe this is right.

Addendum: We probably should not run strpos() multiple times, so we should use something like:

  if (($pos == strpos(...)) !== FALSE) {
    ...
  }

Though I guess strpos is probably fast enough, given our filenames won't be more than 32 characters long, likely, so maybe that optimization doesn't matter.

Thanks for the clarification merlinofchaos.

Thanks for the clarification. Now I understand and see that the documentation definitely needs improvement here. This is how it looks like, and how it confuses me.

+    // Chop off the extension if there is one. We do it this way because
+    // $template will have one extension chopped off, but there might be more
+    // than one, such as with .tpl.php
+    if (strpos($template, '.') !== FALSE) {
+      $template = substr($template, 0, strpos($template, '.'));
+    }

It says $template will have one extension chopped off, but it actually means it is already chopped off at this stage. So this is what confused us. What about?

+    // Chop off the remaining extensions if there are any. $template already
+    // has the rightmost extension removed, but there might still be more,
+    // such as with .tpl.php, which still has .tpl in $template at this point.
+    if (($pos = strpos($template, '.')) !== FALSE) {
+      $template = substr($template, 0, $pos);
+    }

Also included merlinofchaos' suggestion here (not tested). I think this comment covers what we discussed here way better. Opinions?

Hmm. Technically the 'will have' is correct English, but 'have' is one of those words with too many meanings. The funny thing is, every time I try to rewrite it in my head I turn back to that phrase. May be a personal overuse.

Earnie: I think it's good to note that the last extension is chopped off already. Gabor's version of the comment is good by me and makes plenty of sense.

Honestly my problem with 'will have' is that I don't know whether it 'will have' before the code block is run (ie. it is not yet in that form) is 'will have' before the code block is run (ie. it is already in that format). Technically when one reads the code, the code 'will' run, so this might look like ok, but I think is confusing. At least for a non-native English speaker like me.

looks good. tink this is ready to go.

Thanks guys, committed.