From 332521a3696df8e1504933bee5658d005549f3b4 Mon Sep 17 00:00:00 2001 From: Chris Smith Date: Mon, 24 Nov 2008 21:59:33 +0000 Subject: [PATCH] - Make a start on completing the phpDoc comments for the template engine - Tidy template engine code, mainly PHP5 stuff, made some methods void instead of just returning true - Add tests for the remaining untested compilation code git-svn-id: file:///svn/phpbb/trunk@9115 89ea8834-ac86-4346-8a33-228a782c2dd0 --- phpBB/includes/functions_template.php | 66 ++++++++++++++++---- phpBB/includes/template.php | 74 ++++++++++++++++------- tests/template/template.php | 50 ++++++++++++++- tests/template/templates/basic.html | 2 + tests/template/templates/define.html | 24 ++++++++ tests/template/templates/expressions.html | 4 +- tests/template/templates/lang.html | 3 + tests/template/templates/loop.html | 8 +++ 8 files changed, 192 insertions(+), 39 deletions(-) create mode 100644 tests/template/templates/lang.html diff --git a/phpBB/includes/functions_template.php b/phpBB/includes/functions_template.php index 8672c00d50..c8c79ab5e4 100644 --- a/phpBB/includes/functions_template.php +++ b/phpBB/includes/functions_template.php @@ -19,19 +19,32 @@ if (!defined('IN_PHPBB')) /** * The template filter that does the actual compilation * @see template_compile + * @package phpBB3 * */ class template_filter extends php_user_filter { + /** + * @var string Replaceable tokens regex + */ private $regex = '~|{((?:[a-z][a-z_0-9]+\.)*\\$?[A-Z][A-Z_0-9]+)}~'; - private $blocks = array(); + /** + * @var array + */ private $block_names = array(); + + /** + * @var array + */ private $block_else_level = array(); + /** + * @var string + */ private $chunk; - function filter($in, $out, &$consumed, $closing) + public function filter($in, $out, &$consumed, $closing) { $written = false; @@ -203,6 +216,12 @@ class template_filter extends php_user_filter $no_nesting = false; // Is the designer wanting to call another loop in a loop? + // + // + // + // + // 'loop2' is actually on the same nesting level as 'loop' you assign + // variables to it with template->assign_block_vars('loop2', array(...)) if (strpos($tag_args, '!') === 0) { // Count the number if ! occurrences (not allowed in vars) @@ -605,10 +624,15 @@ class template_filter extends php_user_filter * block namespace. This is a string of the form: * ' . $_tpldata['parent'][$_parent_i]['$child1'][$_child1_i]['$child2'][$_child2_i]...['varname'] . ' * It's ready to be inserted into an "echo" line in one of the templates. - * NOTE: expects a trailing "." on the namespace. + * * @access private + * @param string $namespace Namespace to access (expects a trailing "." on the namespace) + * @param string $varname Variable name to use + * @param bool $echo If true return an echo statement, otherwise a reference to the internal variable + * @param bool $defop If true this is a variable created with the DEFINE construct, otherwise template variable + * @return string Code to access variable or echo it if $echo is true */ - function generate_block_varref($namespace, $varname, $echo = true, $defop = false) + private function generate_block_varref($namespace, $varname, $echo = true, $defop = false) { // Strip the trailing period. $namespace = substr($namespace, 0, -1); @@ -653,11 +677,13 @@ class template_filter extends php_user_filter * (possibly nested) block namespace. This is a string of the form: * $_tpldata['parent'][$_parent_i]['$child1'][$_child1_i]['$child2'][$_child2_i]...['$childN'] * - * If $include_last_iterator is true, then [$_childN_i] will be appended to the form shown above. - * NOTE: does not expect a trailing "." on the blockname. * @access private + * @param string $blockname Block to access (does not expect a trailing "." on the blockname) + * @param bool $include_last_iterator If $include_last_iterator is true, then [$_childN_i] will be appended to the form shown above. + * @param bool $defop If true this is a variable created with the DEFINE construct, otherwise template variable + * @return string Code to access variable */ - function generate_block_data_ref($blockname, $include_last_iterator, $defop = false) + private function generate_block_data_ref($blockname, $include_last_iterator, $defop = false) { // Get an array of the blocks involved. $blocks = explode('.', $blockname); @@ -712,14 +738,18 @@ stream_filter_register('template', 'template_filter'); * DEFINE directive inspired by a request by Cyberalien * * @package phpBB3 +* @uses template_filter As a PHP stream filter to perform compilation of templates */ class template_compile { - + /** + * @var template Reference to the {@link template template} object performing compilation + */ private $template; /** * Constructor + * @param template $template {@link template Template} object performing compilation */ function __construct(template $template) { @@ -729,8 +759,10 @@ class template_compile /** * Load template source from file * @access public + * @param string $handle Template handle we wish to load + * @return bool Return true on success otherwise false */ - public function _tpl_load_file($handle/*, $store_in_db = false*/) + public function _tpl_load_file($handle) { // Try and open template for read if (!file_exists($this->template->files[$handle])) @@ -740,9 +772,14 @@ class template_compile // Actually compile the code now. return $this->compile_write($handle, $this->template->files[$handle]); - } + /** + * Load template source from file + * @access public + * @param string $handle Template handle we wish to compile + * @return string|bool Return compiled code on successful compilation otherwise false + */ public function _tpl_gen_src($handle) { // Try and open template for read @@ -752,12 +789,15 @@ class template_compile } // Actually compile the code now. - return $this->compile_gen(/*$handle, */$this->template->files[$handle]); + return $this->compile_gen($this->template->files[$handle]); } /** * Write compiled file to cache directory * @access private + * @param string $handle Template handle to compile + * @param string $source_file Source template file + * @return bool Return true on success otherwise false */ private function compile_write($handle, $source_file) { @@ -788,8 +828,10 @@ class template_compile /** * Generate source for eval() * @access private + * @param string $source_file Source template file + * @return string|bool Return compiled code on successful compilation otherwise false */ - private function compile_gen(/*$handle, */$source_file) + private function compile_gen($source_file) { $source_handle = @fopen($source_file, 'rb'); $destination_handle = @fopen('php://temp' ,'r+b'); diff --git a/phpBB/includes/template.php b/phpBB/includes/template.php index 25f2fa1cad..953cc984c0 100644 --- a/phpBB/includes/template.php +++ b/phpBB/includes/template.php @@ -22,19 +22,39 @@ if (!defined('IN_PHPBB')) */ class template { - /** variable that holds all the data we'll be substituting into + /** + * variable that holds all the data we'll be substituting into * the compiled templates. Takes form: * --> $this->_tpldata[block][iteration#][child][iteration#][child2][iteration#][variablename] == value * if it's a root-level variable, it'll be like this: * --> $this->_tpldata[.][0][varname] == value + * @var array */ private $_tpldata = array('.' => array(0 => array())); + + /** + * @var array Reference to template->_tpldata['.'][0] + */ private $_rootref; - // Root dir and hash of filenames for each template handle. + /** + * @var string Root dir for template. + */ private $root = ''; + + /** + * @var string Path of the cache directory for the template + */ public $cachepath = ''; + + /** + * @var array Hash of handle => file path pairs + */ public $files = array(); + + /** + * @var array Hash of handle => filename pairs + */ public $filename = array(); /** @@ -56,26 +76,24 @@ class template } $this->_rootref = &$this->_tpldata['.'][0]; - - return true; } /** * Set custom template location (able to use directory outside of phpBB) * @access public + * @param string $template_path Path to template directory + * @param string $template_name Name of template */ public function set_custom_template($template_path, $template_name) { $this->root = $template_path; $this->cachepath = PHPBB_ROOT_PATH . 'cache/ctpl_' . str_replace('_', '-', $template_name) . '_'; - - return true; } /** * Sets the template filenames for handles. $filename_array - * should be a hash of handle => filename pairs. * @access public + * @param array $filname_array Should be a hash of handle => filename pairs. */ public function set_filenames(array $filename_array) { @@ -97,7 +115,7 @@ class template * Destroy template data set * @access public */ - function __destruct() + public function __destruct() { $this->_tpldata = array('.' => array(0 => array())); } @@ -105,6 +123,7 @@ class template /** * Reset/empty complete block * @access public + * @param string $blockname Name of block to destroy */ public function destroy_block_vars($blockname) { @@ -128,13 +147,14 @@ class template // Top-level block. unset($this->_tpldata[$blockname]); } - - return true; } /** * Display handle * @access public + * @param string $handle Handle to display + * @param bool $include_once Allow multiple inclusions + * @return bool True on success, false on failure */ public function display($handle, $include_once = true) { @@ -184,6 +204,11 @@ class template /** * Display the handle and assign the output to a template variable or return the compiled result. * @access public + * @param string $handle Handle to operate on + * @param string $template_var Template variable to assign compiled handle to + * @param bool $return_content If true return compiled handle, otherwise assign to $template_var + * @param bool $include_once Allow multiple inclusions of the file + * @return bool|string If $return_content is true return string of the compiled handle, otherwise return true */ public function assign_display($handle, $template_var = '', $return_content = true, $include_once = false) { @@ -204,8 +229,11 @@ class template /** * Load a compiled template if possible, if not, recompile it * @access private + * @param string $handle Handle of the template to load + * @return string|bool Return filename on success otherwise false + * @uses template_compile is used to compile uncached templates */ - private function _tpl_load(&$handle) + private function _tpl_load($handle) { global $config; @@ -241,11 +269,12 @@ class template /** * This code should only run when some high level error prevents us from writing to the cache. * @access private + * @param string $handle Template handle to compile + * @return string|bool Return compiled code on success otherwise false + * @uses template_compile is used to compile template */ - private function _tpl_eval(&$handle) + private function _tpl_eval($handle) { -// global $user, $config; - if (!class_exists('template_compile')) { include(PHPBB_ROOT_PATH . 'includes/functions_template.' . PHP_EXT); @@ -270,6 +299,7 @@ class template /** * Assign key variable pairs from an array * @access public + * @param array $vararray A hash of variable name => value pairs */ public function assign_vars(array $vararray) { @@ -277,24 +307,24 @@ class template { $this->_rootref[$key] = $val; } - - return true; } /** * Assign a single variable to a single key * @access public + * @param string $varname Variable name + * @param string $varval Value to assign to variable */ public function assign_var($varname, $varval) { $this->_rootref[$varname] = $varval; - - return true; } /** * Assign key variable pairs from an array to a specified block * @access public + * @param string $blockname Name of block to assign $vararray to + * @param array $vararray A hash of variable name => value pairs */ public function assign_block_vars($blockname, array $vararray) { @@ -323,15 +353,12 @@ class template // Add a new iteration to this block with the variable assignments we were given. $this->_tpldata[$blockname][] = $vararray; } - - return true; } /** * Change already assigned key variable pair (one-dimensional - single loop entry) * * An example of how to use this function: - * {@example alter_block_array.php} * * @param string $blockname the blockname, for example 'loop' * @param array $vararray the var array to insert/add or merge @@ -425,8 +452,11 @@ class template /** * Include a separate template * @access private + * @param string $filename Template filename to include + * @param bool $include True to include the file, false to just load it + * @uses template_compile is used to compile uncached templates */ - public function _tpl_include($filename, $include = true) + private function _tpl_include($filename, $include = true) { $handle = $filename; $this->filename[$handle] = $filename; diff --git a/tests/template/template.php b/tests/template/template.php index 89de67a453..9558ea09f9 100644 --- a/tests/template/template.php +++ b/tests/template/template.php @@ -21,6 +21,9 @@ require_once '../phpBB/includes/template.php'; class phpbb_template_template_test extends phpbb_test_case { private $template; + + const PRESERVE_CACHE = true; + private function display($handle) { ob_start(); @@ -77,6 +80,13 @@ class phpbb_template_template_test extends phpbb_test_case '', // Expected result ), */ + array( + 'basic.html', + array(), + array(), + array(), + "pass\npass\n", + ), array( 'variable.html', array('VARIABLE' => 'value'), @@ -129,10 +139,17 @@ class phpbb_template_template_test extends phpbb_test_case array( 'loop.html', array(), - array('loop' => array(array(), array())), + array('loop' => array(array(), array()), 'loop.block' => array(array())), array(), "loop\nloop\nloop\nloop", ), + array( + 'loop.html', + array(), + array('loop' => array(array(), array()), 'loop.block' => array(array()), 'block' => array(array(), array())), + array(), + "loop\nloop\nloop\nloop\n\nloop#0-block#0\nloop#0-block#1\nloop#1-block#0\nloop#1-block#1", + ), array( 'loop_vars.html', array(), @@ -164,9 +181,9 @@ class phpbb_template_template_test extends phpbb_test_case array( 'define.html', array(), + array('loop' => array(array(), array(), array(), array(), array(), array(), array())), array(), - array(), - "xyz\nabc", + "xyz\nabc\n\n00\n11\n22\n33\n44\n55\n66\n\n144\n144", ), array( 'expressions.html', @@ -210,6 +227,27 @@ class phpbb_template_template_test extends phpbb_test_case array(), "on\non\non\non\noff\noff\noff\noff\non\non\non\non\n\noff\noff\noff\non\non\non\noff\noff\noff\non\non\non", ), + array( + 'lang.html', + array(), + array(), + array(), + "{ VARIABLE }\n{ VARIABLE }", + ), + array( + 'lang.html', + array('L_VARIABLE' => "Value'"), + array(), + array(), + "Value'\nValue\'", + ), + array( + 'lang.html', + array('LA_VARIABLE' => "Value'"), + array(), + array(), + "{ VARIABLE }\nValue'", + ), ); } @@ -233,6 +271,12 @@ class phpbb_template_template_test extends phpbb_test_case $this->assertEquals($expected, $this->display('test'), "Testing $file"); $this->assertFileExists($cache_file); + + // For debugging + if (self::PRESERVE_CACHE) + { + copy($cache_file, str_replace('ctpl_', 'tests_ctpl_', $cache_file)); + } } /** diff --git a/tests/template/templates/basic.html b/tests/template/templates/basic.html index 1a3fd5a96a..c1dd690260 100644 --- a/tests/template/templates/basic.html +++ b/tests/template/templates/basic.html @@ -16,3 +16,5 @@ fail pass + + diff --git a/tests/template/templates/define.html b/tests/template/templates/define.html index 94741e53fc..b9a2219a02 100644 --- a/tests/template/templates/define.html +++ b/tests/template/templates/define.html @@ -9,3 +9,27 @@ {$VALUE} + + + + + +{loop.$VALUE} + +{loop.$VALUE} + +{loop.$VALUE} + + + + + +{test.deep.defines.$VALUE} + + + +{test.deep.defines.$VALUE} + + + +{test.deep.defines.$VALUE} diff --git a/tests/template/templates/expressions.html b/tests/template/templates/expressions.html index f0b38cc2ec..47a164e481 100644 --- a/tests/template/templates/expressions.html +++ b/tests/template/templates/expressions.html @@ -10,6 +10,8 @@ passfail +failpass + passfail passfail @@ -84,5 +86,3 @@ passfail passfail - -passfail diff --git a/tests/template/templates/lang.html b/tests/template/templates/lang.html new file mode 100644 index 0000000000..2b5ea1cafe --- /dev/null +++ b/tests/template/templates/lang.html @@ -0,0 +1,3 @@ +{L_VARIABLE} + +{LA_VARIABLE} diff --git a/tests/template/templates/loop.html b/tests/template/templates/loop.html index 3912635e9d..f1e1bf7e53 100644 --- a/tests/template/templates/loop.html +++ b/tests/template/templates/loop.html @@ -13,3 +13,11 @@ noloop loop + + + + +loop#{loop.S_ROW_NUM}-block#{block.S_ROW_NUM} + + +