- 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

@@ -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]+\.)*\\$?[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?
+		// <!-- BEGIN loop -->
+		// <!-- BEGIN !loop2 -->
+		// <!-- END !loop2 -->
+		// <!-- END 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');

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;

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<!-- DUMMY var -->",
+			),
 			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));
+		}
 	}
 
 	/**

tests/template/templates/basic.html

@@ -16,3 +16,5 @@ fail
 <!-- BEGINELSE -->
 pass
 <!-- END empty -->
+
+<!-- DUMMY var -->

tests/template/templates/define.html

@@ -9,3 +9,27 @@
 <!-- UNDEFINE $VALUE -->
 
 {$VALUE}
+
+<!-- DEFINE $VALUE -->
+
+<!-- BEGIN loop -->
+<!-- DEFINE loop.$VALUE = loop.S_ROW_NUM -->
+{loop.$VALUE}
+<!-- UNDEFINE $VALUE -->
+{loop.$VALUE}
+<!-- UNDEFINE loop.$VALUE -->
+{loop.$VALUE}
+
+<!-- END loop -->
+
+<!-- DEFINE test.deep.defines.$VALUE = 12 * 12 -->
+
+{test.deep.defines.$VALUE}
+
+<!-- UNDEFINE $VALUE -->
+
+{test.deep.defines.$VALUE}
+
+<!-- UNDEFINE test.deep.defines.$VALUE -->
+
+{test.deep.defines.$VALUE}

tests/template/templates/expressions.html

@@ -10,6 +10,8 @@
 
 <!-- IF 32 is div by 16 -->pass<!-- ELSE -->fail<!-- ENDIF -->
 
+<!-- IF 10 is not even -->fail<!-- ELSE -->pass<!-- ENDIF -->
+
 <!-- IF 24 == 24 -->pass<!-- ELSE -->fail<!-- ENDIF -->
 
 <!-- IF 24 eq 24 -->pass<!-- ELSE -->fail<!-- ENDIF -->
@@ -84,5 +86,3 @@
 <!-- IF 6 % 4 == 2 -->pass<!-- ELSE -->fail<!-- ENDIF -->
 
 <!-- IF 24 mod 12 == 0 -->pass<!-- ELSE -->fail<!-- ENDIF -->
-
-<!-- IF not (43 > 53) -->pass<!-- ELSE -->fail<!-- ENDIF -->

tests/template/templates/lang.html

@@ -0,0 +1,3 @@
+{L_VARIABLE}
+
+{LA_VARIABLE}

tests/template/templates/loop.html

@@ -13,3 +13,11 @@ noloop
 <!-- IF .loop == 2 -->
 loop
 <!-- ENDIF -->
+
+<!-- BEGIN loop -->
+<!-- BEGIN !block -->
+
+loop#{loop.S_ROW_NUM}-block#{block.S_ROW_NUM}
+
+<!-- END !block -->
+<!-- END loop -->