r62957 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r62956‎ | r62957 | r62958 >
Date:15:29, 25 February 2010
Author:catrope
Status:ok (Comments)
Tags:
Comment:
DynamicSidebar: Some cleanup
* Make all functions in the DynamicSidebar class static
* Remove unnecessary entry point check
* Remove unnecessary global $wgFoo; statements on the global level
* $eg -> $wg
* Kill DynamicSidebar::modifySidebarContent(), unnecessary wrapper around modifySidebar()
* Move DynamicSidebarSetup() to DynamicSidebar::setup()
* Use !$title->exists() instead of $a->getID() == 0
* Use User::getUserPage() instead of constructing the user page title manually
Modified paths:
  • /trunk/extensions/DynamicSidebar/DynamicSidebar.body.php (modified) (history)
  • /trunk/extensions/DynamicSidebar/DynamicSidebar.php (modified) (history)

Diff [purge]

Index: trunk/extensions/DynamicSidebar/DynamicSidebar.body.php
——@@ -1,74 +1,64 @@
22 <?php
3 -
4 -if (!defined('MEDIAWIKI')) die();
5 -
63 class DynamicSidebar {
7 -
84 /**
9 - * Called from SkinBeforeParseSidebar hook. Modifies the sidebar
10 - * via callbacks.
11 - *
12 - * @param Skin $skin
13 - * @param string $sidebar
14 - * @access public
 5+ * Called through $wgExtensionFunctions. Disables sidebar cache if necessary
156 */
16 - public function modifySidebarContent( $skin, &$sidebar ) {
17 - $dynamicsidebar = new DynamicSidebar();
18 - $sidebar = $dynamicsidebar->modifySidebar( $skin, $sidebar );
 7+ public static function setup() {
 8+ global $wgUser, $wgEnableSidebarCache;
199
 10+ // Don't pollute the sidebar cache for non-logged-in users
 11+ // Also ensure that logged-in users are getting dynamic content
 12+ // FIXME: Only do this for users who should actually get the non-standard sidebar
 13+ if ( $wgUser->isLoggedIn() ) {
 14+ $wgEnableSidebarCache = false;
 15+ }
2016 return true;
2117 }
2218
2319 /**
24 - * Internal function called to modify the sidebar via callbacks.
 20+ * Called from SkinBeforeParseSidebar hook. Modifies the sidebar
 21+ * via callbacks.
2522 *
2623 * @param Skin $skin
2724 * @param string $sidebar
28 - * @access private
29 - * @return string
3025 */
31 - private function modifySidebar( $skin, $sidebar ) {
32 - global $egDynamicSidebarUseGroups, $egDynamicSidebarUseUserpages;
33 - global $egDynamicSidebarUseCategories;
 26+ private static function modifySidebar( $skin, &$sidebar ) {
 27+ global $wgDynamicSidebarUseGroups, $wgDynamicSidebarUseUserpages;
 28+ global $wgDynamicSidebarUseCategories;
3429
35 - if ( $egDynamicSidebarUseGroups ) {
36 - $sidebar = preg_replace_callback( "/\* GROUP-SIDEBAR/", array( &$this, 'doGroupSidebar' ), $sidebar );
 30+ if ( $wgDynamicSidebarUseGroups ) {
 31+ $sidebar = preg_replace_callback( '/\* GROUP-SIDEBAR/', array( 'self', 'doGroupSidebar' ), $sidebar );
3732 }
38 - if ( $egDynamicSidebarUseUserpages ) {
39 - $sidebar = preg_replace_callback( "/\* USER-SIDEBAR/", array( &$this, 'doUserSidebar' ), $sidebar );
 33+ if ( $wgDynamicSidebarUseUserpages ) {
 34+ $sidebar = preg_replace_callback( '/\* USER-SIDEBAR/', array( 'self', 'doUserSidebar' ), $sidebar );
4035 }
41 - if ( $egDynamicSidebarUseCategories ) {
42 - $sidebar = preg_replace_callback( "/\* CATEGORY-SIDEBAR/", array( &$this, 'doCategorySidebar' ), $sidebar );
 36+ if ( $wgDynamicSidebarUseCategories ) {
 37+ $sidebar = preg_replace_callback( '/\* CATEGORY-SIDEBAR/', array( 'self', 'doCategorySidebar' ), $sidebar );
4338 }
44 -
45 - return $sidebar;
 39+ return true;
4640 }
4741
4842 /**
4943 * Callback function, replaces $matches with the contents of
5044 * User:<username>/Sidebar
5145 *
52 - * @param array $matches
 46+ * @param array $matches unused
5347 * @access private
5448 * @return string
5549 */
56 - private function doUserSidebar( $matches ) {
 50+ private static function doUserSidebar( $matches ) {
5751 global $wgUser;
58 -
5952 $username = $wgUser->getName();
60 -
61 - $title = Title::makeTitle( NS_USER, $username . '/Sidebar' );
62 - $a = new Article( $title );
6353
64 - // does '<username>/Sidebar' page exist?
65 - if ( ( $a === null ) || ( $a->getID() === 0 ) ) {
 54+ // does 'User:<username>/Sidebar' page exist?
 55+ $title = Title::makeTitle( NS_USER, $username . '/Sidebar' );
 56+ if ( !$title->exists() ) {
6657 // Remove this sidebar if not
6758 return '';
6859 }
6960
70 - $text = $a->getContent();
71 -
72 - return $text;
 61+ $a = new Article( $title );
 62+ return $a->getContent();
7363 }
7464
7565 /**
——@@ -76,16 +66,15 @@
7767 * MediaWiki:Sidebar/<group>, based on the current logged in user's
7868 * groups.
7969 *
80 - * @param array $matches
 70+ * @param array $matches unused
8171 * @access private
8272 * @return string
8373 */
84 - private function doGroupSidebar( $matches ) {
 74+ private static function doGroupSidebar( $matches ) {
8575 global $wgUser;
86 -
 76+
8777 // Get group membership array.
8878 $groups = $wgUser->getEffectiveGroups();
89 -
9079 // Did we find any groups?
9180 if ( count( $groups ) == 0 ) {
9281 // Remove this sidebar if not
——@@ -93,22 +82,17 @@
9483 }
9584
9685 $text = '';
97 -
9886 foreach ( $groups as $group ) {
9987 // Form the path to the article:
10088 // MediaWiki:Sidebar/<group>
10189 $title = Title::makeTitle( NS_MEDIAWIKI, 'Sidebar/' . $group );
102 - $a = new Article( $title );
103 -
104 - // Is the corresponding page found?
105 - if ( ( $a === null ) || ( $a->getID() === 0 ) ) {
 90+ if ( !$title->exists() ) {
10691 continue;
10792 }
108 -
 93+ $a = new Article( $title );
10994 $text .= $a->getContent() . "\n";
11095
11196 }
112 -
11397 return $text;
11498 }
11599
——@@ -117,17 +101,15 @@
118102 * MediaWiki:Sidebar/<category>, based on the current logged in user's
119103 * userpage categories.
120104 *
121 - * @param array $matches
 105+ * @param array $matches unused
122106 * @access private
123107 * @return string
124108 */
125 - private function doCategorySidebar( $matches ) {
 109+ private static function doCategorySidebar( $matches ) {
126110 global $wgUser;
127111
128 - $username = $wgUser->getName();
129 - self::printDebug( "User name: $username" );
130 - $userpage = Title::makeTitle( NS_USER, $username );
131 - $categories = $userpage->getParentCategories();
 112+ self::printDebug( "User name: {$wgUser->getName()}" );
 113+ $categories = $wgUser->getUserPage()->getParentCategories();
132114
133115 // Did we find any categories?
134116 if ( count( $categories ) == 0 ) {
——@@ -136,11 +118,10 @@
137119 }
138120
139121 $text = '';
140 -
141122 // getParentCategories() returns categories in the form:
142123 // [ParentCategory] => page
143124 // We only care about the parent category
144 - foreach ( $categories as $category => $userpage ) {
 125+ foreach ( $categories as $category => $unused ) {
145126 // $category is in form Category:<category>
146127 // We need <category>.
147128 $category = explode( ":", $category );
——@@ -150,16 +131,12 @@
151132 // Form the path to the article:
152133 // MediaWiki:Sidebar/<category>
153134 $title = Title::makeTitle( NS_MEDIAWIKI, 'Sidebar/' . $category );
154 - $a = new Article( $title );
155 -
156 - // Is the corresponding page found?
157 - if ( ( $a === null ) || ( $a->getID() === 0 ) ) {
 135+ if ( !$title->exists() ) {
158136 continue;
159137 }
160 -
 138+ $a = new Article( $title );
161139 $text .= $a->getContent() . "\n";
162140 }
163 -
164141 return $text;
165142 }
166143
——@@ -172,9 +149,9 @@
173150 * @access private
174151 */
175152 private static function printDebug( $debugText, $debugArr = null ) {
176 - global $egDynamicSidebarDebug;
 153+ global $wgDynamicSidebarDebug;
177154
178 - if ( $egDynamicSidebarDebug ) {
 155+ if ( $wgDynamicSidebarDebug ) {
179156 if ( isset( $debugArr ) ) {
180157 $text = $debugText . " " . implode( "::", $debugArr );
181158 wfDebugLog( 'dynamic-sidebar', $text, false );
——@@ -183,5 +160,4 @@
184161 }
185162 }
186163 }
187 -
188164 }
Index: trunk/extensions/DynamicSidebar/DynamicSidebar.php
——@@ -25,15 +25,11 @@
2626 }
2727
2828 // Set defaults
29 -global $egDynamicSidebarDebug, $egDynamicSidebarUseGroups, $egDynamicSidebarUseUserpages;
30 -global $egDynamicSidebarUseCategories;
 29+$wgDynamicSidebarDebug = false;
 30+$wgDynamicSidebarUseUserpages = true;
 31+$wgDynamicSidebarUseGroups = true;
 32+$wgDynamicSidebarUseCategories = true;
3133
32 -$egDynamicSidebarDebug = false;
33 -$egDynamicSidebarUseUserpages = true;
34 -$egDynamicSidebarUseGroups = true;
35 -$egDynamicSidebarUseCategories = true;
36 -
37 -global $wgExtensionCredits;
3834 $wgExtensionCredits['other'][] = array(
3935 'name' => 'DynamicSidebar',
4036 'version' => '1.0a',
——@@ -41,23 +37,9 @@
4238 'url' => 'http://www.mediawiki.org/wiki/Extension:DynamicSidebar',
4339 'description' => "Provides dynamic sidebars based on user pages, groups, and categories.",
4440 );
45 -
46 -$wgExtensionFunctions[] = 'DynamicSidebarSetupExtension';
4741
 42+$wgExtensionFunctions[] = 'DynamicSidebar::setup';
 43+$wgHooks['SkinBeforeParseSidebar'][] = 'DynamicSidebar::modifySidebar';
 44+
4845 $dir = dirname( __FILE__ ) . '/';
4946 $wgAutoloadClasses['DynamicSidebar'] = $dir . 'DynamicSidebar.body.php';
50 -
51 -function DynamicSidebarSetupExtension() {
52 - global $wgHooks;
53 - global $wgUser, $wgEnableSidebarCache;
54 -
55 - // Don't pollute the sidebar cache for non-loggedin users
56 - // Also ensure that loggedin users are getting dynamic content
57 - if ( $wgUser->isLoggedIn() ) {
58 - $wgEnableSidebarCache = false;
59 - }
60 -
61 - $wgHooks['SkinBeforeParseSidebar'][] = 'DynamicSidebar::modifySidebarContent';
62 -
63 - return true;
64 -}

Follow-up revisions

RevisionCommit summaryAuthorDate
r62969Follow-up r62957: Make function public. Fixes "Warning: call_user_func_array(...raymond19:03, 25 February 2010

Comments

#Comment by Ryan lane (talk | contribs)   19:32, 25 February 2010

I'm getting the following error from setup():

PHP Warning: call_user_func(DynamicSidebar::setup) [<a href='function.call-user-func'>function.call-user-func</a>]: Unable to call DynamicSidebar::setup() in /apps1/sandbox/includes/Setup.php on line 306

#Comment by Catrope (talk | contribs)   20:01, 25 February 2010

Raymond fixed this in r62969

Status & tagging log