r80836 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r80835‎ | r80836 | r80837 >
Date:21:50, 23 January 2011
Author:jeroendedauw
Status:deferred
Tags:
Comment:
follow up to r80825
Modified paths:
  • /branches/Maps0.8/Maps.php (modified) (history)
  • /branches/Maps0.8/includes/Maps_Location.php (modified) (history)
  • /branches/Maps0.8/includes/Maps_Settings.php (added) (history)
  • /branches/Maps0.8/includes/features/Maps_BasePointMap.php (modified) (history)
  • /branches/Maps0.8/includes/manipulations/Maps_ParamCoordSet.php (deleted) (history)
  • /branches/Maps0.8/includes/manipulations/Maps_ParamLocation.php (modified) (history)
  • /branches/Maps0.8/includes/parserHooks/Maps_DisplayMap.php (modified) (history)
  • /branches/Maps0.8/includes/parserHooks/Maps_DisplayPoint.php (modified) (history)

Diff [purge]

Index: branches/Maps0.8/Maps.php
@@ -72,6 +72,7 @@
7373 $wgAutoloadClasses['iMappingService'] = $incDir . 'iMappingService.php';
7474 $wgAutoloadClasses['MapsMappingServices'] = $incDir . 'Maps_MappingServices.php';
7575 $wgAutoloadClasses['MapsMappingService'] = $incDir . 'Maps_MappingService.php';
 76+ $wgAutoloadClasses['MapsSettings'] = $incDir . 'Maps_Settings.php';
7677
7778 // Autoload the "includes/criteria/" classes.
7879 $criDir = $incDir . 'criteria/';
@@ -80,31 +81,35 @@
8182 $wgAutoloadClasses['CriterionIsLocation'] = $criDir . 'CriterionIsLocation.php';
8283 $wgAutoloadClasses['CriterionMapDimension'] = $criDir . 'CriterionMapDimension.php';
8384 $wgAutoloadClasses['CriterionMapLayer'] = $criDir . 'CriterionMapLayer.php';
 85+ unset( $criDir );
8486
8587 // Autoload the "includes/features/" classes.
8688 $ftDir = $incDir . '/features/';
8789 $wgAutoloadClasses['MapsBaseMap'] = $ftDir . 'Maps_BaseMap.php';
8890 $wgAutoloadClasses['MapsBasePointMap'] = $ftDir . 'Maps_BasePointMap.php';
 91+ unset( $ftDir );
8992
9093 // Autoload the "includes/geocoders/" classes.
9194 $geoDir = $incDir . 'geocoders/';
9295 $wgAutoloadClasses['MapsGeonamesGeocoder'] = $geoDir . 'Maps_GeonamesGeocoder.php';
9396 $wgAutoloadClasses['MapsGoogleGeocoder'] = $geoDir . 'Maps_GoogleGeocoder.php';
9497 $wgAutoloadClasses['MapsYahooGeocoder'] = $geoDir . 'Maps_YahooGeocoder.php';
 98+ unset( $geoDir );
9599
96100 // Autoload the "includes/layers/" classes.
97101 $lyrDir = $incDir . 'layers/';
98102 $wgAutoloadClasses['MapsImageLayer'] = $lyrDir . 'Maps_ImageLayer.php';
99103 $wgAutoloadClasses['MapsKMLLayer'] = $lyrDir . 'Maps_KMLLayer.php';
 104+ unset( $lyrDir );
100105
101106 // Autoload the "includes/manipulations/" classes.
102107 $manDir = $incDir . 'manipulations/';
103 - $wgAutoloadClasses['MapsParamCoordSet'] = $manDir . 'Maps_ParamCoordSet.php';
104108 $wgAutoloadClasses['MapsParamDimension'] = $manDir . 'Maps_ParamDimension.php';
105109 $wgAutoloadClasses['MapsParamImage'] = $manDir . 'Maps_ParamImage.php';
106110 $wgAutoloadClasses['MapsParamLocation'] = $manDir . 'Maps_ParamLocation.php';
107111 $wgAutoloadClasses['MapsParamService'] = $manDir . 'Maps_ParamService.php';
108112 $wgAutoloadClasses['MapsParamZoom'] = $manDir . 'Maps_ParamZoom.php';
 113+ unset( $manDir );
109114
110115 // Autoload the "includes/parserHooks/" classes.
111116 $phDir = $incDir . '/parserHooks/';
@@ -115,6 +120,8 @@
116121 $wgAutoloadClasses['MapsFinddestination'] = $phDir . 'Maps_Finddestination.php';
117122 $wgAutoloadClasses['MapsGeocode'] = $phDir . 'Maps_Geocode.php';
118123 $wgAutoloadClasses['MapsGeodistance'] = $phDir . 'Maps_Geodistance.php';
 124+ unset( $phDir );
 125+ unset( $incDir );
119126
120127 $wgExtensionMessagesFiles['MapsMagic'] = $egMapsDir . 'Maps.i18n.magic.php';
121128
Index: branches/Maps0.8/includes/manipulations/Maps_ParamCoordSet.php
@@ -1,60 +0,0 @@
2 -<?php
3 -
4 -/**
5 - * Parameter manipulation ensuring the value is a coordinate set.
6 - *
7 - * @since 0.7
8 - *
9 - * @file Maps_ParamCoordSet.php
10 - * @ingroup Maps
11 - * @ingroup ParameterManipulations
12 - *
13 - * @author Jeroen De Dauw
14 - */
15 -class MapsParamCoordSet extends ItemParameterManipulation {
16 -
17 - /**
18 - * In some usecases, the parameter values will contain extra location
19 - * metadata, which should be ignored here. This field holds the delimiter
20 - * used to seperata this data from the actual location.
21 - *
22 - * @since 0.7
23 - *
24 - * @var string
25 - */
26 - protected $metaDataSeparator;
27 -
28 - /**
29 - * Constructor.
30 - *
31 - * @since 0.7
32 - */
33 - public function __construct( $metaDataSeparator = false ) {
34 - parent::__construct();
35 -
36 - $this->metaDataSeparator = $metaDataSeparator;
37 - }
38 -
39 - /**
40 - * @see ItemParameterManipulation::doManipulation
41 - *
42 - * @since 0.7
43 - */
44 - public function doManipulation( &$value, Parameter $parameter, array &$parameters ) {
45 - if ( $this->metaDataSeparator !== false ) {
46 - $parts = explode( $this->metaDataSeparator, $value );
47 - $value = array_shift( $parts );
48 - }
49 -
50 - if ( MapsGeocoders::canGeocode() ) {
51 - $value = MapsGeocoders::attemptToGeocodeToString( $value/*, $geoService, $mappingService*/ );
52 - } else {
53 - $value = MapsCoordinateParser::parseAndFormat( $value );
54 - }
55 -
56 - if ( $this->metaDataSeparator !== false ) {
57 - $value = array_merge( array( $value ), $parts );
58 - }
59 - }
60 -
61 -}
\ No newline at end of file
Index: branches/Maps0.8/includes/manipulations/Maps_ParamLocation.php
@@ -18,7 +18,7 @@
1919 * metadata, which should be ignored here. This field holds the delimiter
2020 * used to seperata this data from the actual location.
2121 *
22 - * @since 0.7
 22+ * @since 0.7.2
2323 *
2424 * @var string
2525 */
@@ -27,7 +27,7 @@
2828 /**
2929 * Constructor.
3030 *
31 - * @since 0.7
 31+ * @since 0.7.2
3232 */
3333 public function __construct( $metaDataSeparator = false ) {
3434 parent::__construct();
@@ -38,13 +38,14 @@
3939 /**
4040 * @see ItemParameterManipulation::doManipulation
4141 *
42 - * @since 0.7
 42+ * @since 0.7.2
4343 */
4444 public function doManipulation( &$value, Parameter $parameter, array &$parameters ) {
4545 $parts = $this->metaDataSeparator === false ? array( $value ) : explode( $this->metaDataSeparator, $value );
46 -
47 - $value = new MapsLocation( array_shift( $parts ) );
4846
 47+ $value = array_shift( $parts );
 48+ $value = new MapsLocation( $value );
 49+
4950 if ( $title = array_shift( $parts ) ) {
5051 $value->setTitle( $title );
5152 }
Index: branches/Maps0.8/includes/parserHooks/Maps_DisplayPoint.php
@@ -68,7 +68,7 @@
6969 $params['coordinates'] = new ListParameter( 'coordinates', $type === ParserHook::TYPE_FUNCTION ? ';' : "\n" );
7070 $params['coordinates']->addAliases( 'coords', 'location', 'address', 'addresses', 'locations' );
7171 $params['coordinates']->addCriteria( new CriterionIsLocation( $type === ParserHook::TYPE_FUNCTION ? '~' : '|' ) );
72 - $params['coordinates']->addManipulations( new MapsParamCoordSet( $type === ParserHook::TYPE_FUNCTION ? '~' : '|' ) );
 72+ $params['coordinates']->addManipulations( new MapsParamLocation( $type === ParserHook::TYPE_FUNCTION ? '~' : '|' ) );
7373 $params['coordinates']->addDependencies( 'mappingservice', 'geoservice' );
7474 $params['coordinates']->setDescription( wfMsg( 'maps-displaypoints-par-coordinates' ) );
7575
Index: branches/Maps0.8/includes/parserHooks/Maps_DisplayMap.php
@@ -68,7 +68,7 @@
6969 $params['coordinates'] = new Parameter( 'coordinates' );
7070 $params['coordinates']->addAliases( 'coords', 'location', 'address' );
7171 $params['coordinates']->addCriteria( new CriterionIsLocation() );
72 - $params['coordinates']->addManipulations( new MapsParamCoordSet() );
 72+ $params['coordinates']->addManipulations( new MapsParamLocation() );
7373 $params['coordinates']->addDependencies( 'mappingservice', 'geoservice' );
7474 $params['coordinates']->setDescription( wfMsg( 'maps-displaymap-par-coordinates' ) );
7575
Index: branches/Maps0.8/includes/Maps_Settings.php
@@ -0,0 +1,23 @@
 2+<?php
 3+
 4+/**
 5+ * Static class for hooks handled by the Live Translate extension.
 6+ *
 7+ * @since 0.1
 8+ *
 9+ * @file LiveTranslate.hooks.php
 10+ * @ingroup LiveTranslate
 11+ *
 12+ * @author Jeroen De Dauw
 13+ */
 14+final class MapsSettings extends ExtensionSettings {
 15+
 16+
 17+
 18+}
 19+
 20+abstract class ExtensionSettings {
 21+
 22+
 23+
 24+}
Property changes on: branches/Maps0.8/includes/Maps_Settings.php
___________________________________________________________________
Added: svn:eol-style
125 + native
Index: branches/Maps0.8/includes/features/Maps_BasePointMap.php
@@ -25,7 +25,7 @@
2626 * @since 0.8
2727 *
2828 * @param array $params
29 - * @param $parser
 29+ * @param Parser $parser
3030 *
3131 * @return string
3232 */
@@ -69,15 +69,8 @@
7070 * @return html
7171 */
7272 public final function renderMap( array $params, Parser $parser ) {
73 - $this->setMarkerData( $params );
74 -
75 - $this->setCentre( $params );
 73+ $this->handleMarkerData( $params );
7674
77 - // TODO
78 - /*if ( count( $this->markerData ) <= 1 && $this->zoom == 'null' ) {
79 - $this->zoom = $this->service->getDefaultZoom();
80 - }*/
81 -
8275 $output = $this->getMapHTML( $params, $parser ) . $this->getJSON( $params, $parser );
8376
8477 global $wgTitle;
@@ -128,122 +121,32 @@
129122 }
130123
131124 /**
132 - * Fills the $markerData array with the locations and their meta data.
 125+ * Converts the data in the coordinates parameter to JSON-ready objects.
 126+ * These get stored in the locations parameter, and the coordinates on gets deleted.
 127+ *
 128+ * @since 0.8
 129+ *
 130+ * @param array &$params
133131 */
134 - private function setMarkerData() {
 132+ protected function handleMarkerData( array &$params ) {
135133 global $wgTitle;
136 -
137 - // New parser object to render popup contents with.
 134+
138135 $parser = new Parser();
139 -
140 - $this->title = $parser->parse( $this->title, $wgTitle, new ParserOptions() )->getText();
141 - $this->label = $parser->parse( $this->label, $wgTitle, new ParserOptions() )->getText();
142 -
143 - // Each $args is an array containg the coordinate set as first element, possibly followed by meta data.
144 - foreach ( $this->coordinates as $args ) {
145 - $markerData = MapsCoordinateParser::parseCoordinates( array_shift( $args ) );
 136+ $iconUrl = MapsMapper::getImageUrl( $params['icon'] );
 137+ $params['locations'] = array();
146138
147 - if ( !$markerData ) continue;
148 -
149 - $markerData = array( $markerData['lat'], $markerData['lon'] );
150 -
151 - if ( count( $args ) > 0 ) {
152 - // Parse and add the point specific title if it's present.
153 - $markerData['title'] = $parser->parse( $args[0], $wgTitle, new ParserOptions() )->getText();
 139+ foreach ( $params['coordinates'] as $location ) {
 140+ if ( $location->isValid() ) {
 141+ $jsonObj = $location->getJSONObject( $params['title'], $params['label'], $iconUrl );
154142
155 - if ( count( $args ) > 1 ) {
156 - // Parse and add the point specific label if it's present.
157 - $markerData['label'] = $parser->parse( $args[1], $wgTitle, new ParserOptions() )->getText();
158 -
159 - if ( count( $args ) > 2 ) {
160 - // Add the point specific icon if it's present.
161 - $markerData['icon'] = $args[2];
162 - }
163 - }
 143+ $jsonObj['title'] = $parser->parse( $jsonObj['title'], $wgTitle, new ParserOptions() )->getText();
 144+ $jsonObj['text'] = $parser->parse( $jsonObj['text'], $wgTitle, new ParserOptions() )->getText();
 145+
 146+ $params['locations'][] = $jsonObj;
164147 }
165 -
166 - // If there is no point specific icon, use the general icon parameter when available.
167 - if ( !array_key_exists( 'icon', $markerData ) ) {
168 - $markerData['icon'] = $this->icon;
169 - }
170 -
171 - if ( $markerData['icon'] != '' ) {
172 - $markerData['icon'] = MapsMapper::getImageUrl( $markerData['icon'] );
173 - }
174 -
175 - // Temporary fix, will refactor away later
176 - // TODO
177 - $markerData = array_values( $markerData );
178 - if ( count( $markerData ) < 5 ) {
179 - if ( count( $markerData ) < 4 ) {
180 - $markerData[] = '';
181 - }
182 - $markerData[] = '';
183 - }
184 -
185 - $this->markerData[] = $markerData;
186148 }
187149
 150+ unset( $params['coordinates'] );
188151 }
189152
190 - /**
191 - * Sets the $centre_lat and $centre_lon fields.
192 - * Note: this needs to be done AFTRE the maker coordinates are set.
193 - */
194 - protected function setCentre( array &$params ) {
195 - global $egMapsDefaultMapCentre;
196 -
197 - if ( $this->centre === false ) {
198 - if ( count( $this->markerData ) == 1 ) {
199 - // If centre is not set and there is exactly one marker, use its coordinates.
200 - $this->centreLat = Xml::escapeJsString( $this->markerData[0][0] );
201 - $this->centreLon = Xml::escapeJsString( $this->markerData[0][1] );
202 - }
203 - elseif ( count( $this->markerData ) > 1 ) {
204 - // If centre is not set and there are multiple markers, set the values to null,
205 - // to be auto determined by the JS of the mapping API.
206 - $this->centreLat = 'null';
207 - $this->centreLon = 'null';
208 - }
209 - else {
210 - $this->setCentreToDefault( $params );
211 - }
212 -
213 - }
214 - else { // If a centre value is set, geocode when needed and use it.
215 - $this->centre = MapsGeocoders::attemptToGeocode( $this->centre, $this->geoservice, $this->service->getName() );
216 -
217 - // If the centre is not false, it will be a valid coordinate, which can be used to set the latitude and longitutde.
218 - if ( $this->centre ) {
219 - $this->centreLat = Xml::escapeJsString( $this->centre['lat'] );
220 - $this->centreLon = Xml::escapeJsString( $this->centre['lon'] );
221 - }
222 - else { // If it's false, the coordinate was invalid, or geocoding failed. Either way, the default's should be used.
223 - $this->setCentreToDefault( $params );
224 - }
225 - }
226 -
227 - }
228 -
229 - /**
230 - * Attempts to set the centreLat and centreLon fields to the Maps default.
231 - * When this fails (aka the default is not valid), an exception is thrown.
232 - *
233 - * @since 0.7
234 - */
235 - protected function setCentreToDefault( array &$params ) {
236 - global $egMapsDefaultMapCentre;
237 -
238 - $centre = MapsGeocoders::attemptToGeocode( $egMapsDefaultMapCentre, $this->geoservice, $this->service->getName() );
239 -
240 - if ( $centre === false ) {
241 - throw new Exception( 'Failed to parse the default centre for the map. Please check the value of $egMapsDefaultMapCentre.' );
242 - }
243 - else {
244 - //$params['centre'] =
245 - $this->centreLat = Xml::escapeJsString( $centre['lat'] );
246 - $this->centreLon = Xml::escapeJsString( $centre['lon'] );
247 - }
248 - }
249 -
250 -}
\ No newline at end of file
 153+}
Index: branches/Maps0.8/includes/Maps_Location.php
@@ -102,7 +102,7 @@
103103 *
104104 * @since 0.7.1
105105 */
106 - public function construct( $coordsOrAddress = null, $format = Maps_COORDS_FLOAT, $directional = false, $separator = ',' ) {
 106+ public function __construct( $coordsOrAddress = null, $format = Maps_COORDS_FLOAT, $directional = false, $separator = ',' ) {
107107 $this->format = $format;
108108 $this->directional = $directional;
109109 $this->separator = $separator;
@@ -282,6 +282,17 @@
283283 }
284284
285285 /**
 286+ * Returns if there is any icon.
 287+ *
 288+ * @since 0.8
 289+ *
 290+ * @return boolean
 291+ */
 292+ public function hasIcon() {
 293+ return $this->icon != '';
 294+ }
 295+
 296+ /**
286297 * Sets the icon
287298 *
288299 * @since 0.7.2
@@ -293,6 +304,17 @@
294305 }
295306
296307 /**
 308+ * Returns if there is any title.
 309+ *
 310+ * @since 0.8
 311+ *
 312+ * @return boolean
 313+ */
 314+ public function hasTitle() {
 315+ return $this->title != '';
 316+ }
 317+
 318+ /**
297319 * Returns the title.
298320 *
299321 * @since 0.7.2
@@ -304,6 +326,17 @@
305327 }
306328
307329 /**
 330+ * Returns if there is any text.
 331+ *
 332+ * @since 0.8
 333+ *
 334+ * @return boolean
 335+ */
 336+ public function hasText() {
 337+ return $this->text != '';
 338+ }
 339+
 340+ /**
308341 * Returns the text.
309342 *
310343 * @since 0.7.2
@@ -325,4 +358,23 @@
326359 return $this->icon;
327360 }
328361
 362+ /**
 363+ * Returns an object that can directly be converted to JS using json_encode or similar.
 364+ *
 365+ * @since 0.8
 366+ *
 367+ * @return object
 368+ */
 369+ public function getJSONObject( $defText = '', $defTitle = '', $defIconUrl = '' ) {
 370+ return array(
 371+ 'lat' => $this->getLatitude(),
 372+ 'lon' => $this->getLongitude(),
 373+ 'alt' => $this->getAltitude(),
 374+ 'text' => $this->hasText() ? $this->getText() : $defText,
 375+ 'title' => $this->hasTitle() ? $this->getTitle() : $defTitle,
 376+ 'address' => $this->getAddress( false ),
 377+ 'icon' => $this->hasIcon() ? MapsMapper::getImageUrl( $this->getIcon() ) : $defIconUrl
 378+ );
 379+ }
 380+
329381 }

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r80825a bunch of refactoring, stuff probably rather broken nowjeroendedauw18:34, 23 January 2011

Status & tagging log