r82933 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r82932‎ | r82933 | r82934 >
Date:13:58, 28 February 2011
Author:ashley
Status:ok (Comments)
Tags:
Comment:
Intersection/DynamicPageList: add check for MediaWiki environment, add version number to extension credits, initial stylizing (missing braces added, Yoda conditionals fixed, some variables renamed)
Modified paths:
  • /trunk/extensions/intersection/DynamicPageList.php (modified) (history)

Diff [purge]

Index: trunk/extensions/intersection/DynamicPageList.php
——@@ -1,6 +1,6 @@
22 <?php
33 /*
4 -
 4+
55 Purpose: outputs a bulleted list of most recent
66 items residing in a category, or a union
77 of several categories.
——@@ -26,41 +26,52 @@
2727
2828 Current feature request list
2929 1. Unset cached of calling page
30 - 4. RSS feed output? (GNSM extension?)
 30+ 2. RSS feed output? (GNSM extension?)
3131
3232 To install, add following to LocalSettings.php
33 - include("extensions/intersection/DynamicPageList.php");
 33+ include("$IP/extensions/intersection/DynamicPageList.php");
3434
3535 */
3636
37 -$wgDLPmaxCategories = 6; // Maximum number of categories to look for
38 -$wgDLPMaxResultCount = 200; // Maximum number of results to allow
39 -$wgDLPAllowUnlimitedResults = false; // Allow unlimited results
40 -$wgDLPAllowUnlimitedCategories = false; // Allow unlimited categories
 37+if ( !defined( 'MEDIAWIKI' ) ) {
 38+ die( 'This is not a valid entry point to MediaWiki.' );
 39+}
4140
42 -$wgHooks['ParserFirstCallInit'][] = 'wfDynamicPageList';
43 -
 41+// Extension credits that will show up on Special:Version
4442 $wgExtensionCredits['parserhook'][] = array(
4543 'path' => __FILE__,
4644 'name' => 'DynamicPageList',
 45+ 'version' => '1.5',
4746 'descriptionmsg' => 'intersection-desc',
4847 'url' => 'http://www.mediawiki.org/wiki/Extension:Intersection',
4948 'author' => array( '[http://en.wikinews.org/wiki/User:Amgine Amgine]', '[http://en.wikinews.org/wiki/User:IlyaHaykinson IlyaHaykinson]' ),
5049 );
5150
52 -$dir = dirname(__FILE__) . '/';
 51+// Internationalization file
 52+$dir = dirname( __FILE__ ) . '/';
5353 $wgExtensionMessagesFiles['DynamicPageList'] = $dir . 'DynamicPageList.i18n.php';
5454
 55+# Configuration variables
 56+$wgDLPmaxCategories = 6; // Maximum number of categories to look for
 57+$wgDLPMaxResultCount = 200; // Maximum number of results to allow
 58+$wgDLPAllowUnlimitedResults = false; // Allow unlimited results
 59+$wgDLPAllowUnlimitedCategories = false; // Allow unlimited categories
 60+
 61+$wgHooks['ParserFirstCallInit'][] = 'wfDynamicPageList';
 62+/**
 63+ * Set up the <DynamicPageList> tag.
 64+ *
 65+ * @param $parser Object: instance of Parser
 66+ * @return Boolean: true
 67+ */
5568 function wfDynamicPageList( &$parser ) {
56 - $parser->setHook( "DynamicPageList", "DynamicPageList" );
 69+ $parser->setHook( 'DynamicPageList', 'renderDynamicPageList' );
5770 return true;
5871 }
5972
6073 // The callback function for converting the input text to HTML output
61 -function DynamicPageList( $input ) {
62 - global $wgUser;
63 - global $wgLang;
64 - global $wgContLang;
 74+function renderDynamicPageList( $input ) {
 75+ global $wgUser, $wgLang, $wgContLang;
6576 global $wgDisableCounters; // to determine if to allow sorting by #hits.
6677 global $wgDLPmaxCategories, $wgDLPMaxResultCount;
6778 global $wgDLPAllowUnlimitedResults, $wgDLPAllowUnlimitedCategories;
——@@ -90,9 +101,9 @@
91102
92103 $bNamespace = false;
93104 $iNamespace = 0;
94 -
 105+
95106 $iOffset = 0;
96 -
 107+
97108 $bGoogleHack = false;
98109
99110 $bSuppressErrors = false;
——@@ -105,38 +116,44 @@
106117 $aCategories = array();
107118 $aExcludeCategories = array();
108119
109 - $aParams = explode("\n", $input);
 120+ $aParams = explode( "\n", $input );
110121
111122 $parser = new Parser;
112123 $poptions = new ParserOptions;
113124
114125 foreach ( $aParams as $sParam ) {
115 - $aParam = explode( "=", $sParam, 2 );
 126+ $aParam = explode( '=', $sParam, 2 );
116127 if( count( $aParam ) < 2 ) {
117128 continue;
118129 }
119 - $sType = trim($aParam[0]);
120 - $sArg = trim($aParam[1]);
 130+ $sType = trim( $aParam[0] );
 131+ $sArg = trim( $aParam[1] );
121132 switch ( $sType ) {
122133 case 'category':
123 - $title = Title::newFromText( $parser->transformMsg($sArg, $poptions) );
124 - if( is_null( $title ) )
 134+ $title = Title::newFromText(
 135+ $parser->transformMsg( $sArg, $poptions )
 136+ );
 137+ if( is_null( $title ) ) {
125138 continue;
 139+ }
126140 $aCategories[] = $title;
127141 break;
128142 case 'notcategory':
129 - $title = Title::newFromText( $parser->transformMsg($sArg, $poptions) );
130 - if( is_null( $title ) )
 143+ $title = Title::newFromText(
 144+ $parser->transformMsg( $sArg, $poptions )
 145+ );
 146+ if( is_null( $title ) ) {
131147 continue;
 148+ }
132149 $aExcludeCategories[] = $title;
133150 break;
134151 case 'namespace':
135 - $ns = $wgContLang->getNsIndex($sArg);
136 - if ( null != $ns ) {
 152+ $ns = $wgContLang->getNsIndex( $sArg );
 153+ if ( $ns != null ) {
137154 $iNamespace = $ns;
138155 $bNamespace = true;
139156 } else {
140 - $iNamespace = intval($sArg);
 157+ $iNamespace = intval( $sArg );
141158 if ( $iNamespace >= 0 ) {
142159 $bNamespace = true;
143160 } else {
——@@ -145,21 +162,21 @@
146163 }
147164 break;
148165 case 'count':
149 - //ensure that $iCount is a number;
150 - $iCount = IntVal( $sArg );
 166+ // ensure that $iCount is a number;
 167+ $iCount = intval( $sArg );
151168 $bCountSet = true;
152169 break;
153170 case 'offset':
154 - $iOffset = IntVal( $sArg );
 171+ $iOffset = intval( $sArg );
155172 break;
156173 case 'imagewidth':
157 - $iGalleryImageWidth = IntVal( $sArg );
 174+ $iGalleryImageWidth = intval( $sArg );
158175 break;
159176 case 'imageheight':
160 - $iGalleryImageHeight = IntVal( $sArg );
 177+ $iGalleryImageHeight = intval( $sArg );
161178 break;
162179 case 'imagesperrow':
163 - $iGalleryNumbRows = IntVal( $sArg );
 180+ $iGalleryNumbRows = intval( $sArg );
164181 break;
165182 case 'mode':
166183 switch ( $sArg ) {
——@@ -186,7 +203,7 @@
187204 $bInlineMode = false;
188205 break;
189206 case 'inline':
190 - //aka comma seperated list
 207+ // aka comma seperated list
191208 $sStartList = '';
192209 $sEndList = '';
193210 $sStartItem = '';
——@@ -205,7 +222,7 @@
206223 case 'gallerycaption':
207224 // Should perhaps actually parse caption instead
208225 // as links and what not in caption might be useful.
209 - $sGalleryCaption = $parser->transformMsg( $sArg, $poptions );
 226+ $sGalleryCaption = $parser->transformMsg( $sArg, $poptions );
210227 break;
211228 case 'galleryshowfilesize':
212229 switch ( $sArg ) {
——@@ -316,14 +333,14 @@
317334 }
318335 break;
319336 case 'suppresserrors':
320 - if ( 'true' == $sArg ) {
 337+ if ( $sArg == 'true' ) {
321338 $bSuppressErrors = true;
322339 } else {
323340 $bSuppressErrors = false;
324341 }
325342 break;
326343 case 'addfirstcategorydate':
327 - if ( 'true' == $sArg ) {
 344+ if ( $sArg == 'true' ) {
328345 $bAddFirstCategoryDate = true;
329346 } elseif ( preg_match( '/^(?:[ymd]{2,3}|ISO 8601)$/', $sArg ) ) {
330347 // if it more or less is valid dateformat.
——@@ -359,12 +376,12 @@
360377 } // end main switch()
361378 } // end foreach()
362379
363 - $iCatCount = count($aCategories);
364 - $iExcludeCatCount = count($aExcludeCategories);
 380+ $iCatCount = count( $aCategories );
 381+ $iExcludeCatCount = count( $aExcludeCategories );
365382 $iTotalCatCount = $iCatCount + $iExcludeCatCount;
366383
367384 if ( $iCatCount < 1 && false == $bNamespace ) {
368 - if ( false == $bSuppressErrors ) {
 385+ if ( $bSuppressErrors == false ) {
369386 return htmlspecialchars( wfMsg( 'intersection_noincludecats' ) ); // "!!no included categories!!";
370387 } else {
371388 return '';
——@@ -372,7 +389,7 @@
373390 }
374391
375392 if ( $iTotalCatCount > $wgDLPmaxCategories && !$wgDLPAllowUnlimitedCategories ) {
376 - if ( false == $bSuppressErrors ) {
 393+ if ( $bSuppressErrors == false ) {
377394 return htmlspecialchars( wfMsg( 'intersection_toomanycats' ) ); // "!!too many categories!!";
378395 } else {
379396 return '';
——@@ -391,7 +408,7 @@
392409 $bCountSet = true;
393410 }
394411
395 - //disallow showing date if the query doesn't have an inclusion category parameter
 412+ // disallow showing date if the query doesn't have an inclusion category parameter
396413 if ( $iCatCount < 1 ) {
397414 $bAddFirstCategoryDate = false;
398415 // don't sort by fields relating to categories if there are no categories.
——@@ -400,83 +417,86 @@
401418 }
402419 }
403420
404 -
405 - //build the SQL query
 421+ // build the SQL query
406422 $dbr = wfGetDB( DB_SLAVE );
407 - $aTables = Array( 'page' );
408 - $aFields = Array( 'page_namespace', 'page_title' );
409 - $aWhere = Array();
410 - $aJoin = Array();
411 - $aOptions = Array();
 423+ $tables = array( 'page' );
 424+ $fields = array( 'page_namespace', 'page_title' );
 425+ $where = array();
 426+ $join = array();
 427+ $options = array();
412428
413429 if ( $bGoogleHack ) {
414 - $aFields[] = 'page_id';
 430+ $fields[] = 'page_id';
415431 }
416432
417433 if ( $bAddFirstCategoryDate ) {
418 - $aFields[] = 'c1.cl_timestamp';
 434+ $fields[] = 'c1.cl_timestamp';
419435 }
420436
421 - if ( true == $bNamespace ) {
422 - $aWhere['page_namespace'] = $iNamespace;
 437+ if ( $bNamespace == true ) {
 438+ $where['page_namespace'] = $iNamespace;
423439 }
424440
425441 // Bug 14943 - Allow filtering based on FlaggedRevs stability.
426442 // Check if the extension actually exists before changing the query...
427443 if ( function_exists( 'efLoadFlaggedRevs' ) && $bFlaggedRevs ) {
428 - $aTables[] = 'flaggedpages';
429 - $aJoin['flaggedpages'] = Array( 'LEFT JOIN', 'page_id = fp_page_id' );
 444+ $tables[] = 'flaggedpages';
 445+ $join['flaggedpages'] = array( 'LEFT JOIN', 'page_id = fp_page_id' );
430446
431447 switch( $sStable ) {
432448 case 'only':
433 - $aWhere[] = 'fp_stable IS NOT NULL';
 449+ $where[] = 'fp_stable IS NOT NULL';
434450 break;
435451 case 'exclude':
436 - $aWhere['fp_stable'] = null;
 452+ $where['fp_stable'] = null;
437453 break;
438454 }
439455
440456 switch( $sQuality ) {
441457 case 'only':
442 - $aWhere[] = 'fp_quality >= 1';
 458+ $where[] = 'fp_quality >= 1';
443459 break;
444460 case 'exclude':
445 - $aWhere[] = 'fp_quality = 0 OR fp_quality IS NULL';
 461+ $where[] = 'fp_quality = 0 OR fp_quality IS NULL';
446462 break;
447463 }
448464 }
449465
450466 switch ( $sRedirects ) {
451467 case 'only':
452 - $aWhere['page_is_redirect'] = 1;
 468+ $where['page_is_redirect'] = 1;
453469 break;
454470 case 'exclude':
455 - $aWhere['page_is_redirect'] = 0;
 471+ $where['page_is_redirect'] = 0;
456472 break;
457473 }
458474
459475 $iCurrentTableNumber = 1;
460476 $categorylinks = $dbr->tableName( 'categorylinks' );
461477
462 - for ($i = 0; $i < $iCatCount; $i++) {
463 - $aJoin["$categorylinks AS c$iCurrentTableNumber"] = Array( 'INNER JOIN',
464 - Array( "page_id = c{$iCurrentTableNumber}.cl_from",
 478+ for ( $i = 0; $i < $iCatCount; $i++ ) {
 479+ $join["$categorylinks AS c$iCurrentTableNumber"] = array(
 480+ 'INNER JOIN',
 481+ array(
 482+ "page_id = c{$iCurrentTableNumber}.cl_from",
465483 "c{$iCurrentTableNumber}.cl_to={$dbr->addQuotes($aCategories[$i]->getDBKey())}"
466484 )
467485 );
468 - $aTables[] = "$categorylinks AS c$iCurrentTableNumber";
 486+ $tables[] = "$categorylinks AS c$iCurrentTableNumber";
469487
470488 $iCurrentTableNumber++;
471489 }
472490
473 - for ($i = 0; $i < $iExcludeCatCount; $i++) {
474 - $aJoin["$categorylinks AS c$iCurrentTableNumber"] = Array( 'LEFT OUTER JOIN',
475 - Array( "page_id = c{$iCurrentTableNumber}.cl_from",
476 - "c{$iCurrentTableNumber}.cl_to={$dbr->addQuotes($aExcludeCategories[$i]->getDBKey())}"
 491+ for ( $i = 0; $i < $iExcludeCatCount; $i++ ) {
 492+ $join["$categorylinks AS c$iCurrentTableNumber"] = array(
 493+ 'LEFT OUTER JOIN',
 494+ array(
 495+ "page_id = c{$iCurrentTableNumber}.cl_from",
 496+ "c{$iCurrentTableNumber}.cl_to={$dbr->addQuotes($aExcludeCategories[$i]->getDBKey())}"
477497 )
478498 );
479 - $aTables[] = "$categorylinks AS c$iCurrentTableNumber";
480 - $aWhere["c{$iCurrentTableNumber}.cl_to"] = null;
 499+ $tables[] = "$categorylinks AS c$iCurrentTableNumber";
 500+ $where["c{$iCurrentTableNumber}.cl_to"] = null;
481501 $iCurrentTableNumber++;
482502 }
483503
——@@ -508,28 +528,28 @@
509529 break;
510530 }
511531
512 - $aOptions['ORDER BY'] = "$sSqlSort $sSqlOrder";
 532+ $options['ORDER BY'] = "$sSqlSort $sSqlOrder";
513533
514534 if ( $bCountSet ) {
515 - $aOptions['LIMIT'] = $iCount;
 535+ $options['LIMIT'] = $iCount;
516536 }
517537 if ( $iOffset > 0 ) {
518 - $aOptions['OFFSET'] = $iOffset;
 538+ $options['OFFSET'] = $iOffset;
519539 }
520540
521541 // process the query
522 - $res = $dbr->select( $aTables, $aFields, $aWhere, __METHOD__, $aOptions, $aJoin );
 542+ $res = $dbr->select( $tables, $fields, $where, __METHOD__, $options, $join );
523543 $sk = $wgUser->getSkin();
524544
525545 if ( $dbr->numRows( $res ) == 0 ) {
526 - if ( false == $bSuppressErrors ) {
 546+ if ( $bSuppressErrors == false ) {
527547 return htmlspecialchars( wfMsg( 'intersection_noresults' ) );
528548 } else {
529549 return '';
 550+ }
530551 }
531 - }
532552
533 - //start unordered list
 553+ // start unordered list
534554 $output = $sStartList . "\n";
535555
536556 $categoryDate = '';
——@@ -538,11 +558,12 @@
539559 $df = DateFormatter::getInstance();
540560 }
541561
542 - //process results of query, outputing equivalent of <li>[[Article]]</li> for each result,
543 - //or something similar if the list uses other startlist/endlist
544 - $articleList = Array();
 562+ // process results of query, outputing equivalent of <li>[[Article]]</li>
 563+ // for each result, or something similar if the list uses other
 564+ // startlist/endlist
 565+ $articleList = array();
545566 foreach ( $res as $row ) {
546 - $title = Title::makeTitle( $row->page_namespace, $row->page_title);
 567+ $title = Title::makeTitle( $row->page_namespace, $row->page_title );
547568 if ( true == $bAddFirstCategoryDate ) {
548569 if ( $sDateFormat != '' ) {
549570 # this is a tad ugly
——@@ -567,47 +588,57 @@
568589
569590 $query = array();
570591
571 - if ( true == $bGoogleHack ) {
572 - $query['dpl_id'] = intval($row->page_id);
 592+ if ( $bGoogleHack == true ) {
 593+ $query['dpl_id'] = intval( $row->page_id );
573594 }
574595
575 - if ( true == $bShowNamespace ) {
 596+ if ( $bShowNamespace == true ) {
576597 $titleText = $title->getPrefixedText();
577598 } else {
578599 $titleText = $title->getText();
579600 }
580601
581602 if ( $bUseGallery ) {
582 - # Note, $categoryDate is treated as raw html
583 - # this is safe since the only html present
584 - # would come from the dateformatter <span>.
585 - $gallery->add( $title, $categoryDate );
 603+ # Note, $categoryDate is treated as raw html
 604+ # this is safe since the only html present
 605+ # would come from the dateformatter <span>.
 606+ $gallery->add( $title, $categoryDate );
586607 } else {
587 - $articleList[] = $categoryDate
588 - . $sk->link( $title, htmlspecialchars( $titleText ), $aLinkOptions, $query, array( 'forcearticlepath', 'known' ) );
 608+ $articleList[] = $categoryDate .
 609+ $sk->link(
 610+ $title,
 611+ htmlspecialchars( $titleText ),
 612+ $aLinkOptions,
 613+ $query,
 614+ array( 'forcearticlepath', 'known' )
 615+ );
589616 }
590617 }
591618
592 - //end unordered list
 619+ // end unordered list
593620 if ( $bUseGallery ) {
594621 $gallery->setHideBadImages();
595622 $gallery->setShowFilename( $bGalleryFileName );
596623 $gallery->setShowBytes( $bGalleryFileSize );
597 - if ( $iGalleryImageHeight > 0 )
 624+ if ( $iGalleryImageHeight > 0 ) {
598625 $gallery->setHeights( $iGalleryImageHeight );
599 - if ( $iGalleryImageWidth > 0 )
 626+ }
 627+ if ( $iGalleryImageWidth > 0 ) {
600628 $gallery->setWidths( $iGalleryImageWidth );
601 - if ( $iGalleryNumbRows > 0 )
 629+ }
 630+ if ( $iGalleryNumbRows > 0 ) {
602631 $gallery->setPerRow( $iGalleryNumbRows );
603 - if ( $sGalleryCaption != '' )
 632+ }
 633+ if ( $sGalleryCaption != '' ) {
604634 $gallery->setCaption( $sGalleryCaption ); # gallery class escapes string
 635+ }
605636 $output = $gallery->toHtml();
606637 } else {
607638 $output .= $sStartItem;
608639 if ( $bInlineMode ) {
609640 $output .= $wgContLang->commaList( $articleList );
610 - } else {
611 - $output .= implode( "$sEndItem \n $sStartItem", $articleList );
 641+ } else {
 642+ $output .= implode( "$sEndItem \n $sStartItem", $articleList );
612643 }
613644 $output .= $sEndItem;
614645 $output .= $sEndList . "\n";

Comments

#Comment by Nikerabbit (talk | contribs)   14:08, 28 February 2011

What comes to != and ==, they are not transitive and changing the order may change behavior on edge cases.

#Comment by Bawolff (talk | contribs)   04:40, 7 March 2011

Thanks for giving this extension some love :)

For the change order == thing. The only two variables that can be set by the user, that this would affect is the supresserrors and addfirstcategorydate. In both cases almost without exception people always just say true if they are using either. (the addfirstcategorydate can also have the ymd options, that isn't heavily used, and in any case wouldn't be an edge case to worry about).

marking ok

Status & tagging log