r76069 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r76068‎ | r76069 | r76070 >
Date:00:59, 5 November 2010
Author:ashley
Status:ok
Tags:
Comment:
RSS: coding style and documentation tweaks
Modified paths:
  • /trunk/extensions/RSS/RSS.php (modified) (history)
  • /trunk/extensions/RSS/RSSData.php (modified) (history)
  • /trunk/extensions/RSS/RSSHooks.php (modified) (history)
  • /trunk/extensions/RSS/RSSParser.php (modified) (history)

Diff [purge]

Index: trunk/extensions/RSS/RSSHooks.php
@@ -1,4 +1,5 @@
22 <?php
 3+
34 class RSSHooks {
45 /**
56 * Tell the parser how to handle <rss> elements
@@ -12,9 +13,10 @@
1314
1415 /**
1516 * Static function wrapping RSSParser to handle rendering of RSS elements
16 - * @param String Text inside the tags.
17 - * @param Array value associative list of the element attributes and their values.
18 - * @param Frame parser context
 17+ * @param $input String: text inside the tags.
 18+ * @param $args Array: value associative list of the element attributes and
 19+ * their values.
 20+ * @param $frame Frame parser context
1921 */
2022 static function renderRss( $input, $args, $parser, $frame ) {
2123 global $wgRSSCacheAge, $wgRSSCacheCompare;
@@ -35,11 +37,13 @@
3638 $status = $rss->fetch();
3739
3840 # Check for errors.
39 - if ( !$status->isGood() )
40 - return wfMsg( 'rss-error', htmlspecialchars( $input), $status->getWikiText() );
 41+ if ( !$status->isGood() ) {
 42+ return wfMsg( 'rss-error', htmlspecialchars( $input ), $status->getWikiText() );
 43+ }
4144
42 - if ( !is_object( $rss->rss ) || !is_array( $rss->rss->items ) )
 45+ if ( !is_object( $rss->rss ) || !is_array( $rss->rss->items ) ) {
4346 return wfMsg( 'rss-empty', htmlspecialchars( $input ) );
 47+ }
4448
4549 return $rss->renderFeed( $parser, $frame );
4650 }
Index: trunk/extensions/RSS/RSSData.php
@@ -6,8 +6,8 @@
77
88 /**
99 * Constructor, takes a DOMDocument and returns an array of parsed items.
10 - * @param DOMDocument The pre-parsed XML Document
11 - * @returns Object RSSData object with a member items that is an array of parsed items,
 10+ * @param $xml DOMDocument: the pre-parsed XML Document
 11+ * @return Object RSSData object with a member items that is an array of parsed items,
1212 */
1313 function __construct( $xml ) {
1414 if ( !( $xml instanceOf DOMDocument ) ) {
@@ -17,25 +17,27 @@
1818 $xpath = new DOMXPath( $xml );
1919 $items = $xpath->query( '/rss/channel/item' );
2020
21 - if($items->length !== 0) {
 21+ if( $items->length !== 0 ) {
2222 foreach ( $items as $item ) {
2323 $bit = array();
2424 foreach ( $item->childNodes as $n ) {
2525 $name = $this->rssTokenToName( $n->nodeName );
2626 if ( $name != null ) {
27 - /* Because for DOMElements the nodeValue is just
 27+ /**
 28+ * Because for DOMElements the nodeValue is just
2829 * the text of the containing element, without any
2930 * tags, it makes this a safe, if unattractive,
3031 * value to use. If you want to allow people to
3132 * mark up their RSS, some more precautions are
32 - * needed. */
 33+ * needed.
 34+ */
3335 $bit[$name] = $n->nodeValue;
3436 }
3537 }
3638 $this->items[] = $bit;
3739 }
3840 } else {
39 - $this->ERROR = "No RSS items found.";
 41+ $this->ERROR = 'No RSS items found.';
4042 return;
4143 }
4244 }
@@ -46,8 +48,9 @@
4749 * same array key. This works on WordPress feeds as-is, but it
4850 * probably needs a way to concert dc:date format dates to be the
4951 * same as pubDate.
50 - * @param String $elementName Name of the element we have
51 - * @returns String Name to map it to
 52+ *
 53+ * @param $n String: name of the element we have
 54+ * @return String Name to map it to
5255 */
5356 protected function rssTokenToName( $n ) {
5457 switch( $n ) {
Index: trunk/extensions/RSS/RSSParser.php
@@ -19,8 +19,8 @@
2020
2121 /**
2222 * Convenience function that takes a space-separated string and returns an array of words
23 - * @param String list of words
24 - * @returns Array words found
 23+ * @param $str String: list of words
 24+ * @return Array words found
2525 */
2626 private static function explodeOnSpaces( $str ) {
2727 $found = preg_split( '# +#', $str );
@@ -33,7 +33,6 @@
3434 * and return an object that can produce rendered output.
3535 */
3636 function __construct( $url, $args ) {
37 -
3837 $this->url = $url;
3938
4039 # Get charset from argument array
@@ -74,32 +73,30 @@
7574
7675 if ( isset( $args['filterout'] ) ) {
7776 $this->filterOut = self::explodeOnSpaces( $args['filterout'] );
78 -
7977 }
8078
8179 if ( isset( $args['template'] ) ) {
8280 $titleObject = Title::newFromText( $args['template'], NS_TEMPLATE );
8381 $article = new Article( $titleObject, 0 );
84 - $this->itemTemplate = $article->fetchContent( );
 82+ $this->itemTemplate = $article->fetchContent();
8583 } else {
8684 $this->itemTemplate = wfMsgNoTrans( 'rss-item' );
8785 }
8886 }
8987
9088 /**
91 - * Return RSS object for the given URL, maintaining caching.
92 - *
93 - * NOTES ON RETRIEVING REMOTE FILES:
94 - * No attempt will be made to fetch remote files if there is something in cache.
95 - *
96 - * NOTES ON FAILED REQUESTS:
97 - * If there is an HTTP error while fetching an RSS object, the cached version
98 - * will be returned, if it exists (and if $wgRSSCacheFreshOnly is false)
99 - *
100 - * @param $url String: URL of RSS file
101 - * @return boolean Status object
102 - */
103 - function fetch( ) {
 89+ * Return RSS object for the given URL, maintaining caching.
 90+ *
 91+ * NOTES ON RETRIEVING REMOTE FILES:
 92+ * No attempt will be made to fetch remote files if there is something in cache.
 93+ *
 94+ * NOTES ON FAILED REQUESTS:
 95+ * If there is an HTTP error while fetching an RSS object, the cached version
 96+ * will be returned, if it exists (and if $wgRSSCacheFreshOnly is false)
 97+ *
 98+ * @return boolean Status object
 99+ */
 100+ function fetch() {
104101 global $wgRSSCacheAge, $wgRSSCacheFreshOnly;
105102 global $wgRSSCacheDirectory, $wgRSSFetchTimeout;
106103 global $wgRSSOutputEncoding, $wgRSSInputEncoding;
@@ -120,7 +117,7 @@
121118 wfDebugLog( 'RSS', 'Outputting cached feed for ' . $this->url );
122119 return Status::newGood();
123120 }
124 - wfDebugLog( 'RSS', 'Cache Failed, fetching ' . $this->url. ' from remote.' );
 121+ wfDebugLog( 'RSS', 'Cache Failed, fetching ' . $this->url . ' from remote.' );
125122
126123 $status = $this->fetchRemote( $key );
127124 return $status;
@@ -128,8 +125,8 @@
129126
130127 /**
131128 * Retrieve the URL from the cache
132 - * @param string $key lookup key to associate with this item
133 - * @returns boolean
 129+ * @param $key String: lookup key to associate with this item
 130+ * @return boolean
134131 */
135132 protected function loadFromCache( $key ) {
136133 global $wgMemc, $wgRSSCacheCompare;
@@ -155,7 +152,7 @@
156153
157154 // We only care if $wgRSSCacheCompare is > 0
158155 if ( $wgRSSCacheCompare && time() - $wgRSSCacheCompare > $lastModified ) {
159 - wfDebugLog( 'RSS', "Content is old enough that we need to check cached content");
 156+ wfDebugLog( 'RSS', 'Content is old enough that we need to check cached content' );
160157 return false;
161158 }
162159
@@ -164,8 +161,8 @@
165162
166163 /**
167164 * Store this objects (e.g. etag, lastModified, and RSS) in the cache.
168 - * @param string $key lookup key to associate with this item
169 - * @returns boolean
 165+ * @param $key String: lookup key to associate with this item
 166+ * @return boolean
170167 */
171168 protected function storeInCache( $key ) {
172169 global $wgMemc, $wgRSSCacheAge;
@@ -183,20 +180,19 @@
184181
185182 /**
186183 * Retrieve a feed.
187 - * @param $url String: URL of the feed.
 184+ * @param $key String:
188185 * @param $headers Array: headers to send along with the request
189186 * @return Status object
190187 */
191188 protected function fetchRemote( $key, array $headers = array()) {
192 - global $wgRSSFetchTimeout;
193 - global $wgRSSUserAgent;
 189+ global $wgRSSFetchTimeout, $wgRSSUserAgent;
194190
195191 if ( $this->etag ) {
196192 wfDebugLog( 'RSS', 'Used etag: ' . $this->etag );
197193 $headers['If-None-Match'] = $this->etag;
198194 }
199195 if ( $this->lastModified ) {
200 - $lm = gmdate('r', $this->lastModified);
 196+ $lm = gmdate( 'r', $this->lastModified );
201197 wfDebugLog( 'RSS', "Used last modified: $lm" );
202198 $headers['If-Modified-Since'] = $lm;
203199 }
@@ -228,7 +224,7 @@
229225 * @param $frame the frame param to pass to recursiveTagParse()
230226 */
231227 function renderFeed( $parser, $frame ) {
232 - $output = "";
 228+ $output = '';
233229 if ( isset( $this->itemTemplate ) ) {
234230 $headcnt = 0;
235231 if ( $this->reversed ) {
@@ -250,15 +246,17 @@
251247 }
252248
253249 /**
254 - * Render each item, filtering it out if necessary, applying any highlighting,
255 - * @param $item an array produced by RSSData where keys are the names of the RSS elements
 250+ * Render each item, filtering it out if necessary, applying any highlighting.
 251+ *
 252+ * @param $item Array: an array produced by RSSData where keys are the
 253+ * names of the RSS elements
256254 * @param $parser the parser param to pass to recursiveTagParse()
257255 * @param $frame the frame param to pass to recursiveTagParse()
258256 */
259257 protected function renderItem( $item, $parser, $frame ) {
260258 $parts = explode( '|', $this->itemTemplate );
261259
262 - $output = "";
 260+ $output = '';
263261 if ( count( $parts ) > 1 && isset( $parser ) && isset( $frame ) ) {
264262 $rendered = array();
265263 foreach ( $this->displayFields as $field ) {
@@ -272,7 +270,7 @@
273271 $left = null;
274272
275273 if ( count( $bits ) == 2 ) {
276 - $left = trim( $bits[0] );
 274+ $left = trim( $bits[0] );
277275 }
278276
279277 if ( isset( $item[$left] ) ) {
@@ -282,18 +280,19 @@
283281 $rendered[] = $part;
284282 }
285283 }
286 - $output .= $parser->recursiveTagParse( implode( " | ", $rendered ), $frame );
 284+ $output .= $parser->recursiveTagParse( implode( ' | ', $rendered ), $frame );
287285 }
288286 return $output;
289287 }
290288
291289 /**
292290 * Parse an HTTP response object into an array of relevant RSS data
293 - * @param $key the to use to store the parsaed response in the cache
 291+ *
 292+ * @param $key String: the key to use to store the parsed response in the cache
294293 * @return parsed RSS object (see RSSParse) or false
295294 */
296295 protected function responseToXML( $key ) {
297 - wfDebugLog( 'RSS', "Got '".$this->client->getStatus()."', updating cache for $key" );
 296+ wfDebugLog( 'RSS', "Got '" . $this->client->getStatus() . "', updating cache for $key" );
298297 if ( $this->client->getStatus() === 304 ) {
299298 # Not modified, update cache
300299 wfDebugLog( 'RSS', "Got 304, updating cache for $key" );
@@ -302,8 +301,8 @@
303302 $this->xml = new DOMDocument;
304303 $raw_xml = $this->client->getContent();
305304
306 - if( $raw_xml == "" ) {
307 - return Status::newFatal( 'rss-parse-error', "No XML content" );
 305+ if( $raw_xml == '' ) {
 306+ return Status::newFatal( 'rss-parse-error', 'No XML content' );
308307 }
309308
310309 wfSuppressWarnings();
@@ -331,11 +330,12 @@
332331
333332 /**
334333 * Determine if a given item should or should not be displayed
335 - * @param associative array that RSSData produced for an <item>
336 - * @returns boolean
 334+ *
 335+ * @param $item Array: associative array that RSSData produced for an <item>
 336+ * @return boolean
337337 */
338338 protected function canDisplay( array $item ) {
339 - $check = "";
 339+ $check = '';
340340
341341 /* We're only going to check the displayable fields */
342342 foreach ( $this->displayFields as $field ) {
@@ -355,9 +355,12 @@
356356
357357 /**
358358 * Filters items in or out if the match a string we're looking for.
359 - * @param String the text to examine
360 - * @param String "filterOut" to check for matches in the filterOut member list. Otherwise, uses the filter member list.
361 - * @returns boolean decision to filter or not.
 359+ *
 360+ * @param $text String: the text to examine
 361+ * @param $filterType String: "filterOut" to check for matches in the
 362+ * filterOut member list.
 363+ * Otherwise, uses the filter member list.
 364+ * @return Boolean: decision to filter or not.
362365 */
363366 protected function filter( $text, $filterType ) {
364367 if ( $filterType === 'filterOut' ) {
@@ -366,10 +369,12 @@
367370 $filter = $this->filter;
368371 }
369372
370 - if ( count( $filter ) == 0 ) return $filterType !== 'filterOut';
 373+ if ( count( $filter ) == 0 ) {
 374+ return $filterType !== 'filterOut';
 375+ }
371376
372377 /* Using : for delimiter here since it'll be quoted automatically. */
373 - $match = preg_match( ':(' . implode( "|", array_map('preg_quote', $filter ) ) . '):i', $text ) ;
 378+ $match = preg_match( ':(' . implode( '|', array_map( 'preg_quote', $filter ) ) . '):i', $text ) ;
374379 if ( $match ) {
375380 return true;
376381 }
@@ -378,8 +383,9 @@
379384
380385 /**
381386 * Highlight the words we're supposed to be looking for
382 - * @param String the text to look in.
383 - * @returns String with matched text highlighted in a <span> element
 387+ *
 388+ * @param $text String: the text to look in.
 389+ * @return String with matched text highlighted in a <span> element
384390 */
385391 protected function highlightTerms( $text ) {
386392 if ( count( $this->highlight ) === 0 ) {
@@ -387,7 +393,7 @@
388394 }
389395
390396 RSSHighlighter::setTerms( $this->highlight );
391 - $highlight = ':'. implode( "|", array_map( 'preg_quote', array_values( $this->highlight ) ) ) . ':i';
 397+ $highlight = ':'. implode( '|', array_map( 'preg_quote', array_values( $this->highlight ) ) ) . ':i';
392398 return preg_replace_callback( $highlight, 'RSSHighlighter::highlightThis', $text );
393399 }
394400 }
@@ -398,7 +404,7 @@
399405
400406 /**
401407 * Set the list of terms to match for the next highlighting session
402 - * @param List of words to match.
 408+ * @param $terms Array: list of words to match.
403409 */
404410 static function setTerms( array $terms ) {
405411 self::$terms = array_flip( array_map( 'strtolower', $terms ) );
@@ -406,23 +412,25 @@
407413
408414 /**
409415 * Actually replace the supplied list of words with HTML code to highlight the words.
410 - * @param List of matched words to highlight. The words are assigned colors based upon the order they were supplied in setTerms()
411 - * @returns String word wrapped in HTML code.
 416+ * @param $match Array: list of matched words to highlight.
 417+ * The words are assigned colors based upon the order
 418+ * they were supplied in setTerms()
 419+ * @return String word wrapped in HTML code.
412420 */
413421 static function highlightThis( $match ) {
414422 $styleStart = "<span style='font-weight: bold; background: none repeat scroll 0%% 0%% rgb(%s); color: %s;'>";
415 - $styleEnd = "</span>";
 423+ $styleEnd = '</span>';
416424
417425 # bg colors cribbed from Google's highlighting of search teerms
418426 $bgcolor = array( '255, 255, 102', '160, 255, 255', '153, 255, 153',
419427 '255, 153, 153', '255, 102, 255', '136, 0, 0', '0, 170, 0', '136, 104, 0',
420428 '0, 70, 153', '153, 0, 153' );
421429 # Spelling out the fg colors instead of using processing time to create this list
422 - $color = array("black", "black", "black", "black", "black",
423 - "white", "white", "white", "white", "white" );
 430+ $color = array( 'black', 'black', 'black', 'black', 'black',
 431+ 'white', 'white', 'white', 'white', 'white' );
424432
425 - $index = self::$terms[strtolower($match[0])] % count( $bgcolor );
 433+ $index = self::$terms[strtolower( $match[0] )] % count( $bgcolor );
426434
427 - return sprintf($styleStart, $bgcolor[$index], $color[$index]). $match[0] .$styleEnd;
 435+ return sprintf( $styleStart, $bgcolor[$index], $color[$index] ) . $match[0] . $styleEnd;
428436 }
429437 }
Index: trunk/extensions/RSS/RSS.php
@@ -4,7 +4,7 @@
55 *
66 * @file
77 * @ingroup Extensions
8 - * @version 1.7
 8+ * @version 1.8
99 * @author mutante, Daniel Kinzler, Rdb, Mafs, Alxndr, Chris Reigrut, K001
1010 * @author Kellan Elliott-McCrea <kellan@protest.net> -- author of MagpieRSS
1111 * @author Jeroen De Dauw
@@ -18,9 +18,6 @@
1919 die( "This is not a valid entry point.\n" );
2020 }
2121
22 -// Agent to use for fetching feeds
23 -$wgRSSUserAgent='MediaWikiRSS/0.01 (+http://www.mediawiki.org/wiki/Extension:RSS) / MediaWiki RSS extension';
24 -
2522 // Extension credits that will show up on Special:Version
2623 $wgExtensionCredits['parserhook'][] = array(
2724 'name' => 'RSS feed',
@@ -62,3 +59,6 @@
6360 $wgRSSInputEncoding = null;
6461 $wgRSSDetectEncoding = true;
6562 $wgRSSFetchTimeout = 5; // 5 second timeout
 63+
 64+// Agent to use for fetching feeds
 65+$wgRSSUserAgent = 'MediaWikiRSS/0.01 (+http://www.mediawiki.org/wiki/Extension:RSS) / MediaWiki RSS extension';
\ No newline at end of file

Status & tagging log