From 410034eef0402a6aacb109e7074fab4c7331a5f8 Mon Sep 17 00:00:00 2001 From: Damyon Wiese Date: Mon, 4 May 2015 14:18:42 +0800 Subject: [PATCH] MDL-50085 templates: Minor fixes from peer review * rename get_filename to get_filepath * phpdocs fixes * improve formatting of exceptions * lang string improvement --- admin/tool/templatelibrary/classes/api.php | 3 ++- .../lang/en/tool_templatelibrary.php | 2 +- lib/classes/output/external.php | 2 +- lib/classes/output/mustache_filesystem_loader.php | 5 ++--- lib/classes/output/mustache_template_finder.php | 14 +++++++------- lib/tests/mustache_template_finder_test.php | 8 ++++---- 6 files changed, 17 insertions(+), 17 deletions(-) diff --git a/admin/tool/templatelibrary/classes/api.php b/admin/tool/templatelibrary/classes/api.php index 13cfbfebea7..a2cf3027dd9 100644 --- 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 1ef475e1e78..8cc67365e75 100644 --- 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 cd665fc1f22..17b1ca74712 100644 --- 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 2bba5b8a1ab..f90623ce47a 100644 --- 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 627861868be..8939ded720d 100644 --- 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 61d9a832b2e..df3237e9ed5 100644 --- 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'); } }