r99760 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r99759‎ | r99760 | r99761 >
Date:08:06, 14 October 2011
Author:reedy
Status:ok
Tags:
Comment:
Documentation

Trim trailing whitespace

Make returns return values where appropriate (ie other paths in the same method do)
Modified paths:
  • /trunk/phase3/includes/resourceloader/ResourceLoader.php (modified) (history)
  • /trunk/phase3/includes/resourceloader/ResourceLoaderContext.php (modified) (history)
  • /trunk/phase3/includes/resourceloader/ResourceLoaderFileModule.php (modified) (history)
  • /trunk/phase3/includes/resourceloader/ResourceLoaderFilePageModule.php (modified) (history)
  • /trunk/phase3/includes/resourceloader/ResourceLoaderModule.php (modified) (history)
  • /trunk/phase3/includes/resourceloader/ResourceLoaderStartUpModule.php (modified) (history)
  • /trunk/phase3/includes/resourceloader/ResourceLoaderUserModule.php (modified) (history)
  • /trunk/phase3/includes/resourceloader/ResourceLoaderUserOptionsModule.php (modified) (history)
  • /trunk/phase3/includes/resourceloader/ResourceLoaderUserTokensModule.php (modified) (history)
  • /trunk/phase3/includes/resourceloader/ResourceLoaderWikiModule.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/resourceloader/ResourceLoader.php
——@@ -263,10 +263,10 @@
264264
265265 /**
266266 * Add a foreign source of modules.
267 - *
 267+ *
268268 * Source properties:
269269 * 'loadScript': URL (either fully-qualified or protocol-relative) of load.php for this source
270 - *
 270+ *
271271 * @param $id Mixed: source ID (string), or array( id1 => props1, id2 => props2, ... )
272272 * @param $properties Array: source properties
273273 */
——@@ -340,7 +340,7 @@
341341
342342 /**
343343 * Get the list of sources
344 - *
 344+ *
345345 * @return Array: array( id => array of properties, .. )
346346 */
347347 public function getSources() {
——@@ -401,6 +401,9 @@
402402 // the last modified time
403403 $mtime = wfTimestamp( TS_UNIX, $wgCacheEpoch );
404404 foreach ( $modules as $module ) {
 405+ /**
 406+ * @var $module ResourceLoaderModule
 407+ */
405408 try {
406409 // Bypass Squid and other shared caches if the request includes any private modules
407410 if ( $module->getGroup() === 'private' ) {
——@@ -571,7 +574,7 @@
572575 $this->sendResponseHeaders( $context, $ts, false );
573576 // If there's an If-Modified-Since header, respond with a 304 appropriately
574577 if ( $this->tryRespondLastModified( $context, $ts ) ) {
575 - return; // output handled (buffers cleared)
 578+ return false; // output handled (buffers cleared)
576579 }
577580 $response = $fileCache->fetchText();
578581 // Remove the output buffer and output the response
——@@ -617,6 +620,10 @@
618621
619622 // Generate output
620623 foreach ( $modules as $name => $module ) {
 624+ /**
 625+ * @var $module ResourceLoaderModule
 626+ */
 627+
621628 wfProfileIn( __METHOD__ . '-' . $name );
622629 try {
623630 $scripts = '';
——@@ -876,13 +883,13 @@
877884 /**
878885 * Returns JS code which calls mw.loader.addSource() with the given
879886 * parameters. Has two calling conventions:
880 - *
 887+ *
881888 * - ResourceLoader::makeLoaderSourcesScript( $id, $properties ):
882889 * Register a single source
883 - *
 890+ *
884891 * - ResourceLoader::makeLoaderSourcesScript( array( $id1 => $props1, $id2 => $props2, ... ) );
885892 * Register sources with the given IDs and properties.
886 - *
 893+ *
887894 * @param $id String: source ID
888895 * @param $properties Array: source properties (see addSource())
889896 *
——@@ -982,7 +989,7 @@
983990 $query = self::makeLoaderQuery( $modules, $lang, $skin, $user, $version, $debug,
984991 $only, $printable, $handheld, $extraQuery
985992 );
986 -
 993+
987994 // Prevent the IE6 extension check from being triggered (bug 28840)
988995 // by appending a character that's invalid in Windows extensions ('*')
989996 return wfExpandUrl( wfAppendQuery( $wgLoadScript, $query ) . '&*', PROTO_RELATIVE );
——@@ -1017,7 +1024,7 @@
10181025 $query['handheld'] = 1;
10191026 }
10201027 $query += $extraQuery;
1021 -
 1028+
10221029 // Make queries uniform in order
10231030 ksort( $query );
10241031 return $query;
Index: trunk/phase3/includes/resourceloader/ResourceLoaderUserOptionsModule.php
——@@ -55,7 +55,7 @@
5656 /**
5757 * Fetch the context's user options, or if it doesn't match current user,
5858 * the default options.
59 - *
 59+ *
6060 * @param $context ResourceLoaderContext: Context object
6161 * @return Array: List of user options keyed by option name
6262 */
——@@ -75,7 +75,7 @@
7676 * @return string
7777 */
7878 public function getScript( ResourceLoaderContext $context ) {
79 - return Xml::encodeJsCall( 'mw.user.options.set',
 79+ return Xml::encodeJsCall( 'mw.user.options.set',
8080 array( $this->contextUserOptions( $context ) ) );
8181 }
8282
——@@ -96,7 +96,7 @@
9797
9898 // Underline: 2 = browser default, 1 = always, 0 = never
9999 if ( $options['underline'] < 2 ) {
100 - $rules[] = "a { text-decoration: " .
 100+ $rules[] = "a { text-decoration: " .
101101 ( $options['underline'] ? 'underline !important' : 'none' ) . "; }";
102102 }
103103 if ( $options['highlightbroken'] ) {
——@@ -133,7 +133,10 @@
134134 public function getGroup() {
135135 return 'private';
136136 }
137 -
 137+
 138+ /**
 139+ * @return array
 140+ */
138141 public function getDependencies() {
139142 return array( 'mediawiki.user' );
140143 }
Index: trunk/phase3/includes/resourceloader/ResourceLoaderFilePageModule.php
——@@ -1,8 +1,13 @@
22 <?php
3 -/*
 3+/**
44 * ResourceLoader definition for MediaWiki:Filepage.css
55 */
66 class ResourceLoaderFilePageModule extends ResourceLoaderWikiModule {
 7+
 8+ /**
 9+ * @param $context ResourceLoaderContext
 10+ * @return array
 11+ */
712 protected function getPages( ResourceLoaderContext $context ) {
813 return array(
914 'MediaWiki:Filepage.css' => array( 'type' => 'style' ),
Index: trunk/phase3/includes/resourceloader/ResourceLoaderContext.php
——@@ -21,7 +21,7 @@
2222 */
2323
2424 /**
25 - * Object passed around to modules which contains information about the state
 25+ * Object passed around to modules which contains information about the state
2626 * of a specific loader request
2727 */
2828 class ResourceLoaderContext {
——@@ -42,6 +42,10 @@
4343
4444 /* Methods */
4545
 46+ /**
 47+ * @param $resourceLoader ResourceLoader
 48+ * @param $request WebRequest
 49+ */
4650 public function __construct( $resourceLoader, WebRequest $request ) {
4751 global $wgDefaultSkin, $wgResourceLoaderDebug;
4852
——@@ -63,7 +67,7 @@
6468 $this->skin = $wgDefaultSkin;
6569 }
6670 }
67 -
 71+
6872 /**
6973 * Expand a string of the form jquery.foo,bar|jquery.ui.baz,quux to
7074 * an array of module names like array( 'jquery.foo', 'jquery.bar',
——@@ -99,9 +103,10 @@
100104 }
101105 return $retval;
102106 }
103 -
 107+
104108 /**
105109 * Return a dummy ResourceLoaderContext object suitable for passing into things that don't "really" need a context
 110+ * @return ResourceLoaderContext
106111 */
107112 public static function newDummyContext() {
108113 return new self( null, new FauxRequest( array() ) );
——@@ -218,7 +223,7 @@
219224 public function getHash() {
220225 if ( !isset( $this->hash ) ) {
221226 $this->hash = implode( '|', array(
222 - $this->getLanguage(), $this->getDirection(), $this->skin, $this->user,
 227+ $this->getLanguage(), $this->getDirection(), $this->skin, $this->user,
223228 $this->debug, $this->only, $this->version
224229 ) );
225230 }
Index: trunk/phase3/includes/resourceloader/ResourceLoaderUserModule.php
——@@ -38,16 +38,16 @@
3939 $username = $context->getUser();
4040 $userpageTitle = Title::makeTitleSafe( NS_USER, $username );
4141 $userpage = $userpageTitle->getPrefixedDBkey(); // Needed so $excludepages works
42 -
 42+
4343 $pages = array(
4444 "$userpage/common.js" => array( 'type' => 'script' ),
45 - "$userpage/" . $context->getSkin() . '.js' =>
 45+ "$userpage/" . $context->getSkin() . '.js' =>
4646 array( 'type' => 'script' ),
4747 "$userpage/common.css" => array( 'type' => 'style' ),
48 - "$userpage/" . $context->getSkin() . '.css' =>
 48+ "$userpage/" . $context->getSkin() . '.css' =>
4949 array( 'type' => 'style' ),
5050 );
51 -
 51+
5252 // Hack for bug 26283: if we're on a preview page for a CSS/JS page,
5353 // we need to exclude that page from this module. In that case, the excludepage
5454 // parameter will be set to the name of the page we need to exclude.
Index: trunk/phase3/includes/resourceloader/ResourceLoaderFileModule.php
——@@ -223,7 +223,11 @@
224224 $files = $this->getScriptFiles( $context );
225225 return $this->readScriptFiles( $files );
226226 }
227 -
 227+
 228+ /**
 229+ * @param $context ResourceLoaderContext
 230+ * @return array
 231+ */
228232 public function getScriptURLsForDebug( ResourceLoaderContext $context ) {
229233 $urls = array();
230234 foreach ( $this->getScriptFiles( $context ) as $file ) {
——@@ -232,6 +236,9 @@
233237 return $urls;
234238 }
235239
 240+ /**
 241+ * @return bool
 242+ */
236243 public function supportsURLLoading() {
237244 return $this->debugRaw;
238245 }
——@@ -275,6 +282,10 @@
276283 return $styles;
277284 }
278285
 286+ /**
 287+ * @param $context ResourceLoaderContext
 288+ * @return array
 289+ */
279290 public function getStyleURLsForDebug( ResourceLoaderContext $context ) {
280291 $urls = array();
281292 foreach ( $this->getStyleFiles( $context ) as $mediaType => $list ) {
——@@ -582,7 +593,7 @@
583594 $style, $dir, $remoteDir, true
584595 );
585596 }
586 -
 597+
587598 /**
588599 * Safe version of filemtime(), which doesn't throw a PHP warning if the file doesn't exist
589600 * but returns 1 instead.
Index: trunk/phase3/includes/resourceloader/ResourceLoaderModule.php
——@@ -52,11 +52,11 @@
5353 # limit the types of scripts and styles we allow to load on, say, sensitive special
5454 # pages like Special:UserLogin and Special:Preferences
5555 protected $origin = self::ORIGIN_CORE_SITEWIDE;
56 -
 56+
5757 /* Protected Members */
5858
5959 protected $name = null;
60 -
 60+
6161 // In-object cache for file dependencies
6262 protected $fileDeps = array();
6363 // In-object cache for message blob mtime
——@@ -126,18 +126,18 @@
127127 // Stub, override expected
128128 return '';
129129 }
130 -
 130+
131131 /**
132132 * Get the URL or URLs to load for this module's JS in debug mode.
133133 * The default behavior is to return a load.php?only=scripts URL for
134134 * the module, but file-based modules will want to override this to
135135 * load the files directly.
136 - *
 136+ *
137137 * This function is called only when 1) we're in debug mode, 2) there
138138 * is no only= parameter and 3) supportsURLLoading() returns true.
139139 * #2 is important to prevent an infinite loop, therefore this function
140140 * MUST return either an only= URL or a non-load.php URL.
141 - *
 141+ *
142142 * @param $context ResourceLoaderContext: Context object
143143 * @return Array of URLs
144144 */
——@@ -155,7 +155,7 @@
156156 );
157157 return array( $url );
158158 }
159 -
 159+
160160 /**
161161 * Whether this module supports URL loading. If this function returns false,
162162 * getScript() will be used even in cases (debug mode, no only param) where
——@@ -176,13 +176,13 @@
177177 // Stub, override expected
178178 return array();
179179 }
180 -
 180+
181181 /**
182182 * Get the URL or URLs to load for this module's CSS in debug mode.
183183 * The default behavior is to return a load.php?only=styles URL for
184184 * the module, but file-based modules will want to override this to
185185 * load the files directly. See also getScriptURLsForDebug()
186 - *
 186+ *
187187 * @param $context ResourceLoaderContext: Context object
188188 * @return Array: array( mediaType => array( URL1, URL2, ... ), ... )
189189 */
——@@ -212,10 +212,10 @@
213213 // Stub, override expected
214214 return array();
215215 }
216 -
 216+
217217 /**
218218 * Get the group this module is in.
219 - *
 219+ *
220220 * @return String: Group name
221221 */
222222 public function getGroup() {
——@@ -225,14 +225,14 @@
226226
227227 /**
228228 * Get the origin of this module. Should only be overridden for foreign modules.
229 - *
 229+ *
230230 * @return String: Origin name, 'local' for local modules
231231 */
232232 public function getSource() {
233233 // Stub, override expected
234234 return 'local';
235235 }
236 -
 236+
237237 /**
238238 * Where on the HTML page should this module's JS be loaded?
239239 * 'top': in the <head>
——@@ -273,7 +273,7 @@
274274 // Stub, override expected
275275 return array();
276276 }
277 -
 277+
278278 /**
279279 * Get the files this module depends on indirectly for a given skin.
280280 * Currently these are only image files referenced by the module's CSS.
——@@ -300,7 +300,7 @@
301301 }
302302 return $this->fileDeps[$skin];
303303 }
304 -
 304+
305305 /**
306306 * Set preloaded file dependency information. Used so we can load this
307307 * information for all modules at once.
——@@ -310,7 +310,7 @@
311311 public function setFileDependencies( $skin, $deps ) {
312312 $this->fileDeps[$skin] = $deps;
313313 }
314 -
 314+
315315 /**
316316 * Get the last modification timestamp of the message blob for this
317317 * module in a given language.
——@@ -321,7 +321,7 @@
322322 if ( !isset( $this->msgBlobMtime[$lang] ) ) {
323323 if ( !count( $this->getMessages() ) )
324324 return 0;
325 -
 325+
326326 $dbr = wfGetDB( DB_SLAVE );
327327 $msgBlobMtime = $dbr->selectField( 'msg_resource', 'mr_timestamp', array(
328328 'mr_resource' => $this->getName(),
——@@ -337,7 +337,7 @@
338338 }
339339 return $this->msgBlobMtime[$lang];
340340 }
341 -
 341+
342342 /**
343343 * Set a preloaded message blob last modification timestamp. Used so we
344344 * can load this information for all modules at once.
——@@ -347,9 +347,9 @@
348348 public function setMsgBlobMtime( $lang, $mtime ) {
349349 $this->msgBlobMtime[$lang] = $mtime;
350350 }
351 -
 351+
352352 /* Abstract Methods */
353 -
 353+
354354 /**
355355 * Get this module's last modification timestamp for a given
356356 * combination of language, skin and debug mode flag. This is typically
——@@ -364,7 +364,7 @@
365365 // 0 would mean now
366366 return 1;
367367 }
368 -
 368+
369369 /**
370370 * Check whether this module is known to be empty. If a child class
371371 * has an easy and cheap way to determine that this module is
——@@ -412,7 +412,7 @@
413413 $err = $e->getMessage();
414414 $result = "throw new Error(" . Xml::encodeJsVar("JavaScript parse error: $err") . ");";
415415 }
416 -
 416+
417417 $cache->set( $key, $result );
418418 return $result;
419419 } else {
——@@ -420,6 +420,9 @@
421421 }
422422 }
423423
 424+ /**
 425+ * @return JSParser
 426+ */
424427 protected static function javaScriptParser() {
425428 if ( !self::$jsParser ) {
426429 self::$jsParser = new JSParser();
Index: trunk/phase3/includes/resourceloader/ResourceLoaderStartUpModule.php
——@@ -241,6 +241,9 @@
242242 return $out;
243243 }
244244
 245+ /**
 246+ * @return bool
 247+ */
245248 public function supportsURLLoading() {
246249 return false;
247250 }
Index: trunk/phase3/includes/resourceloader/ResourceLoaderUserTokensModule.php
——@@ -32,7 +32,7 @@
3333
3434 /**
3535 * Fetch the tokens for the current user.
36 - *
 36+ *
3737 * @param $context ResourceLoaderContext: Context object
3838 * @return Array: List of tokens keyed by token type
3939 */
——@@ -50,7 +50,7 @@
5151 * @return string
5252 */
5353 public function getScript( ResourceLoaderContext $context ) {
54 - return Xml::encodeJsCall( 'mw.user.tokens.set',
 54+ return Xml::encodeJsCall( 'mw.user.tokens.set',
5555 array( $this->contextUserTokens( $context ) ) );
5656 }
5757
——@@ -60,7 +60,10 @@
6161 public function getGroup() {
6262 return 'private';
6363 }
64 -
 64+
 65+ /**
 66+ * @return array
 67+ */
6568 public function getDependencies() {
6669 return array( 'mediawiki.user' );
6770 }
Index: trunk/phase3/includes/resourceloader/ResourceLoaderWikiModule.php
——@@ -24,35 +24,39 @@
2525
2626 /**
2727 * Abstraction for resource loader modules which pull from wiki pages
28 - *
29 - * This can only be used for wiki pages in the MediaWiki and User namespaces,
30 - * because of its dependence on the functionality of
 28+ *
 29+ * This can only be used for wiki pages in the MediaWiki and User namespaces,
 30+ * because of its dependence on the functionality of
3131 * Title::isValidCssJsSubpage.
3232 */
3333 abstract class ResourceLoaderWikiModule extends ResourceLoaderModule {
34 -
 34+
3535 /* Protected Members */
3636
3737 # Origin is user-supplied code
3838 protected $origin = self::ORIGIN_USER_SITEWIDE;
39 -
 39+
4040 // In-object cache for title mtimes
4141 protected $titleMtimes = array();
42 -
 42+
4343 /* Abstract Protected Methods */
44 -
 44+
 45+ /**
 46+ * @abstract
 47+ * @param $context ResourceLoaderContext
 48+ */
4549 abstract protected function getPages( ResourceLoaderContext $context );
46 -
 50+
4751 /* Protected Methods */
48 -
 52+
4953 /**
5054 * Get the Database object used in getTitleMTimes(). Defaults to the local slave DB
5155 * but subclasses may want to override this to return a remote DB object.
52 - *
 56+ *
5357 * NOTE: This ONLY works for getTitleMTimes() and getModifiedTime(), NOT FOR ANYTHING ELSE.
5458 * In particular, it doesn't work for getting the content of JS and CSS pages. That functionality
5559 * will use the local DB irrespective of the return value of this method.
56 - *
 60+ *
5761 * @return DatabaseBase
5862 */
5963 protected function getDB() {
——@@ -77,7 +81,7 @@
7882 }
7983 return $revision->getRawText();
8084 }
81 -
 85+
8286 /* Methods */
8387
8488 /**
——@@ -112,7 +116,7 @@
113117 */
114118 public function getStyles( ResourceLoaderContext $context ) {
115119 global $wgScriptPath;
116 -
 120+
117121 $styles = array();
118122 foreach ( $this->getPages( $context ) as $titleText => $options ) {
119123 if ( $options['type'] !== 'style' ) {
——@@ -121,7 +125,7 @@
122126 $title = Title::newFromText( $titleText );
123127 if ( !$title || $title->isRedirect() ) {
124128 continue;
125 - }
 129+ }
126130 $media = isset( $options['media'] ) ? $options['media'] : 'all';
127131 $style = $this->getContent( $title );
128132 if ( strval( $style ) === '' ) {
——@@ -174,13 +178,13 @@
175179 if ( isset( $this->titleMtimes[$hash] ) ) {
176180 return $this->titleMtimes[$hash];
177181 }
178 -
 182+
179183 $this->titleMtimes[$hash] = array();
180184 $batch = new LinkBatch;
181185 foreach ( $this->getPages( $context ) as $titleText => $options ) {
182186 $batch->addObj( Title::newFromText( $titleText ) );
183187 }
184 -
 188+
185189 if ( !$batch->isEmpty() ) {
186190 $dbr = $this->getDB();
187191 $res = $dbr->select( 'page',

Status & tagging log