API: Look for templates of namespaced classes in subfolders.

This change will mean that SilverStripe\Control\Controller will look for its
template in templates/SilverStripe/Control/Controller.ss.

In order to preserve some backwards campatibility, non-namespaced classes
can have the templates stored in any template subfolder, but once you
add a namespace to a class, the namespaced path expression will need to
be a subfolder of <module>/templates or themes/<theme>/templates.

Layout and Content templates are stil supported as special template type,
Includes still functions but is a no-op. Other template subfolders should
not be used.

core/manifest/TemplateManifest.php

@@ -178,34 +178,86 @@ class SS_TemplateManifest {
 		$this->inited = true;
 	}
 
-	public function handleFile($basename, $pathname, $depth) {
+	public function handleFile($basename, $pathname, $depth)
+	{
 		$projectFile = false;
 		$theme = null;
 
-		if (strpos($pathname, $this->base . '/' . THEMES_DIR) === 0) {
-			$start = strlen($this->base . '/' . THEMES_DIR) + 1;
-			$theme = substr($pathname, $start);
-			$theme = substr($theme, 0, strpos($theme, '/'));
-			$theme = strtok($theme, '_');
-		} else if($this->project && (strpos($pathname, $this->base . '/' . $this->project .'/') === 0)) {
+		// Template in theme
+		if (preg_match(
+			'#'.preg_quote($this->base.'/'.THEMES_DIR).'/([^/_]+)(_[^/]+)?/(.*)$#',
+			$pathname,
+			$matches
+		)) {
+			$theme = $matches[1];
+			$relPath = $matches[3];
+
+		// Template in project
+		} elseif (preg_match(
+			'#'.preg_quote($this->base.'/'.$this->project).'/(.*)$#',
+			$pathname,
+			$matches
+		)) {
 			$projectFile = true;
+			$relPath = $matches[1];
+
+		// Template in module
+		} elseif (preg_match(
+			'#'.preg_quote($this->base).'/([^/]+)/(.*)$#',
+			$pathname,
+			$matches
+		)) {
+			$relPath = $matches[2];
+
+		} else {
+			throw new \LogicException("Can't determine meaning of path: $pathname");
 		}
 
-		$type = basename(dirname($pathname));
-		$name = strtolower(substr($basename, 0, -3));
-
-		if ($type == self::TEMPLATES_DIR) {
-			$type = 'main';
+		// If a templates subfolder is used, ignore that
+		if (preg_match('#'.preg_quote(self::TEMPLATES_DIR).'/(.*)$#', $relPath, $matches)) {
+			$relPath = $matches[1];
 		}
 
+		// Layout and Content folders have special meaning
+		if (preg_match('#^(.*/)?(Layout|Content|Includes)/([^/]+)$#', $relPath, $matches)) {
+			$type = $matches[2];
+			$relPath = "$matches[1]$matches[3]";
+		} else {
+			$type = "main";
+		}
+
+		$name = strtolower(substr($relPath, 0, -3));
+		$name = str_replace('/', '\\', $name);
+
 		if ($theme) {
 			$this->templates[$name]['themes'][$theme][$type] = $pathname;
-		} else if($projectFile) {
+		} else if ($projectFile) {
 			$this->templates[$name][$this->project][$type] = $pathname;
 		} else {
 			$this->templates[$name][$type] = $pathname;
 		}
 
+		// If we've found a template in a subdirectory, then allow its use for a non-namespaced class
+		// as well. This was a common SilverStripe 3 approach, where templates were placed into
+		// subfolders to suit the whim of the developer.
+		if (strpos($name, '\\') !== false) {
+			$name2 = substr($name, strrpos($name, '\\') + 1);
+			// In of these cases, the template will only be provided if it isn't already set. This
+			// matches SilverStripe 3 prioritisation.
+			if ($theme) {
+				if (!isset($this->templates[$name2]['themes'][$theme][$type])) {
+					$this->templates[$name2]['themes'][$theme][$type] = $pathname;
+				}
+			} else if ($projectFile) {
+				if (!isset($this->templates[$name2][$this->project][$type])) {
+					$this->templates[$name2][$this->project][$type] = $pathname;
+				}
+			} else {
+				if (!isset($this->templates[$name2][$type])) {
+					$this->templates[$name2][$type] = $pathname;
+				}
+			}
+		}
 	}
 
 	protected function init() {

tests/core/manifest/TemplateManifestTest.php

@@ -25,7 +25,7 @@ class TemplateManifestTest extends SapphireTest {
 	public function testGetTemplates() {
 		$expect = array(
 			'root' => array(
-				'module' => "{$this->base}/module/Root.ss"
+				'main' => "{$this->base}/module/Root.ss"
 			),
 			'page' => array(
 				'main'   => "{$this->base}/module/templates/Page.ss",
@@ -54,6 +54,30 @@ class TemplateManifestTest extends SapphireTest {
 					'theme' => array('main' => "{$this->base}/themes/theme/templates/CustomThemePage.ss",)
 				)
 			),
+			'mynamespace\myclass' => array(
+				'main' => "{$this->base}/module/templates/MyNamespace/MyClass.ss",
+				'Layout' => "{$this->base}/module/templates/MyNamespace/Layout/MyClass.ss",
+				'themes' => array(
+					'theme' => array(
+						'main' => "{$this->base}/themes/theme/templates/MyNamespace/MyClass.ss",
+					)
+				),
+			),
+			'mynamespace\mysubnamespace\mysubclass' => array(
+				'main' => "{$this->base}/module/templates/MyNamespace/MySubnamespace/MySubclass.ss",
+			),
+			'myclass' => array(
+				'main' => "{$this->base}/module/templates/MyNamespace/MyClass.ss",
+				'Layout' => "{$this->base}/module/templates/MyNamespace/Layout/MyClass.ss",
+				'themes' => array(
+					'theme' => array(
+						'main' => "{$this->base}/themes/theme/templates/MyNamespace/MyClass.ss",
+					)
+				),
+			),
+			'mysubclass' => array(
+				'main' => "{$this->base}/module/templates/MyNamespace/MySubnamespace/MySubclass.ss",
+			),
 			'include' => array('themes' => array(
 				'theme' => array(
 					'Includes' => "{$this->base}/themes/theme/templates/Includes/Include.ss"

tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/Layout/MyClass.ss

@@ -0,0 +1 @@
+MyClass.ss

tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/MyClass.ss

@@ -0,0 +1 @@
+MyClass.ss

tests/core/manifest/fixtures/templatemanifest/module/templates/MyNamespace/MySubnamespace/MySubclass.ss

@@ -0,0 +1 @@
+MySubclass.ss

tests/view/SSViewerTest.php

@@ -1124,24 +1124,24 @@ after')
 		$self = $this;
 		$this->useTestTheme(dirname(__FILE__), 'layouttest', function() use ($self) {
 			// Test passing a string
-			$templates = SSViewer::get_templates_by_class('SSViewerTest_Controller', '', 'Controller');
-			$self->assertCount(2, $templates);
+			$templates = SSViewer::get_templates_by_class(
+				'TestNamespace\SSViewerTest_Controller',
+				'',
+				'Controller'
+			);
+			$self->assertEquals([
+				'TestNamespace\SSViewerTest_Controller',
+    			'Controller',
+			], $templates);
 
 			// Test to ensure we're stopping at the base class.
-			$templates = SSViewer::get_templates_by_class('SSViewerTest_Controller', '', 'SSViewerTest_Controller');
+			$templates = SSViewer::get_templates_by_class('TestNamespace\SSViewerTest_Controller', '', 'TestNamespace\SSViewerTest_Controller');
 			$self->assertCount(1, $templates);
 
 			// Make sure we can filter our templates by suffix.
 			$templates = SSViewer::get_templates_by_class('SSViewerTest', '_Controller');
 			$self->assertCount(1, $templates);
 
-			// Test passing a valid object
-			$templates = SSViewer::get_templates_by_class("SSViewerTest_Controller", '', 'Controller');
-
-			// Test that templates are returned in the correct order
-			$self->assertEquals('SSViewerTest_Controller', array_shift($templates));
-			$self->assertEquals('Controller', array_shift($templates));
-
 			// Let's throw something random in there.
 			$self->setExpectedException('InvalidArgumentException');
 			$templates = SSViewer::get_templates_by_class(array());
@@ -1593,11 +1593,6 @@ class SSViewerTest_ViewableData extends ViewableData implements TestOnly {
 	}
 }
 
-
-class SSViewerTest_Controller extends Controller {
-
-}
-
 class SSViewerTest_Object extends DataObject implements TestOnly {
 
 	public $number = null;
@@ -1687,4 +1682,3 @@ class SSViewerTest_LevelTest extends ViewableData implements TestOnly {
 		return new self($number);
 	}
 }
-

tests/view/TestNamespace/SSViewerTest_Controller.php

@@ -0,0 +1,8 @@
+<?php
+
+namespace TestNamespace;
+
+class SSViewerTest_Controller extends \Controller
+{
+
+}