r88457 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r88456‎ \| r88457 \| r88458 >
Date:	15:44, 20 May 2011
Author:	ashley
Status:	ok
Tags:
Comment:	GoogleNewsSitemap: coding style tweaks
Modified paths:	/trunk/extensions/GoogleNewsSitemap/FeedSMItem.php (modified) (history) /trunk/extensions/GoogleNewsSitemap/GoogleNewsSitemap_body.php (modified) (history) /trunk/extensions/GoogleNewsSitemap/SitemapFeed.php (modified) (history)

Diff [purge]

Index: trunk/extensions/GoogleNewsSitemap/GoogleNewsSitemap_body.php
—	—	@@ -2,14 +2,14 @@
3	3	if ( !defined( 'MEDIAWIKI' ) ) die();
4	4
5	5	/**
6		~~- * Class googlenewssitemap creates Atom/RSS feeds for Wikinews~~
7		- **
	6	+ * Class GoogleNewsSitemap creates Atom/RSS feeds for Wikinews
	7	+ *
8	8	* Simple feed using Atom/RSS coupled to DynamicPageList category searching.
9	9	*
10	10	* To use: http://wiki.url/Special:GoogleNewsSitemap?[paramter=value][&parameter2=value]&...
11	11	*
12	12	* Implemented parameters are marked with an @
13		- **
	13	+ *
14	14	* Parameters
15	15	* * categories = string ; default = Published
16	16	* * notcategories = string ; default = null
—	—	@@ -22,7 +22,7 @@
23	23	* * stablepages = string ; default = only
24	24	* * qualitypages = string ; default = include
25	25	* * feed = string ; default = sitemap
26		~~- **/~~
	26	+ */
27	27
28	28	class GoogleNewsSitemap extends SpecialPage {
29	29
—	—	@@ -34,14 +34,14 @@
35	35
36	36	/**
37	37	* Constructor
38		~~- **/~~
	38	+ */
39	39	public function __construct() {
40	40	parent::__construct( 'GoogleNewsSitemap' );
41	41	}
42	42
43	43	/**
44	44	* main()
45		~~- **/~~
	45	+ */
46	46	public function execute( $par ) {
47	47	global $wgContLang, $wgFeedClasses,
48	48	$wgLanguageCode, $wgMemc, $wgOut, $wgGNSMsmaxage;
—	—	@@ -50,7 +50,7 @@
51	51
52	52	// if there's an error parsing the params, bail out and return
53	53	if ( isset( $params['error'] ) ) {
54		~~- wfHttpError( 500, "Internal Server Error", $params['error'] );~~
	54	+ wfHttpError( 500, 'Internal Server Error', $params['error'] );
55	55	return;
56	56	}
57	57
—	—	@@ -63,8 +63,8 @@
64	64	$wgContLang->uc( $params['feed'] ),
65	65	$wgLanguageCode
66	66	),
67		~~- wfMsgExt( 'tagline', array( 'parsemag', 'content') ),~~
68		~~- Title::newMainPage()->getFullUrl()~~
	67	+ wfMsgExt( 'tagline', array( 'parsemag', 'content' ) ),
	68	+ Title::newMainPage()->getFullURL()
69	69	);
70	70	} else {
71	71	// FeedUtils outputs an error if wrong feed type.
—	—	@@ -108,13 +108,13 @@
109	109	*/
110	110	private function getCacheKey( $params, $categories, $notCategories ) {
111	111	global $wgRenderHashAppend;
112		~~- // Note, the implode relies on Title::__toString, which needs php > 5.2~~
	112	+ // Note, the implode relies on Title::__toString, which needs PHP > 5.2
113	113	// Which I think is above the minimum we support.
114	114	$sum = md5( serialize( $params )
115		~~- . implode( "\|", $categories ) . "\|\|"~~
116		~~- . implode( "\|", $notCategories )~~
	115	+ . implode( '\|', $categories ) . '\|\|'
	116	+ . implode( '\|', $notCategories )
117	117	);
118		~~- return wfMemcKey( "GNSM", $sum, $wgRenderHashAppend );~~
	118	+ return wfMemcKey( 'GNSM', $sum, $wgRenderHashAppend );
119	119	}
120	120
121	121	/**
—	—	@@ -163,7 +163,7 @@
164	164	return;
165	165	}
166	166
167		~~- // Fixme: Under what circumstance would cl_timestamp not be set?~~
	167	+ // @todo FIXME: Under what circumstance would cl_timestamp not be set?
168	168	// possibly worth an exception if that happens.
169	169	$pubDate = isset( $row->cl_timestamp ) ? $row->cl_timestamp : wfTimestampNow();
170	170
—	—	@@ -196,7 +196,7 @@
197	197	*
198	198	* @return String All the above info concatenated.
199	199	*/
200		~~- private function getCacheInvalidationInfo ( $params, $categories, $notCategories ) {~~
	200	+ private function getCacheInvalidationInfo( $params, $categories, $notCategories ) {
201	201	wfProfileIn( __METHOD__ );
202	202	$dbr = wfGetDB( DB_SLAVE );
203	203	$cacheInfo = '';
—	—	@@ -212,7 +212,7 @@
213	213	$categoriesKey[] = $key;
214	214	$tsQueries[] = $dbr->selectSQLText(
215	215	'categorylinks',
216		~~- 'MAX(cl_timestamp) as ts',~~
	216	+ 'MAX(cl_timestamp) AS ts',
217	217	array( 'cl_to' => $key ),
218	218	__METHOD__
219	219	);
—	—	@@ -245,11 +245,11 @@
246	246	// Part 2: cl_timestamp:
247	247	// TODO: Double check that the order of the result of union queries
248	248	// is one after another from the order you specified the queries in.
249		~~- $res2 = $dbr->query($dbr->unionQueries( $tsQueries, true ), __METHOD__);~~
	249	+ $res2 = $dbr->query( $dbr->unionQueries( $tsQueries, true ), __METHOD__ );
250	250
251	251	foreach ( $res2 as $row ) {
252		~~- if ( is_null($row->ts) ) {~~
253		~~- $ts = "empty";~~
	252	+ if ( is_null( $row->ts ) ) {
	253	+ $ts = 'empty';
254	254	} else {
255	255	$ts = wfTimestamp( TS_MW, $row->ts );
256	256	}
—	—	@@ -280,7 +280,7 @@
281	281	$conditions['page_namespace'] = $params['namespace'];
282	282	}
283	283
284		~~- wfRunHooks('GoogleNewsSitemap::Query', array($params, &$joins, &$conditions, &$tables));~~
	284	+ wfRunHooks( 'GoogleNewsSitemap::Query', array( $params, &$joins, &$conditions, &$tables ) );
285	285
286	286	switch ( $params['redirects'] ) {
287	287	case self::OPT_ONLY:
—	—	@@ -354,7 +354,7 @@
355	355	* Parse parameters, populates $params
356	356	* @return Array containing the $params, $categories and $notCategories
357	357	* variables that make up the request.
358		~~- **/~~
	358	+ */
359	359	public function getParams() {
360	360	global $wgRequest, $wgGNSMmaxCategories,
361	361	$wgGNSMmaxResultCount, $wgGNSMfallbackCategory;
—	—	@@ -415,7 +415,7 @@
416	416	* @param $default Integer Class constant to return if none match
417	417	* @return Integer Class constant corresponding to value.
418	418	*/
419		~~- private function getIEOVal ( $valName, $default = self::OPT_INCLUDE ) {~~
	419	+ private function getIEOVal( $valName, $default = self::OPT_INCLUDE ) {
420	420	global $wgRequest;
421	421	$val = $wgRequest->getVal( $valName );
422	422	switch ( $val ) {
—	—	@@ -429,12 +429,13 @@
430	430	return $default;
431	431	}
432	432	}
	433	+
433	434	/**
434		~~- * Decode the namespace url parameter.~~
435		~~- * @param $ns String Either numeric ns number, ns name, or special value :all:~~
436		~~- * @return Mixed Integer or false Namespace number or false for no ns filtering.~~
	435	+ * Decode the namespace URL parameter.
	436	+ * @param $ns String Either numeric NS number, NS name, or special value :all:
	437	+ * @return Mixed Integer or false Namespace number or false for no NS filtering.
437	438	*/
438		~~- private function getNS ( $ns ) {~~
	439	+ private function getNS( $ns ) {
439	440	global $wgContLang;
440	441
441	442	$nsNumb = $wgContLang->getNsIndex( $ns );
—	—	@@ -442,7 +443,7 @@
443	444	if ( $nsNumb !== false ) {
444	445	// If they specified something like Talk or Image.
445	446	return $nsNumb;
446		~~- } else if ( is_numeric( $ns ) ) {~~
	447	+ } elseif ( is_numeric( $ns ) ) {
447	448	// If they specified a number.
448	449	$nsVal = intval( $ns );
449	450	if ( $nsVal >= 0 && MWNamespace::exists( $nsVal ) ) {
—	—	@@ -451,7 +452,7 @@
452	453	wfDebug( __METHOD__ . ' Invalid numeric ns number. Using main.' );
453	454	return 0;
454	455	}
455		~~- } else if ( $ns === ':all:' ) {~~
	456	+ } elseif ( $ns === ':all:' ) {
456	457	// Need someway to denote no namespace filtering,
457	458	// This seems as good as any since a namespace can't
458	459	// have colons in it.
—	—	@@ -478,7 +479,7 @@
479	480	global $wgRequest;
480	481
481	482	$value = $wgRequest->getText( $name, $default );
482		~~- $arr = explode( "\|", $value, $max + 2 );~~
	483	+ $arr = explode( '\|', $value, $max + 2 );
483	484	$res = array();
484	485	foreach ( $arr as $name ) {
485	486	$catTitle = Title::newFromText( $name, NS_CATEGORY );
—	—	@@ -500,7 +501,7 @@
501	502	* @param Title $title
502	503	* @return Array of String: list of keywords
503	504	*/
504		~~- public function getKeywords ( $title ) {~~
	505	+ public function getKeywords( $title ) {
505	506	wfProfileIn( __METHOD__ );
506	507	$cats = $title->getParentCategories();
507	508	$res = array();
Index: trunk/extensions/GoogleNewsSitemap/FeedSMItem.php
—	—	@@ -3,9 +3,9 @@
4	4
5	5	/**
6	6	* FeedSMItem Class
7		- **
	7	+ *
8	8	* Base class for basic SiteMap support, for building url containers.
9		~~- **/~~
	9	+ */
10	10	class FeedSMItem extends FeedItem {
11	11
12	12	private $keywords = array();
—	—	@@ -25,17 +25,16 @@
26	26	* An integer for the page name of $title in the specific namespace denoted by that integer.
27	27	*/
28	28	public function __construct( $title, $pubDate, $keywords = '', $comment = true ) {
29		-
30	29	if ( !$title ) {
31	30	// Paranoia
32		~~- throw new MWException( "Invalid title object passed to FeedSMItem" );~~
	31	+ throw new MWException( 'Invalid title object passed to FeedSMItem' );
33	32	}
34	33
35	34	$commentsURL = '';
36	35	if ( $comment === true ) {
37	36	// The comment ns is this article's talk namespace.
38	37	$commentsURL = $title->getTalkPage()->getFullUrl();
39		~~- } else if ( is_int( $comment ) ) {~~
	38	+ } elseif ( is_int( $comment ) ) {
40	39	// There's a specific comments namespace.
41	40	$commentsTitle = Title::makeTitle( $comment, $title->getDBkey() );
42	41	if ( $commentsTitle ) {
—	—	@@ -47,7 +46,7 @@
48	47	$this->keywords = $keywords;
49	48
50	49	parent::__construct( $title->getText(), '' /* Description */,
51		~~- $title->getFullUrl(), $pubDate, '' /* Author */, $commentsURL );~~
	50	+ $title->getFullURL(), $pubDate, '' /* Author */, $commentsURL );
52	51	}
53	52
54	53	/**
—	—	@@ -56,12 +55,12 @@
57	56	* @param FeedItem Original item.
58	57	* @return FeedSMItem Converted item.
59	58	*/
60		~~- static public function newFromFeedItem( FeedItem $item ) {~~
61		~~- // FIXME: This is borked (esp. on history), but better than a fatal (not by much).~~
	59	+ public static function newFromFeedItem( FeedItem $item ) {
	60	+ // @todo FIXME: This is borked (esp. on history), but better than a fatal (not by much).
62	61	// maybe try and get title from url?
63	62	$title = Title::newFromText( $item->getTitle() );
64	63	if ( !$title ) {
65		~~- throw new MWException( "Error getting title object from string in FeedItem." );~~
	64	+ throw new MWException( 'Error getting title object from string in FeedItem.' );
66	65	}
67	66	$date = $item->getDate();
68	67	return new FeedSMItem( $title, $date );
Index: trunk/extensions/GoogleNewsSitemap/SitemapFeed.php
—	—	@@ -16,7 +16,7 @@
17	17
18	18	/**
19	19	* Set the publication language code. Only used if different from
20		~~- * $wgLanguageCode, which could happen if google disagrees with us~~
	20	+ * $wgLanguageCode, which could happen if Google disagrees with us
21	21	* on say what code zh gets.
22	22	* @param String $lang Language code (like en)
23	23	*/
—	—	@@ -46,10 +46,10 @@
47	47
48	48	$this->writer->openURI( 'php://output' );
49	49	$this->writer->setIndent( true );
50		~~- $this->writer->startDocument( "1.0", "UTF-8" );~~
51		~~- $this->writer->startElement( "urlset" );~~
52		~~- $this->writer->writeAttribute( "xmlns", "http://www.sitemaps.org/schemas/sitemap/0.9" );~~
53		~~- $this->writer->writeAttribute( "xmlns:news", "http://www.google.com/schemas/sitemap-news/0.9" );~~
	50	+ $this->writer->startDocument( '1.0', 'UTF-8' );
	51	+ $this->writer->startElement( 'urlset' );
	52	+ $this->writer->writeAttribute( 'xmlns', 'http://www.sitemaps.org/schemas/sitemap/0.9' );
	53	+ $this->writer->writeAttribute( 'xmlns:news', 'http://www.google.com/schemas/sitemap-news/0.9' );
54	54	}
55	55
56	56	/**
—	—	@@ -57,9 +57,8 @@
58	58	* @param FeedSMItem $item to be output
59	59	*/
60	60	public function outItem( $item ) {
61		-
62	61	if ( !( $item instanceof FeedItem ) ) {
63		~~- throw new MWException( "Requires a FeedItem or subclass." );~~
	62	+ throw new MWException( 'Requires a FeedItem or subclass.' );
64	63	}
65	64
66	65	wfProfileIn( __METHOD__ );
—	—	@@ -67,40 +66,40 @@
68	67	$item = FeedSMItem::newFromFeedItem( $item );
69	68	}
70	69
71		~~- $this->writer->startElement( "url" );~~
	70	+ $this->writer->startElement( 'url' );
72	71
73		~~- $this->writer->startElement( "loc" );~~
	72	+ $this->writer->startElement( 'loc' );
74	73	$this->writer->text( $item->getUrl() );
75	74	$this->writer->endElement();
76	75
77		~~- $this->writer->startElement( "news:news" );~~
	76	+ $this->writer->startElement( 'news:news' );
78	77
79		~~- $this->writer->startElement( "news:publication_date" );~~
	78	+ $this->writer->startElement( 'news:publication_date' );
80	79	$this->writer->text( wfTimestamp( TS_ISO_8601, $item->getDate() ) );
81	80	$this->writer->endElement();
82	81
83		~~- $this->writer->startElement( "news:title" );~~
	82	+ $this->writer->startElement( 'news:title' );
84	83	$this->writer->text( $item->getTitle() );
85	84	$this->writer->endElement();
86	85
87		~~- $this->writer->startElement( "news:publication" );~~
88		~~- $this->writer->startElement( "news:name" );~~
	86	+ $this->writer->startElement( 'news:publication' );
	87	+ $this->writer->startElement( 'news:name' );
89	88	$this->writer->text( $this->publicationName );
90	89	$this->writer->endElement();
91		~~- $this->writer->startElement( "news:language" );~~
	90	+ $this->writer->startElement( 'news:language' );
92	91	$this->writer->text( $this->publicationLang );
93	92	$this->writer->endElement();
94	93	$this->writer->endElement();
95	94
96	95	if ( $item->getKeywords() ) {
97		~~- $this->writer->startElement( "news:keywords" );~~
	96	+ $this->writer->startElement( 'news:keywords' );
98	97	$this->writer->text( $item->getKeywords() );
99	98	$this->writer->endElement();
100	99	}
101	100
102	101	$this->writer->endElement(); // end news:news
103	102	if ( $item->getLastMod() ) {
104		~~- $this->writer->startElement( "lastmod" );~~
	103	+ $this->writer->startElement( 'lastmod' );
105	104	$this->writer->text( wfTimestamp( TS_ISO_8601, $item->getLastMod() ) );
106	105	$this->writer->endElement();
107	106	}

Status & tagging log

21:37, 26 May 2011 Reedy (talk | contribs) changed the status of r88457 [removed: new added: ok]