Foxtrott has submitted this change and it was merged. Change subject: Refactor Menu package ......................................................................
Refactor Menu package Change-Id: Ie70b62ac756574c51ea1e2c8ab9b111d3fb8c5f2 --- M src/Menu/Menu.php M src/Menu/MenuFactory.php M src/Menu/MenuFromLines.php M tests/phpunit/Menu/MenuFromLinesTest.php 4 files changed, 155 insertions(+), 103 deletions(-) Approvals: Foxtrott: Verified; Looks good to me, approved diff --git a/src/Menu/Menu.php b/src/Menu/Menu.php index ec50cb3..78ae3f7 100644 --- a/src/Menu/Menu.php +++ b/src/Menu/Menu.php @@ -21,7 +21,7 @@ * with this program. If not, see <http://www.gnu.org/licenses/>. * * @file - * @ingroup Skins + * @ingroup Skins */ namespace Skins\Chameleon\Menu; @@ -29,36 +29,27 @@ /** * Class Menu * - * @author Stephan Gambke - * @since 1.0 + * @author Stephan Gambke + * @since 1.0 * @ingroup Skins */ abstract class Menu { - abstract public function getHtml(); - private $menuItemFormatter = null; private $itemListFormatter = null; + abstract public function getHtml(); + /** + * @param string $href + * @param string $text + * @param int $depth + * @param string $subitems + * * @return string */ - public function getItemListFormatter() { - - if ( $this->itemListFormatter === null ) { - $this->setItemListFormatter( function ( $rawItemsHtml, $depth ) { - return "<ul>$rawItemsHtml</ul>"; - } ); - } - - return $this->itemListFormatter; - } - - /** - * @param string $itemListFormatter - */ - public function setItemListFormatter( $itemListFormatter ) { - $this->itemListFormatter = $itemListFormatter; + protected function getHtmlForMenuItem( $href, $text, $depth, $subitems ) { + return call_user_func( $this->getMenuItemFormatter(), $href, $text, $depth, $subitems ); } /** @@ -71,8 +62,13 @@ $this->setMenuItemFormatter( function ( $href, $text, $depth, $subitems ) { $href = \Sanitizer::cleanUrl( $href ); $text = htmlspecialchars( $text ); + $indent = str_repeat( "\t", 2 * $depth ); - return "<li><a href=\"$href\">$text</a>$subitems</li>"; + if ( $subitems !== '' ) { + return "$indent<li>\n$indent\t<a href=\"$href\">$text</a>\n$subitems$indent</li>\n"; + } else { + return "$indent<li><a href=\"$href\">$text</a></li>\n"; + } } ); } @@ -88,20 +84,8 @@ } /** - * @param $href - * @param $text - * @param $depth - * @param $subitems - * - * @return mixed|string - */ - protected function getHtmlForMenuItem( $href, $text, $depth, $subitems ) { - return call_user_func( $this->getMenuItemFormatter(), $href, $text, $depth, $subitems ); - } - - /** * @param string $rawItemsHtml - * @param int $depth + * @param int $depth * * @return string */ @@ -109,4 +93,26 @@ return call_user_func( $this->getItemListFormatter(), $rawItemsHtml, $depth ); } + /** + * @return callable + */ + public function getItemListFormatter() { + + if ( $this->itemListFormatter === null ) { + $this->setItemListFormatter( function ( $rawItemsHtml, $depth ) { + $indent = str_repeat( "\t", 2 * $depth + 1 ); + return "$indent<ul>\n$rawItemsHtml$indent</ul>\n"; + } ); + } + + return $this->itemListFormatter; + } + + /** + * @param callable $itemListFormatter + */ + public function setItemListFormatter( $itemListFormatter ) { + $this->itemListFormatter = $itemListFormatter; + } + } diff --git a/src/Menu/MenuFactory.php b/src/Menu/MenuFactory.php index 47d238c..9fae9c3 100644 --- a/src/Menu/MenuFactory.php +++ b/src/Menu/MenuFactory.php @@ -21,7 +21,7 @@ * with this program. If not, see <http://www.gnu.org/licenses/>. * * @file - * @ingroup Skins + * @ingroup Skins */ namespace Skins\Chameleon\Menu; @@ -29,8 +29,8 @@ /** * Class MenuFactory * - * @author Stephan Gambke - * @since 1.0 + * @author Stephan Gambke + * @since 1.0 * @ingroup Skins */ class MenuFactory { @@ -66,10 +66,10 @@ /** * @param string $text - * - * @throws \MWException + * @param bool $forContent * * @return Menu + * @throws \MWException */ public function getMenuFromMessageText( $text, $forContent = false ) { @@ -78,8 +78,6 @@ } $lines = explode( "\n", trim( $text ) ); - - array_unshift( $lines, '' ); return new MenuFromLines( $lines, $forContent ); } diff --git a/src/Menu/MenuFromLines.php b/src/Menu/MenuFromLines.php index f522923..ff03401 100644 --- a/src/Menu/MenuFromLines.php +++ b/src/Menu/MenuFromLines.php @@ -21,44 +21,55 @@ * with this program. If not, see <http://www.gnu.org/licenses/>. * * @file - * @ingroup Skins + * @ingroup Skins */ namespace Skins\Chameleon\Menu; + use Title; /** * Class MenuFromLines * - * @author Stephan Gambke - * @since 1.0 + * @author Stephan Gambke + * @since 1.0 * @ingroup Skins */ class MenuFromLines extends Menu { private $lines = null; private $inContentLanguage = false; + private $menuItemData = null; - private $linkData = null; + private $needsParse = true; - /** - * @var Menu[] - */ + /** @var Menu[] */ private $children = array(); - private $html = null; /** - * @param string[] $lines - * @param bool $inContentLanguage + * @param string[] $lines + * @param bool $inContentLanguage + * @param null|string[] $itemData */ - public function __construct( &$lines, $inContentLanguage = false ) { + public function __construct( &$lines, $inContentLanguage = false, $itemData = null ) { + $this->lines = &$lines; $this->inContentLanguage = $inContentLanguage; + + if ( $itemData !== null ) { + $this->menuItemData = $itemData; + } else { + $this->menuItemData = array( + 'text' => '', + 'href' => '#', + 'depth' => 0 + ); + } } /** - * @return mixed|null|string + * @return string */ public function getHtml() { @@ -73,55 +84,39 @@ return $this->html; } + /** + * @return string[]|null + */ public function parseLines() { - if ( empty( $this->lines ) ) { - return; + if ( !$this->needsParse ) { + return null; } - $line = array_shift( $this->lines ); - $this->linkData = $this->parseOneLine( $line ); + $this->needsParse = false; - while ( count( $this->lines ) ) { + $line = $this->getNextLine(); + $subItemData = $this->parseOneLine( $line ); - $line = trim( reset( $this->lines ) ); + while ( $subItemData !== null && $subItemData[ 'depth' ] > $this->menuItemData[ 'depth' ] ) { - if ( empty( $line ) ) { // skip empty lines - array_shift( $this->lines ); - continue; - } + $subItemData = $this->createChildAndParseNextLine( $subItemData ); - if ( strrpos( $line, '*' ) !== $this->linkData[ 'depth' ] ) { - return; - } - - $child = new self( $this->lines ); - $child->setMenuItemFormatter( $this->getMenuItemFormatter() ); - $child->setItemListFormatter( $this->getItemListFormatter() ); - $child->parseLines(); - $this->children[] = $child; } + + return $subItemData; } /** - * @return mixed|string + * @return string */ - private function buildHtml() { + protected function getNextLine() { + $line = ''; - $itemList = ''; - - if ( ! empty( $this->children ) ) { - foreach ( $this->children as $child ) { - $itemList .= $child->getHtml(); - } - $itemList = $this->getHtmlForMenuItemList( $itemList, $this->linkData['depth'] ); - } - - if ( $this->linkData['text'] !== '' ) { - return $this->getHtmlForMenuItem( $this->linkData['href'], $this->linkData['text'], $this->linkData['depth'], $itemList ); - } else { - return $itemList; - } + while ( count( $this->lines ) > 0 && empty( $line ) ) { + $line = trim( array_shift( $this->lines ) ); + }; + return $line; } /** @@ -138,6 +133,10 @@ */ protected function parseOneLine( $rawLine ) { + if ( empty( $rawLine ) ) { + return null; + } + list( $depth, $line ) = $this->extractDepthAndLine( $rawLine ); $lineArr = array_map( 'trim', explode( '|', $line, 2 ) ); @@ -150,14 +149,15 @@ $text = $linkDescription === '' ? $linkTarget : $this->getTextFromMessageName( $linkDescription ); return array( - 'text' => $text, - 'href' => $href, - 'depth' => $depth + 'text' => $text, + 'href' => $href, + 'depth' => $depth ); } /** - * @param $rawLine + * @param string $rawLine + * * @return array */ protected function extractDepthAndLine( $rawLine ) { @@ -173,6 +173,7 @@ /** * @param string $messageName + * * @return string */ protected function getTextFromMessageName( $messageName ) { @@ -182,7 +183,8 @@ } /** - * @param $linkTarget + * @param string $linkTarget + * * @return string * @throws \MWException */ @@ -190,19 +192,20 @@ if ( empty( $linkTarget ) ) { return '#'; - } elseif ( preg_match( '/^(?:' . wfUrlProtocols() . ')/', $linkTarget ) || $linkTarget[ 0 ] === '#') { + } elseif ( preg_match( '/^(?:' . wfUrlProtocols() . ')/', $linkTarget ) || $linkTarget[ 0 ] === '#' ) { return $linkTarget; } else { - return $this->getHrefFromTitle( $linkTarget ); + return $this->getHrefForWikiPage( $linkTarget ); } } /** - * @param $linkTarget + * @param string $linkTarget + * * @return string * @throws \MWException */ - protected function getHrefFromTitle( $linkTarget ) { + protected function getHrefForWikiPage( $linkTarget ) { $title = Title::newFromText( $linkTarget ); if ( $title instanceof Title ) { @@ -212,4 +215,49 @@ return '#'; } + /** + * @param string[] $subItemData + * + * @return null|string[] + */ + protected function createChildAndParseNextLine( $subItemData ) { + $child = new self( $this->lines, $this->inContentLanguage, $subItemData ); + $child->setMenuItemFormatter( $this->getMenuItemFormatter() ); + $child->setItemListFormatter( $this->getItemListFormatter() ); + $subItemData = $child->parseLines(); + $this->children[ ] = $child; + return $subItemData; + } + + /** + * @return string + */ + protected function buildHtml() { + + $submenuHtml = $this->buildSubmenuHtml(); + + if ( $this->menuItemData[ 'text' ] !== '' ) { + return $this->getHtmlForMenuItem( $this->menuItemData[ 'href' ], $this->menuItemData[ 'text' ], $this->menuItemData[ 'depth' ], $submenuHtml ); + } else { + return $submenuHtml; + } + } + + /** + * @return string + */ + protected function buildSubmenuHtml() { + + if ( empty( $this->children ) ) { + return ''; + } + + $itemList = ''; + foreach ( $this->children as $child ) { + $itemList .= $child->getHtml(); + } + + return $this->getHtmlForMenuItemList( $itemList, $this->menuItemData[ 'depth' ] ); + } + } diff --git a/tests/phpunit/Menu/MenuFromLinesTest.php b/tests/phpunit/Menu/MenuFromLinesTest.php index 2329ea9..aed62b2 100644 --- a/tests/phpunit/Menu/MenuFromLinesTest.php +++ b/tests/phpunit/Menu/MenuFromLinesTest.php @@ -73,11 +73,11 @@ $ap = $GLOBALS[ 'wgArticlePath' ]; - $expected = '<ul><li><a href="' . str_replace( '$1', 'Foo', $ap ) . - '">Foo</a><ul><li><a href="' . str_replace( '$1', 'FooBar', $ap ) . - '">FooBar</a><ul><li><a href="' . str_replace( '$1', 'FooBarBaz', $ap ) . - '">FooBarBaz</a></li></ul></li></ul></li><li><a href="' . str_replace( '$1', 'Test', $ap ) . - '">Bar</a></li></ul>'; + $expected = "\t<ul>\n\t\t<li>\n\t\t\t<a href=\"" . str_replace( '$1', 'Foo', $ap ) . + "\">Foo</a>\n\t\t\t<ul>\n\t\t\t\t<li>\n\t\t\t\t\t<a href=\"" . str_replace( '$1', 'FooBar', $ap ) . + "\">FooBar</a>\n\t\t\t\t\t<ul>\n\t\t\t\t\t\t<li><a href=\"" . str_replace( '$1', 'FooBarBaz', $ap ) . + "\">FooBarBaz</a></li>\n\t\t\t\t\t</ul>\n\t\t\t\t</li>\n\t\t\t</ul>\n\t\t</li>\n\t\t<li><a href=\"" . str_replace( '$1', 'Test', $ap ) . + "\">Bar</a></li>\n\t</ul>\n"; /** @var MenuFromLines $instance */ $instance = new MenuFromLines( $lines, true ); -- To view, visit https://gerrit.wikimedia.org/r/172419 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ie70b62ac756574c51ea1e2c8ab9b111d3fb8c5f2 Gerrit-PatchSet: 2 Gerrit-Project: mediawiki/skins/chameleon Gerrit-Branch: master Gerrit-Owner: Foxtrott <[email protected]> Gerrit-Reviewer: Foxtrott <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
