r113986 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r113985‎ | r113986 | r113987 >
Date:01:26, 16 March 2012
Author:reedy
Status:reverted (Comments)
Tags:gerritmigration 
Comment:
Documentation stuffs
Modified paths:
  • /trunk/extensions/Collection/Collection.body.php (modified) (history)
  • /trunk/extensions/Collection/Collection.hooks.php (modified) (history)
  • /trunk/extensions/Collection/Collection.session.php (modified) (history)
  • /trunk/extensions/Collection/Collection.suggest.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Collection/Collection.body.php
@@ -23,6 +23,9 @@
2424 class SpecialCollection extends SpecialPage {
2525 var $tempfile;
2626
 27+ /**
 28+ * @param $PODPartners bool|array
 29+ */
2730 public function __construct( $PODPartners = false ) {
2831 parent::__construct( "Book" );
2932 global $wgCollectionPODPartners;
@@ -190,7 +193,8 @@
191194 }
192195 $collection = $this->loadCollection( $title );
193196 $partner = $wgRequest->getVal( 'partner', key( $this->mPODPartners ) );
194 - return $this->postZIP( $collection, $partner );
 197+ $this->postZIP( $collection, $partner );
 198+ return;
195199 case 'save_collection':
196200 if ( $wgRequest->getVal( 'abort' ) ) {
197201 $wgOut->redirect( SkinTemplate::makeSpecialUrl( 'Book' ) );
@@ -229,11 +233,12 @@
230234 }
231235 return;
232236 case 'render':
233 - return $this->renderCollection(
 237+ $this->renderCollection(
234238 CollectionSession::getCollection(),
235239 SpecialPage::getTitleFor( 'Book' ),
236240 $wgRequest->getVal( 'writer', '' )
237241 );
 242+ return;
238243 case 'forcerender':
239244 $this->forceRenderCollection();
240245 return;
@@ -298,6 +303,11 @@
299304 }
300305 }
301306
 307+ /**
 308+ * @param $referer
 309+ * @param $par
 310+ * @return mixed
 311+ */
302312 function renderBookCreatorPage( $referer, $par ) {
303313 global $wgOut, $wgJsMimeType;
304314
@@ -379,6 +389,9 @@
380390 $wgOut->addWikiMsg( 'coll-book_creator_help' );
381391 }
382392
 393+ /**
 394+ * @param $referer
 395+ */
383396 function renderStopBookCreatorPage( $referer ) {
384397 global $wgOut;
385398
@@ -420,6 +433,9 @@
421434 );
422435 }
423436
 437+ /**
 438+ * @return array
 439+ */
424440 static function getBookPagePrefixes() {
425441 global $wgUser, $wgCommunityCollectionNamespace;
426442
@@ -475,6 +491,11 @@
476492 CollectionSession::setCollection( $collection );
477493 }
478494
 495+ /**
 496+ * @param $a array
 497+ * @param $b array
 498+ * @return int
 499+ */
479500 static function title_cmp( $a, $b ) {
480501 return strcasecmp( $a['title'], $b['title'] );
481502 }
@@ -497,6 +518,9 @@
498519 CollectionSession::setCollection( $collection );
499520 }
500521
 522+ /**
 523+ * @param $name string
 524+ */
501525 static function addChapter( $name ) {
502526 $collection = CollectionSession::getCollection();
503527 array_push( $collection['items'], array(
@@ -506,6 +530,10 @@
507531 CollectionSession::setCollection( $collection );
508532 }
509533
 534+ /**
 535+ * @param $index int
 536+ * @param $name string
 537+ */
510538 static function renameChapter( $index, $name ) {
511539 if ( !is_int( $index ) ) {
512540 return;
@@ -518,6 +546,12 @@
519547 CollectionSession::setCollection( $collection );
520548 }
521549
 550+ /**
 551+ * @param $namespace int
 552+ * @param $name string
 553+ * @param int $oldid
 554+ * @return bool
 555+ */
522556 static function addArticleFromName( $namespace, $name, $oldid = 0 ) {
523557 $title = Title::makeTitleSafe( $namespace, $name );
524558 if ( !$title ) {
@@ -581,6 +615,12 @@
582616 return true;
583617 }
584618
 619+ /**
 620+ * @param $namespace string
 621+ * @param $name string
 622+ * @param $oldid int
 623+ * @return bool
 624+ */
585625 static function removeArticleFromName( $namespace, $name, $oldid = 0 ) {
586626 $title = Title::makeTitleSafe( $namespace, $name );
587627 return self::removeArticle( $title, $oldid );
@@ -604,18 +644,26 @@
605645 return true;
606646 }
607647
 648+ /**
 649+ * @param $name string
 650+ * @return bool
 651+ */
608652 static function addCategoryFromName( $name ) {
609653 $title = Title::makeTitleSafe( NS_CATEGORY, $name );
610654 return self::addCategory( $title );
611655 }
612656
 657+ /**
 658+ * @param $title Title
 659+ * @return bool
 660+ */
613661 static function addCategory( $title ) {
614662 global $wgCollectionMaxArticles, $wgCollectionArticleNamespaces;
615663
616664 $limit = $wgCollectionMaxArticles - CollectionSession::countArticles();
617665 if ( $limit <= 0 ) {
618666 self::limitExceeded();
619 - return;
 667+ return false;
620668 }
621669 $db = wfGetDB( DB_SLAVE );
622670 $tables = array( 'page', 'categorylinks' );
@@ -668,6 +716,11 @@
669717 return true;
670718 }
671719
 720+ /**
 721+ * @param $index
 722+ * @param $delta
 723+ * @return bool
 724+ */
672725 static function moveItem( $index, $delta ) {
673726 if ( !CollectionSession::hasSession() ) {
674727 return false;
@@ -680,6 +733,9 @@
681734 return true;
682735 }
683736
 737+ /**
 738+ * @param $items array
 739+ */
684740 static function setSorting( $items ) {
685741 if ( !CollectionSession::hasSession() ) {
686742 return;
@@ -694,6 +750,12 @@
695751 CollectionSession::setCollection( $collection );
696752 }
697753
 754+ /**
 755+ * @param $collection
 756+ * @param $line
 757+ * @param $append
 758+ * @return array|null
 759+ */
698760 function parseCollectionLine( &$collection, $line, $append ) {
699761 $line = trim( $line );
700762 if ( !$append && preg_match( '/^===\s*(.*?)\s*===$/', $line, $match ) ) {
@@ -775,7 +837,7 @@
776838
777839 if ( is_null( $title ) ) {
778840 $wgOut->showErrorPage( 'coll-notitle_title', 'coll-notitle_msg' );
779 - return;
 841+ return null;
780842 }
781843
782844 if ( !$title->exists() ) {
@@ -872,6 +934,9 @@
873935 return true;
874936 }
875937
 938+ /**
 939+ * @return array
 940+ */
876941 function getLicenseInfos() {
877942 global $wgCollectionLicenseName, $wgCollectionLicenseURL, $wgRightsIcon;
878943 global $wgRightsPage, $wgRightsText, $wgRightsUrl;
@@ -904,6 +969,10 @@
905970 return array( $licenseInfo );
906971 }
907972
 973+ /**
 974+ * @param $collection array
 975+ * @return string
 976+ */
908977 function buildJSONCollection( $collection ) {
909978 $result = array(
910979 'type' => 'collection',
@@ -943,6 +1012,12 @@
9441013 return $json->encode( $result );
9451014 }
9461015
 1016+ /**
 1017+ * @param $collection
 1018+ * @param $referrer Title
 1019+ * @param $writer
 1020+ * @return mixed
 1021+ */
9471022 function renderCollection( $collection, $referrer, $writer ) {
9481023 global $wgOut, $wgContLang, $wgScriptPath, $wgScriptExtension;
9491024
@@ -1031,7 +1106,6 @@
10321107
10331108 switch ( $response['state'] ) {
10341109 case 'progress':
1035 - $url = htmlspecialchars( SkinTemplate::makeSpecialUrl( 'Book', 'bookcmd=rendering&' . $query ) );
10361110 $wgOut->addHeadItem( 'refresh-nojs', '<noscript><meta http-equiv="refresh" content="2" /></noscript>' );
10371111 $wgOut->addInlineScript( 'var collection_id = "' . urlencode( $response['collection_id'] ) . '";' );
10381112 $wgOut->addInlineScript( 'var writer = "' . urlencode( $response['writer'] ) . '";' );
@@ -1123,6 +1197,11 @@
11241198 $wgOut->disable();
11251199 }
11261200
 1201+ /**
 1202+ * @param $res
 1203+ * @param $content
 1204+ * @return int
 1205+ */
11271206 function writeToTempFile( $res, $content ) {
11281207 return fwrite( $this->tempfile, $content );
11291208 }
@@ -1157,6 +1236,11 @@
11581237 $this->renderCollection( array( 'items' => array( $article ) ), $title, $writer );
11591238 }
11601239
 1240+ /**
 1241+ * @param $collection
 1242+ * @param $partner
 1243+ * @return mixed
 1244+ */
11611245 function postZIP( $collection, $partner ) {
11621246 global $wgScriptPath, $wgScriptExtension, $wgOut;
11631247
@@ -1181,6 +1265,12 @@
11821266 $wgOut->redirect( $response['redirect_url'] );
11831267 }
11841268
 1269+ /**
 1270+ * @param $colltype
 1271+ * @param $title
 1272+ * @param $pcollname
 1273+ * @param $ccollname
 1274+ */
11851275 private function renderSaveOverwritePage( $colltype, $title, $pcollname, $ccollname ) {
11861276 global $wgOut;
11871277
@@ -1195,6 +1285,9 @@
11961286 $wgOut->addTemplate( $template );
11971287 }
11981288
 1289+ /**
 1290+ * @param $title
 1291+ */
11991292 private function renderLoadOverwritePage( $title ) {
12001293 global $wgOut;
12011294
@@ -1206,13 +1299,19 @@
12071300 $wgOut->addTemplate( $template );
12081301 }
12091302
 1303+ /**
 1304+ * @param $command
 1305+ * @param $args
 1306+ * @return bool|array
 1307+ */
12101308 static function mwServeCommand( $command, $args ) {
12111309 global $wgOut, $wgCollectionMWServeURL, $wgCollectionMWServeCredentials, $wgCollectionFormatToServeURL;
12121310
12131311 $serveURL = $wgCollectionMWServeURL;
12141312 if ( isset ( $args['writer'] ) ) {
1215 - if ( array_key_exists( $args['writer'], $wgCollectionFormatToServeURL ) )
 1313+ if ( array_key_exists( $args['writer'], $wgCollectionFormatToServeURL ) ) {
12161314 $serveURL = $wgCollectionFormatToServeURL[ $args['writer'] ];
 1315+ }
12171316 }
12181317
12191318 $args['command'] = $command;
Index: trunk/extensions/Collection/Collection.suggest.php
@@ -83,7 +83,7 @@
8484 * @param $param (type string) name of the article to be added, banned or removed
8585 * or a number of articles to add or a value (1 - 1.5) all articles with a
8686 * higher value will be added to the collection
87 - * @return (type string) html-code for the proposallist and the memberlist
 87+ * @return string html-code for the proposallist and the memberlist
8888 */
8989 public static function refresh( $mode, $param ) {
9090 global $wgLang;
@@ -96,6 +96,11 @@
9797 );
9898 }
9999
 100+ /**
 101+ * @param $lastAction
 102+ * @param $article
 103+ * @return array
 104+ */
100105 public static function undo( $lastAction, $article ) {
101106 switch ( $lastAction ) {
102107 case 'add':
@@ -155,7 +160,7 @@
156161 * @param $param (type string) name of the article to be added, banned or removed
157162 * or a number of articles to add or a value (1 - 1.5) all articles with a
158163 * higher value will be added to the collection
159 - * @return the template for the wikipage
 164+ * @return CollectionSuggestTemplate the template for the wikipage
160165 */
161166 private static function getCollectionSuggestTemplate( $mode, $param ) {
162167 global $wgCollectionMaxSuggestions;
@@ -204,8 +209,8 @@
205210 /**
206211 * Add some articles and update the book of the Proposal-Object
207212 *
208 - * @param $articleList an array with the names of the articles to be added
209 - * @param $prop the proposal-Object
 213+ * @param $articleList array with the names of the articles to be added
 214+ * @param $prop Proposals the proposal-Object
210215 */
211216 private static function addArticlesFromName( $articleList, $prop ) {
212217 foreach ( $articleList as $article ) {
@@ -312,6 +317,9 @@
313318 }
314319 }
315320
 321+ /**
 322+ * @return bool
 323+ */
316324 public function hasBans() {
317325 return count( $this->mBanList ) > 0;
318326 }
@@ -368,6 +376,10 @@
369377 $this->mLinkList = $newList;
370378 }
371379
 380+ /**
 381+ * @param $title Title
 382+ * @return Title
 383+ */
372384 private function resolveRedirects( $title ) {
373385 if ( !$title->isRedirect() ) {
374386 return $title;
@@ -384,8 +396,8 @@
385397 /**
386398 * Extract & count links from wikitext
387399 *
388 - * @param wikitext: article text
389 - * @return an array with links and their weights
 400+ * @param wikitext string article text
 401+ * @return array with links and their weights
390402 */
391403 private function getWeightedLinks( $num_articles, $wikitext ) {
392404 global $wgCollectionSuggestCheapWeightThreshhold;
@@ -493,7 +505,9 @@
494506 }
495507 }
496508
497 - // Calculate the $mPropList from $mLinkList and $mBanList
 509+ /**
 510+ * Calculate the $mPropList from $mLinkList and $mBanList
 511+ */
498512 private function getPropList() {
499513 $prop = array();
500514 foreach ( $this->mLinkList as $article ) {
@@ -531,9 +545,9 @@
532546 * if the array doesn't contain the article
533547 *
534548 * @param $entry (type string) an articlename
535 - * @param $array the array to be searched, it has to 2-dimensional
 549+ * @param $array array to be searched, it has to 2-dimensional
536550 * the 2nd dimension needs the key 'name'
537 - * @return the key as integer or false
 551+ * @return bool|int the key as integer or false
538552 */
539553 private function searchEntry( $entry, $array ) {
540554 for ( $i = 0; $i < count( $array ); $i++ ) {
@@ -547,8 +561,8 @@
548562 /**
549563 * Check if an article is banned or belongs to the book/collection
550564 *
551 - * @param $link (type string) an articlename
552 - * @return (type boolean) true: if the article can be added to the proposals
 565+ * @param $link string an articlename
 566+ * @return boolean true: if the article can be added to the proposals
553567 * false: if the article can't be added to the proposals
554568 */
555569 private function checkLink( $link ) {
@@ -565,28 +579,10 @@
566580 return true;
567581 }
568582
 583+ /**
 584+ * @return int
 585+ */
569586 private function getPropCount() {
570587 return count( $this->mPropList );
571588 }
572589 }
573 -
574 -/**
575 - * sort $mPropList by the entries values
576 - * sort alphabetically by equal values
577 - *
578 - * @param $a, $b: arrays that contain two entries
579 - * the keys: 'name' & 'val'
580 - * 'name': an articlename
581 - * 'val' : a value from 1 to 1.5
582 - * @return 1, -1 or 0
583 - */
584 -function wgCollectionCompareProps( $a, $b ) {
585 - if ( $a['val'] == $b['val'] ) {
586 - return strcmp( $a['name'], $b['name'] );
587 - }
588 - if ( $a['val'] < $b['val'] ) {
589 - return 1;
590 - } else {
591 - return - 1;
592 - }
593 -}
Index: trunk/extensions/Collection/Collection.hooks.php
@@ -74,7 +74,7 @@
7575 $title = $sk->getTitle();
7676
7777 if ( is_null( $title ) || !$title->exists() ) {
78 - return;
 78+ return false;
7979 }
8080
8181 $namespace = $title->getNamespace();
@@ -155,9 +155,9 @@
156156 global $wgOut;
157157 if ( !$wgOut->isPrintable() ) {
158158 $attribs = array(
159 - 'href' => $sk->getTitle()->getLocalUrl( $wgRequest->appendQueryValue( 'printable', 'yes', true ) ),
160 - 'title' => $sk->titleAttrib( 't-print', 'withaccess' ),
161 - 'accesskey' => $sk->accesskey( 't-print' ),
 159+ 'href' => $title->getLocalUrl( $wgRequest->appendQueryValue( 'printable', 'yes', true ) ),
 160+ 'title' => Linker::titleAttrib( 't-print', 'withaccess' ),
 161+ 'accesskey' => Linker::accesskey( 't-print' ),
162162 );
163163 if ( $attribs['title'] === false ) {
164164 unset( $attribs['title'] );
@@ -178,6 +178,9 @@
179179
180180 /**
181181 * Callback for hook SiteNoticeAfter
 182+ * @param $siteNotice
 183+ * @param $skin Skin
 184+ * @return bool
182185 */
183186 static function siteNoticeAfter( &$siteNotice, $skin = null ) {
184187 global $wgCollectionArticleNamespaces;
@@ -227,7 +230,6 @@
228231
229232 /**
230233 * @param $title Title
231 - * @param $skin Skin
232234 * @param $mode string
233235 * @return string
234236 */
@@ -316,6 +318,12 @@
317319 return $html;
318320 }
319321
 322+ /**
 323+ * @param $title
 324+ * @param $ajaxHint null
 325+ * @param $oldid null|int
 326+ * @return string
 327+ */
320328 static function getBookCreatorBoxContent( $title, $ajaxHint = null, $oldid = null ) {
321329 $imagePath = SpecialCollection::getMediaPath();
322330
@@ -325,7 +333,6 @@
326334 }
327335
328336 /**
329 - * @param $sk Skin
330337 * @param $imagePath
331338 * @param $ajaxHint
332339 * @param $title Title
@@ -403,6 +410,11 @@
404411
405412 }
406413
 414+ /**
 415+ * @param $imagePath
 416+ * @param $ajaxHint
 417+ * @return string
 418+ */
407419 static function getBookCreatorBoxShowBookLink( $imagePath, $ajaxHint ) {
408420 $numArticles = CollectionSession::countArticles();
409421
@@ -446,6 +458,11 @@
447459 }
448460 }
449461
 462+ /**
 463+ * @param $imagePath
 464+ * @param $ajaxHint
 465+ * @return string
 466+ */
450467 static function getBookCreatorBoxSuggestLink( $imagePath, $ajaxHint ) {
451468 if ( wfMsg( 'coll-suggest_enabled' ) != '1' ) {
452469 return '';
@@ -492,8 +509,10 @@
493510 }
494511
495512 /**
496 - * OutputPageCheckLastModified hook
497 - */
 513+ * OutputPageCheckLastModified hook
 514+ * @param $modifiedTimes array
 515+ * @return bool
 516+ */
498517 static function checkLastModified( $modifiedTimes ) {
499518 if ( CollectionSession::hasSession() ) {
500519 $modifiedTimes['collection'] = $_SESSION['wsCollection']['timestamp'];
@@ -503,6 +522,8 @@
504523
505524 /**
506525 * ResourceLoaderGetConfigVars hook
 526+ * @param $vars array
 527+ * @return bool
507528 */
508529 static function resourceLoaderGetConfigVars( &$vars ) {
509530 $vars['wgCollectionVersion'] = $GLOBALS['wgCollectionVersion'];
Index: trunk/extensions/Collection/Collection.session.php
@@ -22,8 +22,14 @@
2323 */
2424
2525 class CollectionSession {
 26+
 27+ /**
 28+ * @return bool
 29+ */
2630 static function hasSession() {
27 - if ( !session_id() ) return false;
 31+ if ( !session_id() ) {
 32+ return false;
 33+ }
2834 return isset( $_SESSION['wsCollection'] );
2935 }
3036
@@ -69,10 +75,16 @@
7076 self::touchSession();
7177 }
7278
 79+ /**
 80+ * @return bool
 81+ */
7382 static function isEnabled() {
7483 return ( self::hasSession() && $_SESSION['wsCollection']['enabled'] );
7584 }
7685
 86+ /**
 87+ * @return int
 88+ */
7789 static function countArticles() {
7890 if ( !self::hasSession() ) {
7991 return 0;
@@ -86,6 +98,11 @@
8799 return $count;
88100 }
89101
 102+ /**
 103+ * @param $title
 104+ * @param $oldid int
 105+ * @return int
 106+ */
90107 static function findArticle( $title, $oldid = 0 ) {
91108 if ( !self::hasSession() ) {
92109 return - 1;
@@ -107,6 +124,9 @@
108125 return - 1;
109126 }
110127
 128+ /**
 129+ * @return bool
 130+ */
111131 static function purge() {
112132 if ( !self::hasSession() ) {
113133 return false;
@@ -139,10 +159,16 @@
140160 return true;
141161 }
142162
 163+ /**
 164+ * @return array
 165+ */
143166 static function getCollection() {
144167 return self::purge() ? $_SESSION['wsCollection'] : array();
145168 }
146169
 170+ /**
 171+ * @param $collection
 172+ */
147173 static function setCollection( $collection ) {
148174 $_SESSION['wsCollection'] = $collection;
149175 self::touchSession();

Follow-up revisions

RevisionCommit summaryAuthorDate
r114012Partial revert r113986...reedy14:39, 16 March 2012
r114399Revert r113740 and its followups r113785, r113985, r113986, r113988, r113991,...catrope19:56, 21 March 2012

Comments

#Comment by Reedy (talk | contribs)   01:30, 16 March 2012

Oh, normalised some return statements, and remove some unused code and added a few braces

#Comment by Brion VIBBER (talk | contribs)   05:21, 16 March 2012

That code wasn't unused! bug 35253

Status & tagging log