r113256 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r113255‎ \| r113256 \| r113257 >
Date:	17:32, 7 March 2012
Author:	netbrain
Status:	deferred
Tags:
Comment:	added some constants instead of referencing same strings over and over again.
Modified paths:	/trunk/extensions/SideBarMenu/SideBarMenu.hooks.php (modified) (history) /trunk/extensions/SideBarMenu/SideBarMenu.php (modified) (history) /trunk/extensions/SideBarMenu/includes/MenuItem.php (modified) (history) /trunk/extensions/SideBarMenu/includes/MenuParser.php (modified) (history) /trunk/extensions/SideBarMenu/js/ext.sidebarmenu.js (modified) (history) /trunk/extensions/SideBarMenu/test/MenuParserTest.php (modified) (history)

Diff [purge]

Index: trunk/extensions/SideBarMenu/test/MenuParserTest.php
—	—	@@ -3,86 +3,95 @@
4	4	class MenuParserTest extends MediaWikiTestCase
5	5	{
6	6
	7	+ private $menuParser;
	8	+
	9	+ protected function setUp()
	10	+ {
	11	+ $this->menuParser = new MenuParser(true);
	12	+ }
	13	+
7	14	public function testValidInputWhenNull(){
8		~~- $this->assertFalse(MenuParser::isValidInput(null));~~
	15	+ $this->assertFalse($this->menuParser->isValidInput(null));
9	16	}
10	17
11	18	public function testValidInputWhenEmpty(){
12		~~- $this->assertFalse(MenuParser::isValidInput(""));~~
	19	+ $this->assertFalse($this->menuParser->isValidInput(""));
13	20	}
14	21
15	22	public function testValidInput(){
16		~~- $this->assertTrue(MenuParser::isValidInput("+MenuItem"));~~
	23	+ $this->assertTrue($this->menuParser->isValidInput("+MenuItem"));
17	24	}
18	25
19	26	public function testGetLevelWhenNull(){
20		~~- $this->assertEquals(0,MenuParser::getLevel(null));~~
	27	+ $this->assertEquals(0,$this->menuParser->getLevel(null));
21	28	}
22	29
23	30	public function testGetLevelWhenEmpty(){
24		~~- $this->assertEquals(0,MenuParser::getLevel(""));~~
	31	+ $this->assertEquals(0,$this->menuParser->getLevel(""));
25	32	}
26	33
27	34	public function testGetLevelWhenValidButNoStars(){
28		~~- $this->assertEquals(0,MenuParser::getLevel(""));~~
	35	+ $this->assertEquals(0,$this->menuParser->getLevel(""));
29	36	}
30	37
31	38	public function testGetLevelWithValid(){
32		~~- $this->assertEquals(3,MenuParser::getLevel("***MenuItem"));~~
	39	+ $this->assertEquals(3,$this->menuParser->getLevel("***MenuItem"));
33	40	}
34	41
35	42	public function testGetExpandedParameterWhenNoneSupplied(){
36		~~- //default is false~~
37		~~- $this->assertFalse(MenuParser::getExpandedParameter("MenuItem"));~~
	43	+ $this->menuParser = new MenuParser(true);
	44	+ $this->assertTrue($this->menuParser->getExpandedParameter("MenuItem"));
	45	+ $this->menuParser = new MenuParser(false);
	46	+ $this->assertFalse($this->menuParser->getExpandedParameter("MenuItem"));
38	47	}
39	48
40	49	public function testGetExpandedParameterWhenNotExpanded(){
41		~~- $this->assertFalse(MenuParser::getExpandedParameter("-MenuItem"));~~
	50	+ $this->assertFalse($this->menuParser->getExpandedParameter("-MenuItem"));
42	51	}
43	52
44	53	public function testGetExpandedParameterWhenExpanded(){
45		~~- $this->assertTrue(MenuParser::getExpandedParameter("+MenuItem"));~~
	54	+ $this->assertTrue($this->menuParser->getExpandedParameter("+MenuItem"));
46	55	}
47	56
48	57	public function testGetTextParameter(){
49		~~- $this->assertEquals("MenuItem",MenuParser::getTextParameter("+***MenuItem"));~~
50		~~- $this->assertEquals("+MenuItem",MenuParser::getTextParameter("+***+MenuItem"));~~
51		~~- $this->assertEquals("MenuItem",MenuParser::getTextParameter("-MenuItem"));~~
52		~~- $this->assertEquals("MenuItem",MenuParser::getTextParameter("-*MenuItem"));~~
53		~~- $this->assertEquals("MenuItem",MenuParser::getTextParameter("MenuItem"));~~
54		~~- $this->assertEquals("+MenuItem",MenuParser::getTextParameter("+*+MenuItem"));~~
	58	+ $this->assertEquals("MenuItem",$this->menuParser->getTextParameter("+***MenuItem"));
	59	+ $this->assertEquals("+MenuItem",$this->menuParser->getTextParameter("+***+MenuItem"));
	60	+ $this->assertEquals("MenuItem",$this->menuParser->getTextParameter("-MenuItem"));
	61	+ $this->assertEquals("MenuItem",$this->menuParser->getTextParameter("-*MenuItem"));
	62	+ $this->assertEquals("MenuItem",$this->menuParser->getTextParameter("MenuItem"));
	63	+ $this->assertEquals("+MenuItem",$this->menuParser->getTextParameter("+*+MenuItem"));
55	64	}
56	65
57	66	public function testGetMenuItemWhenInputIsNull(){
58	67	$this->setExpectedException('InvalidArgumentException');
59		~~- $this->assertNull(MenuParser::getMenuItem(null));~~
	68	+ $this->assertNull($this->menuParser->getMenuItem(null));
60	69	}
61	70
62	71	public function testGetMenuItemWhenInputIsEmpty(){
63	72	$this->setExpectedException('InvalidArgumentException');
64		~~- $this->assertNull(MenuParser::getMenuItem(""));~~
	73	+ $this->assertNull($this->menuParser->getMenuItem(""));
65	74	}
66	75
67	76	public function testGetMenuItemWhenInputIsValid(){
68	77	$data = "MenuItem";
69		~~- $menuItem = MenuParser::getMenuItem($data);~~
	78	+ $menuItem = $this->menuParser->getMenuItem($data);
70	79	$this->assertNotNull($menuItem);
71	80	$this->assertEquals($data,$menuItem->getText());
72		~~- $this->assertFalse($menuItem->isExpanded()); //false is default~~
	81	+ $this->assertTrue($menuItem->isExpanded());
73	82	}
74	83
75	84	public function testGetMenuItemWhenInputIsValidAndExpandIsSet(){
76	85	$text = "MenuItem";
77	86	$data = "+".$text;
78		~~- $menuItem = MenuParser::getMenuItem($data);~~
	87	+ $menuItem = $this->menuParser->getMenuItem($data);
79	88	$this->assertNotNull($menuItem);
80	89	$this->assertEquals($text,$menuItem->getText());
81		~~- $this->assertTrue($menuItem->isExpanded()); //false is default~~
	90	+ $this->assertTrue($menuItem->isExpanded());
82	91	}
83	92
84	93	public function testParseDataIntoHierarchicalArray(){
85	94	$data = "MenuItem";
86		~~- $array = MenuParser::parseDataIntoHierarchicalArray($data);~~
	95	+ $array = $this->menuParser->parseDataIntoHierarchicalArray($data);
87	96	$this->assertNotNull($array);
88	97	$this->assertEquals($data,$array[0]);
89	98	}
—	—	@@ -90,7 +99,7 @@
91	100	public function testParseDataIntoHierarchicalArrayWithSubLevel(){
92	101	$lines = array("MenuItem","*SubMenuItem");
93	102	$data = join("\n",$lines);
94		~~- $array = MenuParser::parseDataIntoHierarchicalArray($data);~~
	103	+ $array = $this->menuParser->parseDataIntoHierarchicalArray($data);
95	104	$this->assertNotNull($array);
96	105	$this->assertArrayHasKey($lines[0],$array);
97	106	$this->assertEquals(
—	—	@@ -105,7 +114,7 @@
106	115	public function testParseDataIntoHierarchicalArrayWithSeveralSubLevels(){
107	116	$lines = array("MenuItem","SubMenuItem","SubMenuItem2","**SubMenuItemOf2");
108	117	$data = join("\n",$lines);
109		~~- $array = MenuParser::parseDataIntoHierarchicalArray($data);~~
	118	+ $array = $this->menuParser->parseDataIntoHierarchicalArray($data);
110	119	$this->assertNotNull($array);
111	120	$this->assertEquals(
112	121	array(
—	—	@@ -122,7 +131,7 @@
123	132	public function testParseDataIntoHierarchicalArrayWithSubLevelAndBack(){
124	133	$lines = array("MenuItem","*SubMenuItem","MenuItem2");
125	134	$data = join("\n",$lines);
126		~~- $array = MenuParser::parseDataIntoHierarchicalArray($data);~~
	135	+ $array = $this->menuParser->parseDataIntoHierarchicalArray($data);
127	136	$this->assertNotNull($array);
128	137	$this->assertEquals(
129	138	array(
—	—	@@ -137,7 +146,7 @@
138	147	public function testParseDataIntoHierarchicalArrayWithSubLevelAndBackSeveralLevels(){
139	148	$lines = array("MenuItem","SubMenuItem1","SubMenuItem2","**SubMenuItem3","MenuItem2");
140	149	$data = join("\n",$lines);
141		~~- $array = MenuParser::parseDataIntoHierarchicalArray($data);~~
	150	+ $array = $this->menuParser->parseDataIntoHierarchicalArray($data);
142	151	$this->assertNotNull($array);
143	152	$this->assertEquals(
144	153	array(
—	—	@@ -155,12 +164,12 @@
156	165
157	166
158	167	public function testGetMenuWithInvalidInput(){
159		~~- $this->assertNull(MenuParser::getMenuTree(null));~~
160		~~- $this->assertNull(MenuParser::getMenuTree(""));~~
	168	+ $this->assertNull($this->menuParser->getMenuTree(null));
	169	+ $this->assertNull($this->menuParser->getMenuTree(""));
161	170	}
162	171
163	172	public function testGetMenuWithValidInput(){
164		~~- $menu = MenuParser::getMenuTree("MenuItem");~~
	173	+ $menu = $this->menuParser->getMenuTree("MenuItem");
165	174	$this->assertNotNull($menu);
166	175	$this->assertTrue($menu->isRoot());
167	176	$this->assertEquals(1,sizeof($menu->getChildren()));
—	—	@@ -180,7 +189,7 @@
181	190	'MenuItem2',
182	191	'*SubMenuItem1OfMenuItem2'
183	192	);
184		~~- $menu = MenuParser::getMenuTree(join("\n",$data));~~
	193	+ $menu = $this->menuParser->getMenuTree(join("\n",$data));
185	194	$this->assertNotNull($menu);
186	195	$this->assertEquals(2,sizeof($menu->getChildren()));
187	196
—	—	@@ -193,7 +202,7 @@
194	203	'',
195	204	''
196	205	);
197		~~- $menu = MenuParser::getMenuTree(join("\n",$data));~~
	206	+ $menu = $this->menuParser->getMenuTree(join("\n",$data));
198	207	$this->assertNotNull($menu);
199	208	$this->assertEquals(1,sizeof($menu->getChildren()));
200	209
Index: trunk/extensions/SideBarMenu/SideBarMenu.php
—	—	@@ -4,6 +4,19 @@
5	5	die('Not an entry point.');
6	6	}
7	7
	8	+//SideBarMenu constants
	9	+const SBM_EXPANDED = 'parser.menuitem.expanded';
	10	+const SBM_CONTROLS_SHOW = 'controls.show';
	11	+const SBM_CONTROLS_HIDE = 'controls.hide';
	12	+const SBM_JS_ANIMATE = 'js.animate';
	13	+
	14	+
	15	+//default settings
	16	+$wgSideBarMenuConfig[SBM_CONTROLS_SHOW] = null;
	17	+$wgSideBarMenuConfig[SBM_CONTROLS_HIDE]= null;
	18	+$wgSideBarMenuConfig[SBM_JS_ANIMATE] = true;
	19	+$wgSideBarMenuConfig[SBM_EXPANDED] = true;
	20	+
8	21	$wgExtensionCredits['parserhook'][] = array(
9	22	'path' => __FILE__,
10	23	'name' => 'SideBarMenu',
—	—	@@ -54,9 +67,4 @@
55	68	'group' => 'ext.sidebarmenu',
56	69	'localBasePath' => dirname( __FILE__ ),
57	70	'remoteExtPath' => 'SideBarMenu'
58		~~-);~~
59		-
60		~~-//default settings~~
61		~~-$wgSideBarMenuConfig['controls.show'] = null;~~
62		~~-$wgSideBarMenuConfig['controls.hide']= null;~~
63		~~-$wgSideBarMenuConfig['js.animate'] = true;~~
\ No newline at end of file
	71	+);
\ No newline at end of file
Index: trunk/extensions/SideBarMenu/SideBarMenu.hooks.php
—	—	@@ -2,6 +2,8 @@
3	3
4	4	class SideBarMenuHooks
5	5	{
	6	+
	7	+
6	8	public static function init(Parser &$parser){
7	9	$parser->setHook('sidebarmenu','SideBarMenuHooks::renderFromTag');
8	10	return true;
—	—	@@ -10,16 +12,20 @@
11	13	public static function renderFromTag( $input, array $args, Parser $parser, PPFrame $frame ){
12	14	$parser->getOutput()->addModules('ext.sidebarmenu.core');
13	15
	16	+ //default settings
	17	+ $config = self::getTagConfig($args);
	18	+
14	19	$output = '<div class="sidebar-menu-container">';
15	20	try{
16		~~- $output .= MenuParser::getMenuTree($input)->toHTML();~~
	21	+ $menuParser = new MenuParser($config[SBM_EXPANDED]);
	22	+ $output .= $menuParser->getMenuTree($input)->toHTML();
17	23	}catch(Exception $x){
18	24	wfDebug("An error occured during parsing of: '$input' caught exception: $x");
19	25	return wfMsg('parser.input-error',$x->getMessage());
20	26	}
21	27	$output .= '</div>';
22	28
23		~~- $jsOutput = self::getJSConfig($args);~~
	29	+ $jsOutput = self::getJSConfig($config);
24	30
25	31	return array( $jsOutput.$parser->recursiveTagParse($output,$frame), 'noparse' => true, 'isHTML' => true );
26	32	}
—	—	@@ -41,40 +47,45 @@
42	48	return true;
43	49	}
44	50
45		~~- private static function minifyJavascript(&$js)~~
	51	+ private static function minifyJavascript($js)
46	52	{
47	53	$js = preg_replace("/[\n\r]/", "", $js); //remove newlines
48	54	$js = preg_replace("/[\s]{2,}/", " ", $js); //remove spaces
	55	+
	56	+ return $js;
49	57	}
50	58
51	59	private static function getAsJSEncodedString($s)
52	60	{
53		~~- return "'" . $s . "'";~~
	61	+ return "'$s'";
54	62	}
55	63
56		~~- private static function getJSConfig(&$args){~~
57		~~- global $wgSideBarMenuConfig;~~
58		~~- //default settings~~
59		~~- $defaults['controls.show'] = isset($wgSideBarMenuConfig['controls.show']) ? $wgSideBarMenuConfig['controls.show'] : wfMsg('controls.show');~~
60		~~- $defaults['controls.hide'] = isset($wgSideBarMenuConfig['controls.hide']) ? $wgSideBarMenuConfig['controls.hide'] : wfMsg('controls.hide');~~
61		~~- $defaults['js.animate'] = $wgSideBarMenuConfig['js.animate'];~~
62		-
	64	+ private static function getJSConfig($config){
63	65	//javascript config output
64	66	$jsOutput = Html::inlineScript("
65	67	var sidebarmenu = {
66	68	config: {
67	69	controls: {
68		~~- show: " . (array_key_exists('controls.show', $args) ? self::getAsJSEncodedString($args['controls.show']) : self::getAsJSEncodedString($defaults['controls.show'])) . ",~~
69		~~- hide: " . (array_key_exists('controls.hide', $args) ? self::getAsJSEncodedString($args['controls.hide']) : self::getAsJSEncodedString($defaults['controls.hide'])) . "~~
	70	+ show: " . self::getAsJSEncodedString($config[SBM_CONTROLS_SHOW]) . ",
	71	+ hide: " . self::getAsJSEncodedString($config[SBM_CONTROLS_HIDE]) . "
70	72	},
71	73	js: {
72		~~- animate: " . (array_key_exists('js.animate', $args) ? is_bool($args['js.animate']) ? 'true' : 'false' : $defaults['js.animate']) . "~~
	74	+ animate: " . (filter_var($config[SBM_JS_ANIMATE], FILTER_VALIDATE_BOOLEAN) ? 'true' : 'false') . "
73	75	}
74	76	}
75	77	};
76	78	");
77	79	//minify js to prevent <p> tags to be rendered
78		~~- self::minifyJavascript($jsOutput);~~
79		~~- return $jsOutput;~~
	80	+ return self::minifyJavascript($jsOutput);
80	81	}
	82	+
	83	+ private static function getTagConfig($args)
	84	+ {
	85	+ global $wgSideBarMenuConfig;
	86	+ $config[SBM_EXPANDED] = array_key_exists(SBM_EXPANDED, $args) ? filter_var($args[SBM_EXPANDED], FILTER_VALIDATE_BOOLEAN) : $wgSideBarMenuConfig[SBM_EXPANDED];
	87	+ $config[SBM_CONTROLS_SHOW] = array_key_exists(SBM_CONTROLS_SHOW, $args) ? $args[SBM_CONTROLS_SHOW] : (isset($wgSideBarMenuConfig[SBM_CONTROLS_SHOW]) ? $wgSideBarMenuConfig[SBM_CONTROLS_SHOW] : wfMsg(SBM_CONTROLS_SHOW));
	88	+ $config[SBM_CONTROLS_HIDE] = array_key_exists(SBM_CONTROLS_HIDE, $args) ? $args[SBM_CONTROLS_HIDE] : (isset($wgSideBarMenuConfig[SBM_CONTROLS_HIDE]) ? $wgSideBarMenuConfig[SBM_CONTROLS_HIDE] : wfMsg(SBM_CONTROLS_HIDE));
	89	+ $config[SBM_JS_ANIMATE] = array_key_exists(SBM_JS_ANIMATE, $args) ? $args[SBM_JS_ANIMATE] : $wgSideBarMenuConfig[SBM_JS_ANIMATE];
	90	+ return $config;
	91	+ }
81	92	}
Index: trunk/extensions/SideBarMenu/includes/MenuParser.php
—	—	@@ -1,65 +1,74 @@
2	2	<?php
3	3	class MenuParser {
4	4
5		~~- public static function isValidInput($data)~~
	5	+ private $expandedByDefault;
	6	+
	7	+ function __construct($expandedByDefault)
6	8	{
	9	+ $this->expandedByDefault = $expandedByDefault;
	10	+ }
	11	+
	12	+
	13	+ public function isValidInput($data)
	14	+ {
7	15	return !(is_null($data) \|\| strlen($data) == 0);
8	16	}
9	17
10	18
11		~~- public static function getLevel($line)~~
	19	+ public function getLevel($line)
12	20	{
13	21	return substr_count($line, '*');
14	22	}
15	23
16		~~- public static function getExpandedParameter($line)~~
	24	+ public function getExpandedParameter($line)
17	25	{
18		~~- if (self::startsWith($line, '+')) {~~
	26	+ if ($this->startsWith($line, '+')) {
19	27	return true;
20		~~- } else {~~
	28	+ } else if($this->startsWith($line, '-')) {
21	29	return false;
22	30	}
	31	+ return $this->expandedByDefault;
23	32	}
24	33
25		~~- public static function getTextParameter($line)~~
	34	+ public function getTextParameter($line)
26	35	{
27	36	return preg_filter("/\+\|\-?\*(.)/", "$1", $line);
28	37	}
29	38
30	39
31		~~- public static function getMenuTree($data){~~
32		~~- if(self::isValidInput($data)){~~
33		~~- $data = self::cleanupData($data);~~
	40	+ public function getMenuTree($data){
	41	+ if($this->isValidInput($data)){
	42	+ $data = $this->cleanupData($data);
34	43	$root = new MenuItem();
35	44	$root->setExpanded(true);
36		~~- $arrayData = self::parseDataIntoHierarchicalArray($data);~~
37		~~- self::addChildrenToMenuItemFromArrayData($root,$arrayData);~~
	45	+ $arrayData = $this->parseDataIntoHierarchicalArray($data);
	46	+ $this->addChildrenToMenuItemFromArrayData($root,$arrayData);
38	47	return $root;
39	48	}
40	49	}
41	50
42	51
43		~~- public static function getMenuItem($line)~~
	52	+ public function getMenuItem($line)
44	53	{
45	54	$line = trim($line);
46		~~- if(self::isValidInput($line)){~~
	55	+ if($this->isValidInput($line)){
47	56	$menuItem = new MenuItem();
48		~~- $menuItem->setExpanded(self::getExpandedParameter($line));~~
49		~~- $menuItem->setText(self::getTextParameter($line));~~
	57	+ $menuItem->setExpanded($this->getExpandedParameter($line));
	58	+ $menuItem->setText($this->getTextParameter($line));
50	59	return $menuItem;
51	60	}else{
52	61	throw new InvalidArgumentException();
53	62	}
54	63	}
55	64
56		~~- public static function parseDataIntoHierarchicalArray($data)~~
	65	+ public function parseDataIntoHierarchicalArray($data)
57	66	{
58	67	$rootArray = array();
59	68	$prevArray = &$rootArray;
60	69	$prevLevel = 0;
61	70	$levelArray[0] = &$rootArray;
62	71	foreach(preg_split("/\n/",$data) as $line){
63		~~- $level = self::getLevel($line);~~
	72	+ $level = $this->getLevel($line);
64	73	if($level == $prevLevel){
65	74	$levelArray[$level][] = $line;
66	75	}else if($level-1 == $prevLevel){
—	—	@@ -79,15 +88,15 @@
80	89	return $rootArray;
81	90	}
82	91
83		~~- public static function addChildrenToMenuItemFromArrayData(&$rootMenuItem,$arrayData)~~
	92	+ public function addChildrenToMenuItemFromArrayData(&$rootMenuItem,$arrayData)
84	93	{
85	94	foreach ($arrayData as $key => $val) {
86	95	if (is_string($key)) {
87		~~- $menuItem = self::getMenuItem($key);~~
	96	+ $menuItem = $this->getMenuItem($key);
88	97	$rootMenuItem->addChild($menuItem);
89		~~- self::addChildrenToMenuItemFromArrayData($menuItem,$val);~~
	98	+ $this->addChildrenToMenuItemFromArrayData($menuItem,$val);
90	99	} else {
91		~~- $menuItem = self::getMenuItem($val);~~
	100	+ $menuItem = $this->getMenuItem($val);
92	101	$rootMenuItem->addChild($menuItem);
93	102	}
94	103	}
Index: trunk/extensions/SideBarMenu/includes/MenuItem.php
—	—	@@ -2,7 +2,7 @@
3	3
4	4	class MenuItem
5	5	{
6		~~- private $expanded;~~
	6	+ private $expanded = false;
7	7	private $children = array();
8	8	private $parent = null;
9	9	private $text;
—	—	@@ -10,7 +10,11 @@
11	11
12	12	public function setExpanded($expanded)
13	13	{
14		~~- $this->expanded = $expanded;~~
	14	+ if(is_null($expanded)){
	15	+ throw new InvalidArgumentException();
	16	+ }else{
	17	+ $this->expanded = $expanded;
	18	+ }
15	19	}
16	20
17	21	public function isExpanded()
Index: trunk/extensions/SideBarMenu/js/ext.sidebarmenu.js
—	—	@@ -1,5 +1,5 @@
2	2	$(document).ready(function(){
3		~~- if(sidebarmenu != undefined){~~
	3	+ if(typeof(sidebarmenu) !== 'undefined'){
4	4	var showText = sidebarmenu.config.controls.show;
5	5	var hideText = sidebarmenu.config.controls.hide;
6	6	var useAnimations = sidebarmenu.config.js.animate;

Status & tagging log

17:47, 7 March 2012 Reedy (talk | contribs) changed the status of r113256 [removed: new added: deferred]