r54217 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r54216‎ | r54217 | r54218 >
Date:16:24, 2 August 2009
Author:ashley
Status:deferred
Tags:
Comment:
DPLforum:
*readability tweaks
*added __METHOD__ to SQL query
*doxygen tweaks
*fixed bug in DPLForum::link when running under new parser (code by wikia's techs)
*bumped version number
*else if => elseif, NULL => null
Modified paths:
  • /trunk/extensions/DPLforum/DPLforum.i18n.php (modified) (history)
  • /trunk/extensions/DPLforum/DPLforum.php (modified) (history)
  • /trunk/extensions/DPLforum/DPLforum_body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/DPLforum/DPLforum_body.php
——@@ -1,5 +1,5 @@
22 <?php
3 -/*
 3+/**
44 DPLforum v3.2 -- DynamicPageList-based forum extension
55
66 Author: Ross McClure
——@@ -24,7 +24,8 @@
2525 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
2626 http://www.gnu.org/copyleft/gpl.html
2727
28 - * @addtogroup Extensions
 28+ * @file
 29+ * @ingroup Extensions
2930 */
3031
3132 if ( !defined( 'MEDIAWIKI' ) ) {
——@@ -66,27 +67,32 @@
6768 foreach ( $matches[1] as $cat ) {
6869 $title = Title::newFromText( $parser->replaceVariables( trim( $cat ) ) );
6970 if ( !is_null( $title ) )
70 - $cats[] = $title;
 71+ $cats[] = $title;
7172 }
7273 }
7374 return $cats;
7475 }
7576
76 - function get( $name, $value = NULL, $parser = NULL ) {
 77+ function get( $name, $value = null, $parser = null ) {
7778 if ( preg_match( "/^\s*$name\s*=\s*(.*)/mi", $this->sInput, $matches ) ) {
7879 $arg = trim( $matches[1] );
79 - if ( is_int( $value ) ) return intval( $arg );
80 - else if ( is_null( $parser ) ) return htmlspecialchars( $arg );
81 - else return $parser->replaceVariables( $arg );
 80+ if ( is_int( $value ) )
 81+ return intval( $arg );
 82+ elseif ( is_null( $parser ) )
 83+ return htmlspecialchars( $arg );
 84+ else
 85+ return $parser->replaceVariables( $arg );
8286 }
8387 return $value;
8488 }
8589
8690 function link( &$parser, $count, $page = '', $text = '' ) {
8791 $count = intval( $count );
88 - if ( $count < 1 ) return '';
 92+ if ( $count < 1 )
 93+ return '';
8994
90 - if ( $this->requireCache ) $offset = 0;
 95+ if ( $this->requireCache )
 96+ $offset = 0;
9197 else {
9298 global $wgRequest;
9399 $parser->disableCache();
——@@ -94,15 +100,20 @@
95101 }
96102
97103 $i = intval( $page );
98 - if ( ( $i != 0 ) && ctype_digit( $page[0] ) ) $i -= 1;
99 - else $i += intval( $offset / $count );
100 - if ( $this->link_test( $i, $page ) ) return '';
 104+ if ( ( $i != 0 ) && ctype_digit( $page[0] ) )
 105+ $i -= 1;
 106+ else
 107+ $i += intval( $offset / $count );
 108+ if ( $this->link_test( $i, $page ) )
 109+ return '';
101110
102 - if ( $text === '' ) $text = ( $i + 1 );
 111+ if ( $text === '' )
 112+ $text = ( $i + 1 );
103113 $page = ( $count * $i );
104 - if ( $page == $offset ) return $text;
 114+ if ( $page == $offset )
 115+ return $text;
105116
106 - return '[{{fullurl:{{FULLPAGENAME}}|offset=' . $page . '}} ' . $text . ']';
 117+ return '[' . $parser->replaceVariables( '{{fullurl:{{FULLPAGENAME}}|offset=' . $page . '}} ' ) . $text . ']';
107118 }
108119
109120 function link_test( $page, $cond ) {
——@@ -110,16 +121,20 @@
111122 $m[1] = strtr( $m[1], array( ( '&l' . 't;' ) => '<', ( '&g' . 't;' ) => '>' ) );
112123 $m[2] = intval( $m[2] ) - 1;
113124 switch( $m[1] ) {
114 - case '<': return ( $page >= $m[2] );
115 - case '>': return ( $page <= $m[2] );
116 - case '<=': return ( $page > $m[2] );
117 - case '>=': return ( $page < $m[2] );
 125+ case '<':
 126+ return ( $page >= $m[2] );
 127+ case '>':
 128+ return ( $page <= $m[2] );
 129+ case '<=':
 130+ return ( $page > $m[2] );
 131+ case '>=':
 132+ return ( $page < $m[2] );
118133 }
119134 }
120135 return ( $page < 0 );
121136 }
122137
123 - function msg( $type, $error = NULL ) {
 138+ function msg( $type, $error = null ) {
124139 if ( $error && ( $this->get( 'suppresserrors' ) == 'true' ) )
125140 return '';
126141
——@@ -140,18 +155,20 @@
141156
142157 switch( $this->get( 'historylink' ) ) {
143158 case 'embed':
144 - case 'true': $this->bEmbedHistory = true;
 159+ case 'true':
 160+ $this->bEmbedHistory = true;
145161 case 'append':
146 - case 'show': $this->bLinkHistory = true;
 162+ case 'show':
 163+ $this->bLinkHistory = true;
147164 }
148165 $sOrder = 'rev_timestamp';
149166 switch( $this->get( 'ordermethod' ) ) {
150167 case 'categoryadd':
151168 case 'created':
152 - $sOrder = 'first_time';
 169+ $sOrder = 'first_time';
153170 break;
154171 case 'pageid':
155 - $sOrder = 'page_id';
 172+ $sOrder = 'page_id';
156173 }
157174
158175 $arg = $this->get( 'compact' );
——@@ -162,12 +179,15 @@
163180 $arg = $this->get( 'namespace', '', $parser );
164181 $iNamespace = $wgContLang->getNsIndex( $arg );
165182 if ( !$iNamespace ) {
166 - if ( ( $arg ) || ( $arg === '0' ) ) $iNamespace = intval( $arg );
167 - else $iNamespace = - 1;
 183+ if ( ( $arg ) || ( $arg === '0' ) )
 184+ $iNamespace = intval( $arg );
 185+ else
 186+ $iNamespace = - 1;
168187 }
169188 if ( $iNamespace < 0 )
170189 $this->bShowNamespace = ( $this->get( 'shownamespace' ) != 'false' );
171 - else $this->bShowNamespace = ( $this->get( 'shownamespace' ) == 'true' );
 190+ else
 191+ $this->bShowNamespace = ( $this->get( 'shownamespace' ) == 'true' );
172192
173193 $this->bTableMode = false;
174194 $sStartItem = $sEndItem = '';
——@@ -175,22 +195,22 @@
176196 $arg = $this->get( 'mode' );
177197 switch( $arg ) {
178198 case 'none':
179 - $sEndItem = '<br />';
 199+ $sEndItem = '<br />';
180200 break;
181201 case 'count':
182 - $bCountMode = true;
 202+ $bCountMode = true;
183203 break;
184204 case 'list':
185205 case 'ordered':
186206 case 'unordered':
187 - $sStartItem = '<li>';
188 - $sEndItem = '</li>';
 207+ $sStartItem = '<li>';
 208+ $sEndItem = '</li>';
189209 break;
190210 case 'table':
191211 default:
192 - $this->bTableMode = true;
193 - $sStartItem = '<tr>';
194 - $sEndItem = '</tr>';
 212+ $this->bTableMode = true;
 213+ $sStartItem = '<tr>';
 214+ $sEndItem = '</tr>';
195215 }
196216 $aCategories = $this->cat( $parser, 'category' );
197217 $aExcludeCategories = $this->cat( $parser, 'notcategory' );
——@@ -200,14 +220,15 @@
201221 $output = '';
202222
203223 if ( $sPrefix === '' && ( ( $cats < 1 && $iNamespace < 0 ) ||
204 - ( $total < $this->minCategories ) ) ) return $this->msg( 'dplforum-toofew', 1 );
 224+ ( $total < $this->minCategories ) ) )
 225+ return $this->msg( 'dplforum-toofew', 1 );
205226 if ( ( $total > $this->maxCategories ) && ( !$this->unlimitedCategories ) )
206 - return $this->msg( 'dplforum-toomany', 1 );
 227+ return $this->msg( 'dplforum-toomany', 1 );
207228
208229 $count = 1;
209230 $start = $this->get( 'start', 0 );
210231 $title = Title::newFromText( $parser->replaceVariables(
211 - trim( $this->get( 'title' ) ) ) );
 232+ trim( $this->get( 'title' ) ) ) );
212233 if ( !( $bCountMode || $this->requireCache || $this->get( 'cache' ) == 'true' ) ) {
213234 $parser->disableCache();
214235
——@@ -216,22 +237,22 @@
217238 $start += intval( $wgRequest->getVal( 'offset' ) );
218239 }
219240 }
220 - if ( $start < 0 ) $start = 0;
 241+ if ( $start < 0 )
 242+ $start = 0;
221243
222244 if ( is_null( $title ) ) {
223245 $count = $this->get( 'count', 0 );
224246 if ( $count > 0 ) {
225247 if ( $count > $this->maxResultCount )
 248+ $count = $this->maxResultCount;
 249+ } elseif ( $this->unlimitedResults )
 250+ $count = 0x7FFFFFFF; // maximum integer value
 251+ else
226252 $count = $this->maxResultCount;
227 - }
228 - else if ( $this->unlimitedResults )
229 - $count = 0x7FFFFFFF; // maximum integer value
230 - else
231 - $count = $this->maxResultCount;
232253 }
233254
234255 // build the SQL query
235 - $dbr =& wfGetDB( DB_SLAVE );
 256+ $dbr = wfGetDB( DB_SLAVE );
236257 $sPageTable = $dbr->tableName( 'page' );
237258 $sRevTable = $dbr->tableName( 'revision' );
238259 $categorylinks = $dbr->tableName( 'categorylinks' );
——@@ -242,8 +263,7 @@
243264
244265 if ( $bCountMode ) {
245266 $sSqlSelectFrom = "SELECT COUNT(*) AS num_rows FROM $sPageTable";
246 - }
247 - else if ( ( $this->bAddAuthor || $this->bAddCreationDate ||
 267+ } elseif ( ( $this->bAddAuthor || $this->bAddCreationDate ||
248268 ( $sOrder == 'first_time' ) ) && ( ( !$this->restrictNamespace ) ||
249269 ( $iNamespace >= 0 && !in_array( $iNamespace, $this->restrictNamespace ) ) ) ) {
250270 $sSqlSelectFrom .= ", o.rev_user_text AS first_user, o.rev_timestamp AS"
——@@ -251,13 +271,14 @@
252272 . " ON o.rev_id =( SELECT MIN(q.rev_id) FROM $sRevTable"
253273 . " AS q WHERE q.rev_page = page_id )";
254274 } else {
255 - if ( $sOrder == 'first_time' ) $sOrder = 'page_id';
 275+ if ( $sOrder == 'first_time' )
 276+ $sOrder = 'page_id';
256277 $sSqlSelectFrom .= $arg;
257278 }
258279
259280 $sSqlWhere = ' WHERE 1=1';
260281 if ( $iNamespace >= 0 )
261 - $sSqlWhere = ' WHERE page_namespace=' . $iNamespace;
 282+ $sSqlWhere = ' WHERE page_namespace=' . $iNamespace;
262283
263284 if ( $sPrefix !== '' ) {
264285 // Escape SQL special characters
——@@ -268,12 +289,12 @@
269290
270291 switch( $this->get( 'redirects' ) ) {
271292 case 'only':
272 - $sSqlWhere .= ' AND page_is_redirect = 1';
 293+ $sSqlWhere .= ' AND page_is_redirect = 1';
273294 case 'include':
274295 break;
275296 case 'exclude':
276297 default:
277 - $sSqlWhere .= ' AND page_is_redirect = 0';
 298+ $sSqlWhere .= ' AND page_is_redirect = 0';
278299 break;
279300 }
280301
——@@ -307,10 +328,10 @@
308329 // $output .= 'QUERY: [' . $sSqlSelectFrom . $sSqlWhere . "]<br />";
309330
310331 // process the query
311 - $res = $dbr->query( $sSqlSelectFrom . $sSqlWhere );
 332+ $res = $dbr->query( $sSqlSelectFrom . $sSqlWhere, __METHOD__ );
312333
313334 $this->vMarkNew = $dbr->timestamp( time() -
314 - intval( $this->get( 'newdays', 7 ) * 86400 ) );
 335+ intval( $this->get( 'newdays', 7 ) * 86400 ) );
315336
316337 if ( $bCountMode ) {
317338 if ( $row = $dbr->fetchObject( $res ) ) {
——@@ -318,8 +339,7 @@
319340 } else {
320341 $output .= '0';
321342 }
322 - }
323 - else if ( is_null( $title ) ) {
 343+ } elseif ( is_null( $title ) ) {
324344 while ( $row = $dbr->fetchObject( $res ) ) {
325345 if( isset( $row->first_time ) ) {
326346 $first_time = $row->first_time;
——@@ -336,16 +356,16 @@
337357 $title = Title::makeTitle( $row->page_namespace, $row->page_title );
338358 $output .= $sStartItem;
339359 $output .= $this->buildOutput( $title, $title, $row->rev_timestamp,
340 - $row->rev_user_text, $first_user, $first_time );
 360+ $row->rev_user_text, $first_user, $first_time );
341361 $output .= $sEndItem . "\n";
342362 }
343363 } else {
344364 $output .= $sStartItem;
345365 if ( $row = $dbr->fetchObject( $res ) ) {
346366 $output .= $this->buildOutput( Title::makeTitle( $row->page_namespace,
347 - $row->page_title ), $title, $row->rev_timestamp, $row->rev_user_text );
 367+ $row->page_title ), $title, $row->rev_timestamp, $row->rev_user_text );
348368 } else {
349 - $output .= $this->buildOutput( NULL, $title, $this->msg( 'dplforum-never' ) );
 369+ $output .= $this->buildOutput( null, $title, $this->msg( 'dplforum-never' ) );
350370 }
351371 $output .= $sEndItem . "\n";
352372 }
——@@ -356,7 +376,7 @@
357377 function buildOutput( $page, $title, $time, $user = '', $author = '', $made = '' ) {
358378 global $wgLang, $wgUser;
359379
360 - $sk =& $wgUser->getSkin();
 380+ $sk = $wgUser->getSkin();
361381 $tm =& $this->bTableMode;
362382 $output = '';
363383
——@@ -376,7 +396,7 @@
377397
378398 if ( $tm ) {
379399 $output .= "<td class='forum_created'>$made</td>";
380 - } else if ( $made ) {
 400+ } elseif ( $made ) {
381401 $output = "{$made}: ";
382402 }
383403 }
——@@ -420,7 +440,7 @@
421441 if ( $tm ) {
422442 if ( $this->bCompactAuthor ) {
423443 if ( $author ) {
424 - $byAuthor = wfMsg( 'word-separator') . wfMsgHTML( 'dplforum-by', $author );
 444+ $byAuthor = wfMsg( 'word-separator' ) . wfMsgHtml( 'dplforum-by', $author );
425445 $output .= " <span class='forum_author'>$byAuthor</span>";
426446 } else {
427447 $output .= " <span class='forum_author'>&nb" . "sp;</span>";
——@@ -428,8 +448,8 @@
429449 } else {
430450 $output .= "</td><td class='forum_author'>$author";
431451 }
432 - } else if ( $author ) {
433 - $byAuthor = wfMsg( 'word-separator') . wfMsgHTML( 'dplforum-by', $author );
 452+ } elseif ( $author ) {
 453+ $byAuthor = wfMsg( 'word-separator' ) . wfMsgHtml( 'dplforum-by', $author );
434454 $output .= $byAuthor;
435455 }
436456 }
——@@ -465,7 +485,7 @@
466486 if ( $tm ) {
467487 if ( $this->bCompactEdit ) {
468488 if ( $user ) {
469 - $byUser = wfMsgHTML( 'dplforum-by', $user );
 489+ $byUser = wfMsgHtml( 'dplforum-by', $user );
470490 $output .= " <span class='forum_editor'>$byUser</span>";
471491 } else {
472492 $output .= " <span class='forum_editor'>&nb" . "sp;</span>";
——@@ -473,16 +493,15 @@
474494 } else {
475495 $output .= "</td><td class='forum_editor'>$user";
476496 }
477 - }
478 - else if ( $user ) {
479 - $byUser = wfMsgHTML( 'dplforum-by', $user );
 497+ } elseif ( $user ) {
 498+ $byUser = wfMsgHtml( 'dplforum-by', $user );
480499 $text .= $byUser;
481500 }
482501 }
483502
484503 if ( $tm ) {
485 - $output .= "</td>";
486 - } else if ( $text ) {
 504+ $output .= '</td>';
 505+ } elseif ( $text ) {
487506 $output .= wfMsg( 'word-separator' ) . $this->msg( 'dplforum-edited' ) . " $text";
488507 }
489508
Index: trunk/extensions/DPLforum/DPLforum.i18n.php
——@@ -1,8 +1,9 @@
22 <?php
33 /**
4 - * Internationalization file for DPLforum extension
 4+ * Internationalization file for DPLforum extension.
55 *
6 - * @addtogroup Extensions
 6+ * @file
 7+ * @ingroup Extensions
78 */
89
910 $messages = array();
——@@ -11,11 +12,11 @@
1213 * @author Ross McClure
1314 */
1415 $messages['en'] = array(
15 - 'dplforum-desc' => 'DPL-based forum extension',
16 - 'dplforum-by' => 'by $1',
17 - 'dplforum-edited' => '- Last edited',
18 - 'dplforum-never' => 'Never',
19 - 'dplforum-toofew' => 'DPL Forum: Too few categories!',
 16+ 'dplforum-desc' => 'DPL-based forum extension',
 17+ 'dplforum-by' => 'by $1',
 18+ 'dplforum-edited' => '- Last edited',
 19+ 'dplforum-never' => 'Never',
 20+ 'dplforum-toofew' => 'DPL Forum: Too few categories!',
2021 'dplforum-toomany' => 'DPL Forum: Too many categories!'
2122 );
2223
Index: trunk/extensions/DPLforum/DPLforum.php
——@@ -1,7 +1,7 @@
22 <?php
3 -/*
 3+/**
44
5 - DPLforum v3.2 -- DynamicPageList-based forum extension
 5+ DPLforum v3.3.1 -- DynamicPageList-based forum extension
66
77 Author: Ross McClure
88 http://www.mediawiki.org/wiki/User:Algorithm
——@@ -38,13 +38,13 @@
3939 $wgExtensionFunctions[] = 'wfDPLforum';
4040 $wgHooks['LanguageGetMagic'][] = 'wfDPLmagic';
4141 $wgExtensionCredits['parserhook'][] = array(
42 - 'path' => __FILE__,
43 - 'name' => 'DPLforum',
44 - 'url' => 'http://www.mediawiki.org/wiki/Extension:DPLforum',
45 - 'description' => 'DPL-based forum extension',
 42+ 'path' => __FILE__,
 43+ 'name' => 'DPLforum',
 44+ 'author' => 'Ross McClure',
 45+ 'version' => '3.3.1',
 46+ 'url' => 'http://www.mediawiki.org/wiki/Extension:DPLforum',
 47+ 'description' => 'DPL-based forum extension',
4648 'descriptionmsg' => 'dplforum-desc',
47 - 'author' => 'Ross McClure',
48 - 'version' => '3.3'
4949 );
5050
5151 $dir = dirname( __FILE__ ) . '/';
——@@ -59,9 +59,10 @@
6060 $wgParser->setFunctionHook( 'forumlink', array( new DPLForum(), 'link' ) );
6161 }
6262
63 -function wfDPLmagic( &$magicWords, $langCode = "en" ) {
 63+function wfDPLmagic( &$magicWords, $langCode = 'en' ) {
6464 switch( $langCode ) {
65 - default: $magicWords['forumlink'] = array ( 0, 'forumlink' );
 65+ default:
 66+ $magicWords['forumlink'] = array( 0, 'forumlink' );
6667 }
6768 return true;
6869 }

Status & tagging log