r108391 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r108390‎ | r108391 | r108392 >
Date:08:48, 9 January 2012
Author:maxsem
Status:deferred
Tags:geodata 
Comment:
Refactoring, fixed one edge case
Modified paths:
  • /trunk/extensions/GeoData/CoordinatesParserFunction.php (added) (history)
  • /trunk/extensions/GeoData/GeoData.body.php (modified) (history)
  • /trunk/extensions/GeoData/GeoData.php (modified) (history)
  • /trunk/extensions/GeoData/GeoDataHooks.php (modified) (history)
  • /trunk/extensions/GeoData/tests/TagTest.php (modified) (history)

Diff [purge]

Index: trunk/extensions/GeoData/tests/TagTest.php
@@ -12,6 +12,11 @@
1313 $opt = new ParserOptions();
1414 $out = $p->parse( $input, Title::newMainPage(), $opt );
1515 $this->assertTrue( isset( $out->geoData ) );
 16+ if ( !$expected ) {
 17+ $this->assertFalse( $out->geoData['primary'] );
 18+ $this->assertEmpty( $out->geoData['secondary'] );
 19+ return;
 20+ }
1621 $coord = $out->geoData['primary'] ? $out->geoData['primary'] : $out->geoData['secondary'][0];
1722 foreach ( $expected as $field => $value ) {
1823 $this->assertEquals( $value, $coord->$field, "Checking field $field" );
@@ -24,6 +29,14 @@
2530 '{{#coordinates: 10|20|primary}}',
2631 array( 'lat' => 10, 'lon' => 20, 'globe' => 'earth', 'primary' => true ),
2732 ),
 33+ array(
 34+ '{{#coordinates: 10| primary | 20}}',
 35+ array( 'lat' => 10, 'lon' => 20, 'globe' => 'earth', 'primary' => true ),
 36+ ),
 37+ array(
 38+ '{{#coordinates: primary|10|20}}',
 39+ array( 'lat' => 10, 'lon' => 20, 'globe' => 'earth', 'primary' => true ),
 40+ ),
2841 array(
2942 '{{#coordinates:10|20|globe:Moon dim:10_region:RU-mos}}',
3043 array( 'lat' => 10, 'lon' => 20, 'globe' => 'moon', 'country' => 'RU', 'region' => 'MOS' ),
Index: trunk/extensions/GeoData/CoordinatesParserFunction.php
@@ -0,0 +1,179 @@
 2+<?php
 3+
 4+
 5+class CoordinatesParserFunction {
 6+ /**
 7+ * @var Parser
 8+ */
 9+ private $parser;
 10+
 11+ /**
 12+ * @var ParserOutput
 13+ */
 14+ private $output;
 15+
 16+ private $named = array(),
 17+ $unnamed = array(),
 18+ $info;
 19+
 20+ public function __construct( Parser $parser ) {
 21+ $this->parser = $parser;
 22+ $this->info = GeoData::getCoordInfo();
 23+ }
 24+
 25+ /**
 26+ * Handler for the #coordinates parser function
 27+ *
 28+ * @param Parser $parser
 29+ * @param PPFrame $frame
 30+ * @param Array $args
 31+ * @return Mixed
 32+ */
 33+ public function coordinates( $parser, $frame, $args ) {
 34+ $this->output = $parser->getOutput();
 35+ $this->prepareOutput();
 36+
 37+ $this->unnamed = array();
 38+ $this->named = array();
 39+ $first = trim( $frame->expand( array_shift( $args ) ) );
 40+ $this->addArg( $first );
 41+ foreach ( $args as $arg ) {
 42+ $bits = $arg->splitArg();
 43+ $value = trim( $frame->expand( $bits['value'] ) );
 44+ if ( $bits['index'] === '' ) {
 45+ $this->named[trim( $frame->expand( $bits['name'] ) )] = $value;
 46+ } else {
 47+ $this->addArg( $value );
 48+ }
 49+ }
 50+ $status = GeoData::parseCoordinates( $this->unnamed );
 51+ if ( $status->isGood() ) {
 52+ $coord = $status->value;
 53+ $status = $this->parseTagArgs( $coord );
 54+ if ( $status->isGood() ) {
 55+ $status = $this->applyCoord( $coord );
 56+ if ( $status->isGood() ) {
 57+ return '';
 58+ }
 59+ }
 60+ }
 61+
 62+ $this->addCategory( wfMessage( 'geodata-broken-tags-category' ) );
 63+ $errorText = $status->getWikiText();
 64+ if ( $errorText == '&lt;&gt;' ) {
 65+ // Error that doesn't require a message,
 66+ // can't think of a better way to pass this condition
 67+ return '';
 68+ }
 69+ return array( "<span class=\"error\">{$errorText}</span>", 'noparse' => false );
 70+ }
 71+
 72+ private function addArg( $value ) {
 73+ if ( isset( $this->info['primary'][$value] ) ) {
 74+ $this->named['primary'] = true;
 75+ } elseif ( preg_match( '/\S+?:\S*?([ _]+\S+?:\S*?)*/', $value ) ) {
 76+ $this->named['geohack'] = $value;
 77+ } elseif ( $value != '' ) {
 78+ $this->unnamed[] = $value;
 79+ }
 80+ }
 81+ /**
 82+ * Make sure that parser output has our storage array
 83+ */
 84+ private function prepareOutput() {
 85+ if ( !isset( $this->output->geoData ) ) {
 86+ $this->output->geoData = array(
 87+ 'primary' => false,
 88+ 'secondary' => array(),
 89+ 'limitExceeded' => false,
 90+ );
 91+ }
 92+ }
 93+
 94+ /**
 95+ * Applies a coordinate to parser output
 96+ *
 97+ * @param Coord $coord
 98+ * @return Status: whether save went OK
 99+ */
 100+ private function applyCoord( Coord $coord ) {
 101+ global $wgMaxCoordinatesPerPage;
 102+ $output = $this->output;
 103+ $count = count( $output->geoData['secondary'] ) + ( $output->geoData['primary'] ? 1 : 0 );
 104+ if ( $count >= $wgMaxCoordinatesPerPage ) {
 105+ if ( $output->geoData['limitExceeded'] ) {
 106+ return Status::newFatal( '' );
 107+ }
 108+ $output->geoData['limitExceeded'] = true;
 109+ return Status::newFatal( 'geodata-limit-exceeded' );
 110+ }
 111+ if ( $coord->primary ) {
 112+ if ( $output->geoData['primary'] ) {
 113+ $output->geoData['secondary'][] = $coord;
 114+ return Status::newFatal( 'geodata-multiple-primary' );
 115+ } else {
 116+ $output->geoData['primary'] = $coord;
 117+ }
 118+ } else {
 119+ $output->geoData['secondary'][] = $coord;
 120+ }
 121+ return Status::newGood();
 122+ }
 123+
 124+ /**
 125+ *
 126+ * @param Coord $coord
 127+ */
 128+ private function parseTagArgs( Coord $coord ) {
 129+ global $wgDefaultGlobe, $wgContLang;
 130+ $result = Status::newGood();
 131+ $args = $this->named;
 132+ // fear not of overwriting the stuff we've just received from the geohack param, it has minimum precedence
 133+ if ( isset( $args['geohack'] ) ) {
 134+ $args = array_merge( $this->parseGeoHackArgs( $args['geohack'] ), $args );
 135+ }
 136+ $coord->primary = isset( $args['primary'] );
 137+ $coord->globe = isset( $args['globe'] ) ? $wgContLang->lc( $args['globe'] ) : $wgDefaultGlobe;
 138+ $coord->dim = isset( $args['dim'] ) && is_numeric( $args['dim'] ) && $args['dim'] > 0
 139+ ? $args['dim']
 140+ : null;
 141+ $coord->type = isset( $args['type'] ) ? $args['type'] : null;
 142+ $coord->name = isset( $args['name'] ) ? $args['name'] : null;
 143+ if ( isset( $args['region'] ) ) {
 144+ $code = strtoupper( $args['region'] );
 145+ if ( preg_match( '/([A-Z]{2})(?:-([A-Z0-9]{1,3}))/', $code, $m ) ) {
 146+ $coord->country = $m[1];
 147+ $coord->region = $m[2];
 148+ } else {
 149+ $result->warning( 'geodata-bad-region', $args['region'] ); //@todo: actually use this warning
 150+ }
 151+ }
 152+ return $result;
 153+ }
 154+
 155+ private function parseGeoHackArgs( $str ) {
 156+ $result = array();
 157+ $str = str_replace( '_', ' ', $str ); // per GeoHack docs, spaces and underscores are equivalent
 158+ $parts = explode( ' ', $str );
 159+ foreach ( $parts as $arg ) {
 160+ $keyVal = explode( ':', $arg, 2 );
 161+ if ( count( $keyVal ) != 2 ) {
 162+ continue;
 163+ }
 164+ $result[$keyVal[0]] = $keyVal[1];
 165+ }
 166+ return $result;
 167+ }
 168+
 169+ /**
 170+ * Adds a category to the output
 171+ *
 172+ * @param String|Message $name: Category name
 173+ */
 174+ private function addCategory( $name ) {
 175+ if ( $name instanceof Message ) {
 176+ $name = $name->inContentLanguage()->text();
 177+ }
 178+ $this->output->addCategory( $name, $this->parser->getTitle()->getText() );
 179+ }
 180+}
Property changes on: trunk/extensions/GeoData/CoordinatesParserFunction.php
___________________________________________________________________
Added: svn:eol-style
1181 + native
Index: trunk/extensions/GeoData/GeoData.body.php
@@ -135,51 +135,6 @@
136136 return 0;
137137 }
138138
139 - /**
140 - *
141 - * @param Coord $coord
142 - * @param Array $args
143 - */
144 - public static function parseTagArgs( Coord $coord, $args ) {
145 - global $wgDefaultGlobe, $wgContLang;
146 - $result = Status::newGood();
147 - // fear not of overwriting the stuff we've just received from the geohack param, it has minimum precedence
148 - if ( isset( $args['geohack'] ) ) {
149 - $args = array_merge( self::parseGeoHackArgs( $args['geohack'] ), $args );
150 - }
151 - $coord->primary = isset( $args['primary'] );
152 - $coord->globe = isset( $args['globe'] ) ? $wgContLang->lc( $args['globe'] ) : $wgDefaultGlobe;
153 - $coord->dim = isset( $args['dim'] ) && is_numeric( $args['dim'] ) && $args['dim'] > 0
154 - ? $args['dim']
155 - : null;
156 - $coord->type = isset( $args['type'] ) ? $args['type'] : null;
157 - $coord->name = isset( $args['name'] ) ? $args['name'] : null;
158 - if ( isset( $args['region'] ) ) {
159 - $code = strtoupper( $args['region'] );
160 - if ( preg_match( '/([A-Z]{2})(?:-([A-Z0-9]{1,3}))/', $code, $m ) ) {
161 - $coord->country = $m[1];
162 - $coord->region = $m[2];
163 - } else {
164 - $result->warning( 'geodata-bad-region', $args['region'] ); //@todo: actually use this warning
165 - }
166 - }
167 - return $result;
168 - }
169 -
170 - public static function parseGeoHackArgs( $str ) {
171 - $result = array();
172 - $str = str_replace( '_', ' ', $str ); // per GeoHack docs, spaces and underscores are equivalent
173 - $parts = explode( ' ', $str );
174 - foreach ( $parts as $arg ) {
175 - $keyVal = explode( ':', $arg, 2 );
176 - if ( count( $keyVal ) != 2 ) {
177 - continue;
178 - }
179 - $result[$keyVal[0]] = $keyVal[1];
180 - }
181 - return $result;
182 - }
183 -
184139 public static function getCoordInfo() {
185140 global $wgContLang;
186141 static $result = null;
Index: trunk/extensions/GeoData/GeoData.php
@@ -13,12 +13,13 @@
1414
1515 $dir = dirname( __FILE__ );
1616
 17+$wgAutoloadClasses['ApiQueryCoordinates'] = "$dir/ApiQueryCoordinates.php";
 18+$wgAutoloadClasses['ApiQueryGeoSearch'] = "$dir/ApiQueryGeoSearch.php";
1719 $wgAutoloadClasses['Coord'] = "$dir/GeoData.body.php";
 20+$wgAutoloadClasses['CoordinatesParserFunction'] = "$dir/CoordinatesParserFunction.php";
1821 $wgAutoloadClasses['GeoData'] = "$dir/GeoData.body.php";
1922 $wgAutoloadClasses['GeoDataHooks'] = "$dir/GeoDataHooks.php";
2023 $wgAutoloadClasses['GeoMath'] = "$dir/GeoMath.php";
21 -$wgAutoloadClasses['ApiQueryGeoSearch'] = "$dir/ApiQueryGeoSearch.php";
22 -$wgAutoloadClasses['ApiQueryCoordinates'] = "$dir/ApiQueryCoordinates.php";
2324
2425 $wgAPIListModules['geosearch'] = 'ApiQueryGeoSearch';
2526 $wgAPIPropModules['coordinates'] = 'ApiQueryCoordinates';
Index: trunk/extensions/GeoData/GeoDataHooks.php
@@ -33,7 +33,10 @@
3434 * @param Parser $parser
3535 */
3636 public static function onParserFirstCallInit( &$parser ) {
37 - $parser->setFunctionHook( 'coordinates', 'GeoDataHooks::coordinateHandler', SFH_OBJECT_ARGS );
 37+ $parser->setFunctionHook( 'coordinates',
 38+ array( new CoordinatesParserFunction( $parser ), 'coordinates' ),
 39+ SFH_OBJECT_ARGS
 40+ );
3841 return true;
3942 }
4043
@@ -49,112 +52,6 @@
5053 }
5154
5255 /**
53 - * Handler for the #coordinates parser function
54 - *
55 - * @param Parser $parser
56 - * @param PPFrame $frame
57 - * @param Array $args
58 - * @return Mixed
59 - */
60 - public static function coordinateHandler( $parser, $frame, $args ) {
61 - $output = $parser->getOutput();
62 - self::prepareOutput( $output );
63 - $info = GeoData::getCoordInfo();
64 - $primary = $info['primary'];
65 -
66 - $unnamed = array();
67 - $named = array();
68 - $first = trim( $frame->expand( array_shift( $args ) ) );
69 - if ( $first !== '' ) {
70 - $unnamed[] = $first;
71 - }
72 - foreach ( $args as $arg ) {
73 - $bits = $arg->splitArg();
74 - $value = trim( $frame->expand( $bits['value'] ) );
75 - if ( $bits['index'] === '' ) {
76 - $named[trim( $frame->expand( $bits['name'] ) )] = $value;
77 - } elseif ( isset( $primary[$value] ) ) {
78 - $named['primary'] = true;
79 - } elseif ( preg_match( '/\S+?:\S*?([ _]+\S+?:\S*?)*/', $value ) ) {
80 - $named['geohack'] = $value;
81 - } else {
82 - $unnamed[] = $value;
83 - }
84 - }
85 - $status = GeoData::parseCoordinates( $unnamed );
86 - if ( $status->isGood() ) {
87 - $coord = $status->value;
88 - $status = GeoData::parseTagArgs( $coord, $named );
89 - if ( $status->isGood() ) {
90 - $status = self::applyCoord( $output, $coord );
91 - if ( $status->isGood() ) {
92 - return '';
93 - }
94 - }
95 - }
96 - // Apply tracking category
97 - if ( !$output->geoData['failures'] ) {
98 - $output->geoData['failures'] = true;
99 - $output->addCategory(
100 - wfMessage( 'geodata-broken-tags-category' )->inContentLanguage()->text(),
101 - $parser->getTitle()->getText()
102 - );
103 - }
104 - $errorText = $status->getWikiText();
105 - if ( $errorText == '&lt;&gt;' ) {
106 - // Error condition that doesn't require a message,
107 - // can't think of a better way to pass this condition
108 - return '';
109 - }
110 - return array( "<span class=\"error\">{$errorText}</span>", 'noparse' => false );
111 - }
112 -
113 - /**
114 - * Make sure that parser output has our storage array
115 - * @param ParserOutput $output
116 - */
117 - private static function prepareOutput( ParserOutput $output ) {
118 - if ( !isset( $output->geoData ) ) {
119 - $output->geoData = array(
120 - 'primary' => false,
121 - 'secondary' => array(),
122 - 'failures' => false,
123 - 'limitExceeded' => false,
124 - );
125 - }
126 - }
127 -
128 - /**
129 - * Applies a coordinate to parser output
130 - *
131 - * @param ParserOutput $output
132 - * @param Coord $coord
133 - * @return Status: whether save went OK
134 - */
135 - private static function applyCoord( ParserOutput $output, Coord $coord ) {
136 - global $wgMaxCoordinatesPerPage;
137 - $count = count( $output->geoData['secondary'] ) + ( $output->geoData['primary'] ? 1 : 0 );
138 - if ( $count >= $wgMaxCoordinatesPerPage ) {
139 - if ( $output->geoData['limitExceeded'] ) {
140 - return Status::newFatal( '' );
141 - }
142 - $output->geoData['limitExceeded'] = true;
143 - return Status::newFatal( 'geodata-limit-exceeded' );
144 - }
145 - if ( $coord->primary ) {
146 - if ( $output->geoData['primary'] ) {
147 - $output->geoData['secondary'][] = $coord;
148 - return Status::newFatal( 'geodata-multiple-primary' );
149 - } else {
150 - $output->geoData['primary'] = $coord;
151 - }
152 - } else {
153 - $output->geoData['secondary'][] = $coord;
154 - }
155 - return Status::newGood();
156 - }
157 -
158 - /**
15956 * ArticleDeleteComplete hook handler
16057 * @see https://www.mediawiki.org/wiki/Manual:Hooks/ArticleDeleteComplete
16158 *

Status & tagging log