[ticket/15538] Split macros and add docblocks

PHPBB3-15538

phpBB/config/default/container/services_twig.yml

@@ -57,11 +57,18 @@ services:
     template.twig.extensions.icon:
         class: phpbb\template\twig\extension\icon
         arguments:
-            - '@template.twig.environment'
             - '@user'
         tags:
             - { name: twig.extension }
 
+    template.twig.extensions.implode:
+        class: phpbb\template\twig\extension\implode
+
+    template.twig.extensions.macro:
+        class: phpbb\template\twig\extension\macro
+        arguments:
+            - '@template.twig.environment'
+
     template.twig.extensions.routing:
         class: phpbb\template\twig\extension\routing
         arguments:

phpBB/phpbb/template/twig/extension/icon.php

@@ -15,21 +15,25 @@ namespace phpbb\template\twig\extension;
 
 use phpbb\template\twig\environment;
 
-abstract class icon extends \Twig\Extension\AbstractExtension implements \Twig\Extension\GlobalsInterface
+abstract class icon extends \Twig\Extension\AbstractExtension
 {
-	protected $twig;
+	/** @var \phpbb\user */
 	protected $user;
 
-	public function __construct(environment $twig, \phpbb\user $user)
+	/**
+	 * Constructor.
+	 *
+	 * @param \phpbb\user	$user		User object
+	 */
+	public function __construct(\phpbb\user $user)
 	{
-		$this->twig = $twig;
 		$this->user = $user;
 	}
 
 	/**
-	 * Get the name of this extension.
+	 * Returns the name of this extension.
 	 *
-	 * @return string
+	 * @return string The extension name
 	 */
 	public function getName()
 	{
@@ -37,27 +41,10 @@ abstract class icon extends \Twig\Extension\AbstractExtension implements \Twig\E
 	}
 
 	/**
-	 * Returns a list of global variables to add to the existing list.
+	 * Returns a list of filters to add to the existing list.
 	 *
-	 * @return array An array of global variables
+	 * @return \Twig\TwigFilter[]
 	 */
-	public function getGlobals()
-	{
-		$macros = null;
-
-		try
-		{
-			$macros = $this->twig->loadTemplate('macros.html');
-		}
-		catch (\Twig\Error\Error $e)
-		{
-		}
-
-		return [
-			'macro'	=> $macros,
-		];
-	}
-
 	public function getFilters()
 	{
 		return [
@@ -65,15 +52,25 @@ abstract class icon extends \Twig\Extension\AbstractExtension implements \Twig\E
 		];
 	}
 
+	/**
+	 * Returns a list of functions to add to the existing list.
+	 *
+	 * @return \Twig\TwigFunction[]
+	 */
 	public function getFunctions()
 	{
 		return [
 			new \Twig\TwigFunction('Svg_clean', [$this, 'svg_clean'], ['needs_environment' => true]),
-			new \Twig\TwigFunction('Implode_attributes', [$this, 'implode_attributes']),
-			new \Twig\TwigFunction('Implode_classes', [$this, 'implode_classes']),
 		];
 	}
 
+	/**
+	 * Create a path to a PNG template icon.
+	 *
+	 * @param environment	$environment	Twig environment object
+	 * @param string		$icon			The icon name
+	 * @return string
+	 */
 	protected function png_path(environment $environment, $icon)
 	{
 		$board_url	= defined('PHPBB_USE_BOARD_URL_PATH') && PHPBB_USE_BOARD_URL_PATH;
@@ -83,6 +80,13 @@ abstract class icon extends \Twig\Extension\AbstractExtension implements \Twig\E
 		return "{$web_path}styles/{$style_path}/template/icons/png/{$icon}.png";
 	}
 
+	/**
+	 * Load and clean an SVG template icon.
+	 *
+	 * @param environment	$environment	Twig environment object
+	 * @param string		$icon			The icon name
+	 * @return string
+	 */
 	protected function svg_clean(environment $environment, $icon)
 	{
 		try
@@ -98,7 +102,6 @@ abstract class icon extends \Twig\Extension\AbstractExtension implements \Twig\E
 		$svg = $src->getCode();
 
 		$doc = new \DOMDocument();
-		$doc->formatOutput = false;
 		$doc->preserveWhiteSpace = false;
 		$doc->strictErrorChecking = false;
 
@@ -142,90 +145,4 @@ abstract class icon extends \Twig\Extension\AbstractExtension implements \Twig\E
 
 		return $string;
 	}
-
-	protected function implode_attributes(...$arguments)
-	{
-		$string = '';
-		$attributes = [];
-
-		foreach ($arguments as $argument)
-		{
-			if (is_string($argument))
-			{
-				$attributes[] = $argument;
-			}
-			else if (is_array($argument))
-			{
-				foreach ($argument as $key => $value)
-				{
-					if (is_integer($key) && is_string($value))
-					{
-						$attributes[] = $value;
-					}
-					else
-					{
-						$attributes[$key] = $value;
-					}
-				}
-			}
-		}
-
-		foreach ($attributes as $attribute => $value)
-		{
-			if (is_string($attribute))
-			{
-				$string .= ' ' . $attribute . '="' . $value . '"';
-			}
-			else
-			{
-				$string .= ' ' . $attribute;
-			}
-		}
-
-		return $string;
-	}
-
-	protected function implode_classes(...$arguments)
-	{
-		$classes = [];
-
-		foreach ($arguments as $argument)
-		{
-			if (is_string($argument))
-			{
-				$classes[] = $argument;
-			}
-			else if (is_array($argument))
-			{
-				foreach ($argument as $key => $value)
-				{
-					if (is_integer($key) && is_string($value))
-					{
-						$classes[] = $value;
-					}
-					else if (is_string($key))
-					{
-						if ($value)
-						{
-							$classes[] = $key;
-						}
-					}
-					else if (is_array($value))
-					{
-						foreach ($value as $class => $condition)
-						{
-							if ($condition)
-							{
-								$classes[] = $class;
-							}
-						}
-					}
-				}
-			}
-		}
-
-		$string = implode(' ', array_unique($classes));
-
-		return $string ? ' ' . $string : $string;
-	}
 }

phpBB/phpbb/template/twig/extension/implode.php

@@ -0,0 +1,175 @@
+<?php
+/**
+ *
+ * This file is part of the phpBB Forum Software package.
+ *
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @license GNU General Public License, version 2 (GPL-2.0)
+ *
+ * For full copyright and license information, please see
+ * the docs/CREDITS.txt file.
+ *
+ */
+
+namespace phpbb\template\twig\extension;
+
+abstract class implode extends \Twig\Extension\AbstractExtension
+{
+	/**
+	 * Returns the name of this extension.
+	 *
+	 * @return string The extension name
+	 */
+	public function getName()
+	{
+		return 'implode';
+	}
+
+	/**
+	 * Returns a list of functions to add to the existing list.
+	 *
+	 * @return \Twig\TwigFunction[]
+	 */
+	public function getFunctions()
+	{
+		return [
+			new \Twig\TwigFunction('Implode_attributes', [$this, 'implode_attributes']),
+			new \Twig\TwigFunction('Implode_classes', [$this, 'implode_classes']),
+		];
+	}
+
+	/**
+	 * Implode an array of attributes to a string.
+	 *
+	 * This string will be prepended by a space for ease-of-use.
+	 *
+	 * Examples would be:
+	 * Implode_attributes('checked', {'data-ajax': 'true'})
+	 * Implode_attributes(['checked', {'data-ajax': 'true'}])
+	 *
+	 * @param mixed		$arguments
+	 * @return string
+	 */
+	protected function implode_attributes(...$arguments)
+	{
+		$string = '';
+		$attributes = [];
+
+		foreach ($arguments as $argument)
+		{
+			if (is_string($argument))
+			{
+				$attributes[] = $argument;
+			}
+			else if (is_array($argument))
+			{
+				foreach ($argument as $key => $value)
+				{
+					if (is_integer($key) && is_string($value))
+					{
+						$attributes[] = $value;
+					}
+					else if (is_array($value))
+					{
+						if (is_integer($key) && is_string($value))
+						{
+							$attributes[] = $value;
+						}
+						else
+						{
+							$attributes[$key] = $value;
+						}
+					}
+					else
+					{
+						$attributes[$key] = $value;
+					}
+				}
+			}
+		}
+
+		foreach ($attributes as $attribute => $value)
+		{
+			if (is_string($attribute))
+			{
+				$string .= ' ' . $attribute . '="' . $value . '"';
+			}
+			else
+			{
+				$string .= ' ' . $attribute;
+			}
+		}
+
+		return $string;
+	}
+
+	/**
+	 * Implode an array or classes to a string.
+	 *
+	 * This string will be prepended with a space for ease-of-use.
+	 *
+	 * Conditions can be added to the classes, which will determine if the classes is added to the string.
+	 * @see https://twig.symfony.com/doc/2.x/functions/html_classes.html
+	 *
+	 * An example would be:
+	 * Implode_classes('a-class', 'another-class', {
+	 * 		'reported-class': S_POST_REPORTED,
+	 * 		'hidden-class': S_POST_HIDDEN,
+	 * })
+	 *
+	 * This function differs from the html_classes function linked above,
+	 * in that it allows another depth level, so it also supports a single argument.
+	 *
+	 * An example would be:
+	 * Implode_classes(['a-class', 'another-class', {
+	 * 		'reported-class': S_POST_REPORTED,
+	 * 		'hidden-class': S_POST_HIDDEN,
+	 * }])
+	 *
+	 * @param mixed		$arguments
+	 * @return string					The classes string prepended with a space
+	 */
+	protected function implode_classes(...$arguments)
+	{
+		$classes = [];
+
+		foreach ($arguments as $argument)
+		{
+			if (is_string($argument))
+			{
+				$classes[] = $argument;
+			}
+			else if (is_array($argument))
+			{
+				foreach ($argument as $key => $value)
+				{
+					if (is_integer($key) && is_string($value))
+					{
+						$classes[] = $value;
+					}
+					else if (is_string($key))
+					{
+						if ($value)
+						{
+							$classes[] = $key;
+						}
+					}
+					else if (is_array($value))
+					{
+						foreach ($value as $class => $condition)
+						{
+							if ($condition)
+							{
+								$classes[] = $class;
+							}
+						}
+					}
+				}
+			}
+		}
+
+		$string = implode(' ', array_unique($classes));
+
+		return $string ? ' ' . $string : $string;
+	}
+}

phpBB/phpbb/template/twig/extension/macro.php

@@ -0,0 +1,64 @@
+<?php
+/**
+ *
+ * This file is part of the phpBB Forum Software package.
+ *
+ * @copyright (c) phpBB Limited <https://www.phpbb.com>
+ * @license GNU General Public License, version 2 (GPL-2.0)
+ *
+ * For full copyright and license information, please see
+ * the docs/CREDITS.txt file.
+ *
+ */
+
+namespace phpbb\template\twig\extension;
+
+use phpbb\template\twig\environment;
+
+abstract class macro extends \Twig\Extension\AbstractExtension implements \Twig\Extension\GlobalsInterface
+{
+	/** @var environment */
+	protected $twig;
+
+	/**
+	 * Constructor.
+	 *
+	 * @param environment	$twig		Twig environment object
+	 */
+	public function __construct(environment $twig)
+	{
+		$this->twig = $twig;
+	}
+
+	/**
+	 * Returns the name of this extension.
+	 *
+	 * @return string The extension name
+	 */
+	public function getName()
+	{
+		return 'macro';
+	}
+
+	/**
+	 * Returns a list of global variables to add to the existing list.
+	 *
+	 * @return array An array of global variables
+	 */
+	public function getGlobals()
+	{
+		$macros = null;
+
+		try
+		{
+			$macros = $this->twig->loadTemplate('macros.html');
+		}
+		catch (\Twig\Error\Error $e)
+		{
+		}
+
+		return [
+			'macros' => $macros,
+		];
+	}
+}

phpBB/styles/all/template/macros.html

@@ -10,7 +10,7 @@
 
 {# FA icons #}
 {% macro Fa(icon, classes = '', title = '', hidden = false, attributes = {}) %}
-	<i class="o-icon font fa-{{ icon ~ Implode_classes(classes) }}"{% if hidden %}{% if title %} title="{{ lang(title) }}"{% endif %} aria-hidden="true"{% endif %}{{ _self.attributes(attributes) }}></i>
+	<i class="o-icon font fa-{{ icon ~ Implode_classes(classes) }}"{% if hidden %}{% if title %} title="{{ lang(title) }}"{% endif %} aria-hidden="true"{% endif %}{{ Implode_attributes(attributes) }}></i>
 	{% if title %}<span{% if hidden %} class="sr-only"{% endif %}>{{ lang(title) }}</span>{% endif %}
 {% endmacro Fa %}
 
@@ -23,7 +23,7 @@
 {% macro Svg(icon, classes = '', title = '', hidden = false, attributes = {}) %}
 	{% set title_id = title ? title|lower|replace({' ': '_'}) ~ '-' ~ random() %}
 
-	<svg class="o-icon svg svg-{{ icon ~ Implode_classes(classes) }}" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"{% if title %}{% if hidden %} aria-hidden="true"{% endif %} aria-labelledby="{{ title_id }}"{% endif %} role="img"{{ _self.attributes(attributes) }}>
+	<svg class="o-icon svg svg-{{ icon ~ Implode_classes(classes) }}" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 24 24"{% if title %}{% if hidden %} aria-hidden="true"{% endif %} aria-labelledby="{{ title_id }}"{% endif %} role="img"{{ Implode_attributes(attributes) }}>
 	{% if title %}
 		<title id="{{ title_id }}">{{ lang(title) }}</title>
 	{% endif %}