Let Drupal templates use single filename extension

Either you remove the 'pattern' test, or you include the nyan cat theme engine to this patch, but as I wrote them one cannot live without the other.

Seems fair enough.

It's very weird, lots of errors seems unrelated. Did you test the patch locally?

Just run the "theme" group, it is quick, if it works, then it's probably unrelated. It does not have any possible side effects outside of theming, so if the "group" test passes, it should pass globally.

Very nice, it is still missing one test case: right now we are testing with a pattern that is completely different from the theme hook name, hence the #theme => ['first_once', 'second_one'] which is actually an edge case (even more a side effect of the current algorithm) test, I'm going to add a more "normal" use case where the pattern starts with the theme hook name, which actually would be the real life use case.

In the meantime, the actual patch is RTBC.

If anyone does asks, the line that replaces the strtr() call using preg_quote() was actually not necessary, it just seemed more natural and safer to do this. The original strtr() would allow patterns to use special preg escade characters but in the end, I'm not sure this would be a good thing. In the end I'm even not sure keeping the 'pattern' feature is a good thing.

I renamed test templates names to have a logical suite. Also added more boundary tests, to make it more robust. Only tests are altered theme.inc code remains unchanged.

Thank you very much Pol, you have more patience than me and I really appreciate that.

Hum my comment about the theme_hook_suggestion is wrong, it is actually documented, but bugguy, I should fix it.

Comments fixed, everything should be good now.

This one looks solid to me as well. Would be awesome if this could be committed before the 7.52 window is closed.

and theme
+  // engines extensions may contain as many dot-separated segment as they with
+  // (e.g. '.tpl.php', '.html.twig', '.phtml', ...).

The grammar here seems a bit wonky (can be fixed on commit)

@stefan.r since I'm not a native english speaker, please give us something else we can write in there :)

So far I checked that this seems to be fixed in Drupal 8 already, so that is good.

Fabianx yes I did look in Drupal 8 and had the same conclusion, just forgot to mention it here, sorry.

About the preg_quote(), did ever even saw a single project using the 'pattern' feature ? By quoting it might quote preg special chars that users might have voluntarily used (although see this actually working would definitely be the second most mystical thing among the universe after the universe itself).

Good :)

Are we going to have this in Drupal core?

@RajabNatshah I think so, the patch looks fine. Assigning to David for further review.

Perhaps I'm missing something, but I was unable to reproduce this bug in Drupal 7.

For example, see the attached patch (single-filename-extension-do-not-test.patch). Trying that manually and clearing caches, everything with the Seven theme worked fine even though I made it use .php2 as the extension rather than .tpl.php.

The tests included in this issue also don't seem to test that bug - not to say they aren't useful tests in general, but when I ran them without the rest of the patch, they still passed. I am attaching a tests-only version of the patch here just to make sure the same thing happens with the testbot.

As for this part:

-  $regex = '/' . str_replace('.', '\.', $extension) . '$/';
+  $regex = '/' . preg_quote($extension, '/') . '$/';

It definitely looks like a good change, but I think it needs to go in Drupal 8 also.

Hum, fun thing, did you test with a git version ? Considering that we applied a series of patch related to theming not so long ago, it might have been fixed along the way. I happened to experience the bug, can't remember the exact circumstances.