MDL-50085 templates: Minor fixes from peer review

* rename get_filename to get_filepath
* phpdocs fixes
* improve formatting of exceptions
* lang string improvement

diff --git a/admin/tool/templatelibrary/classes/api.php b/admin/tool/templatelibrary/classes/api.php
index 13cfbfe..a2cf302 100644 (file)
--- a/admin/tool/templatelibrary/classes/api.php
+++ b/admin/tool/templatelibrary/classes/api.php
@@ -42,6 +42,7 @@ class api {
      *
      * @param string $component Filter the list to a single component.
      * @param string $search Search string to optionally filter the list of templates.
+     * @param string $themename The name of the current theme.
      * @return array[string] Where each template is in the form "component/templatename".
      */
     public static function list_templates($component = '', $search = '', $themename = '') {
@@ -50,7 +51,7 @@ class api {
         $templatedirs = array();
         $results = array();
 
-        if ($component != '') {
+        if ($component !== '') {
             // Just look at one component for templates.
             $dirs = mustache_template_finder::get_template_directories_for_component($component, $themename);
 

diff --git a/admin/tool/templatelibrary/lang/en/tool_templatelibrary.php b/admin/tool/templatelibrary/lang/en/tool_templatelibrary.php
index 1ef475e..8cc6736 100644 (file)
--- a/admin/tool/templatelibrary/lang/en/tool_templatelibrary.php
+++ b/admin/tool/templatelibrary/lang/en/tool_templatelibrary.php
@@ -24,7 +24,7 @@
 
 $string['all'] = 'All components';
 $string['component'] = 'Component';
-$string['coresubsystem'] = 'Core subsystem ({$a})';
+$string['coresubsystem'] = 'Subsystem ({$a})';
 $string['documentation'] = 'Documentation';
 $string['example'] = 'Example';
 $string['noresults'] = 'No results';

diff --git a/lib/classes/output/external.php b/lib/classes/output/external.php
index cd665fc..17b1ca7 100644 (file)
--- a/lib/classes/output/external.php
+++ b/lib/classes/output/external.php
@@ -88,7 +88,7 @@ class external extends external_api {
         $templatename = $component . '/' . $template;
 
         // Will throw exceptions if the template does not exist.
-        $filename = mustache_template_finder::get_template_filename($templatename, $themename);
+        $filename = mustache_template_finder::get_template_filepath($templatename, $themename);
         $templatestr = file_get_contents($filename);
 
         return $templatestr;

diff --git a/lib/classes/output/mustache_filesystem_loader.php b/lib/classes/output/mustache_filesystem_loader.php
index 2bba5b8..f90623c 100644 (file)
--- a/lib/classes/output/mustache_filesystem_loader.php
+++ b/lib/classes/output/mustache_filesystem_loader.php
@@ -44,14 +44,13 @@ class mustache_filesystem_loader extends \Mustache_Loader_FilesystemLoader {
 
     /**
      * Helper function for getting a Mustache template file name.
-     * Use the leading component to restrict us specific directories.
+     * Uses the leading component to restrict us specific directories.
      *
      * @param string $name
-     *
      * @return string Template file name
      */
     protected function getFileName($name) {
         // Call the Moodle template finder.
-        return mustache_template_finder::get_template_filename($name);
+        return mustache_template_finder::get_template_filepath($name);
     }
 }

diff --git a/lib/classes/output/mustache_template_finder.php b/lib/classes/output/mustache_template_finder.php
index 6278618..8939ded 100644 (file)
--- a/lib/classes/output/mustache_template_finder.php
+++ b/lib/classes/output/mustache_template_finder.php
@@ -41,8 +41,8 @@ class mustache_template_finder {
     /**
      * Helper function for getting a list of valid template directories for a specific component.
      *
-     * @param string $component
-     *
+     * @param string $component The component to search
+     * @param string $themename The current theme name
      * @return string[] List of valid directories for templates for this compoonent. Directories are not checked for existence.
      */
     public static function get_template_directories_for_component($component, $themename = '') {
@@ -61,7 +61,7 @@ class mustache_template_finder {
         $dirs = array();
         $compdirectory = core_component::get_component_directory($component);
         if (!$compdirectory) {
-            throw new coding_exception("Component was not valid:" . s($component));
+            throw new coding_exception("Component was not valid: " . s($component));
         }
 
         // Find the parent themes.
@@ -90,20 +90,20 @@ class mustache_template_finder {
      * Helper function for getting a filename for a template from the template name.
      *
      * @param string $name - This is the componentname/templatename combined.
-     *
+     * @param string $themename - This is the current theme name.
      * @return string
      */
-    public static function get_template_filename($name, $themename = '') {
+    public static function get_template_filepath($name, $themename = '') {
         global $CFG, $PAGE;
 
         if (strpos($name, '/') === false) {
             throw new coding_exception('Templates names must be specified as "componentname/templatename"' .
-                                       ' (' . $name . ' requested) ');
+                                       ' (' . s($name) . ' requested) ');
         }
         list($component, $templatename) = explode('/', $name, 2);
         $component = clean_param($component, PARAM_COMPONENT);
         if (strpos($templatename, '/') !== false) {
-            throw new coding_exception('Templates cannot be placed in sub directories (' . $name . ' requested)');
+            throw new coding_exception('Templates cannot be placed in sub directories (' . s($name) . ' requested)');
         }
 
         $dirs = self::get_template_directories_for_component($component, $themename);

diff --git a/lib/tests/mustache_template_finder_test.php b/lib/tests/mustache_template_finder_test.php
index 61d9a83..df3237e 100644 (file)
--- a/lib/tests/mustache_template_finder_test.php
+++ b/lib/tests/mustache_template_finder_test.php
@@ -80,10 +80,10 @@ class core_output_mustache_template_finder_testcase extends advanced_testcase {
         $dirs = mustache_template_finder::get_template_directories_for_component('octopus', 'clean');
     }
 
-    public function test_get_template_filename() {
+    public function test_get_template_filepath() {
         global $CFG;
 
-        $filename = mustache_template_finder::get_template_filename('core/pix_icon', 'clean');
+        $filename = mustache_template_finder::get_template_filepath('core/pix_icon', 'clean');
         $correct = $CFG->dirroot . '/lib/templates/pix_icon.mustache';
         $this->assertSame($correct, $filename);
     }
@@ -91,8 +91,8 @@ class core_output_mustache_template_finder_testcase extends advanced_testcase {
     /**
      * @expectedException moodle_exception
      */
-    public function test_invalid_get_template_filename() {
+    public function test_invalid_get_template_filepath() {
         // Test something invalid.
-        $dirs = mustache_template_finder::get_template_filename('core/octopus', 'clean');
+        $dirs = mustache_template_finder::get_template_filepath('core/octopus', 'clean');
     }
 }