Mostly fixed the display_map geoservice parameter

It is now used for the main parameter. Still needs fixing for the
center parameter

Maps.php

@@ -70,7 +70,7 @@
 		);
 	}
 
-	define( 'Maps_VERSION' , '4.0.2' );
+	define( 'Maps_VERSION' , '4.0.3 alpha' );
 	define( 'SM_VERSION', Maps_VERSION );
 
 	if ( $GLOBALS['egMapsGMaps3Language'] === '' ) {

README.md

@@ -72,7 +72,7 @@ To run the tests with MediaWiki, change into `tests/phpunit` of your MediaWiki i
     php phpunit.php --wiki wiki --configuration suite.xml ../../extensions/Maps/tests/
 
 Where you either update `wiki` to match your wikis name, or drop the parameter. The above command
-works without modification if you are using the [MediaWiki Vagrant](https://www.mediawiki.org/wiki/MediaWiki-Vagrant)
+works without modification if you are using the [MediaWiki Vagrant](https://www.mediawiki.org/wiki/MediaWiki-Vagrant).
 
 ## Links
 

RELEASE-NOTES.md

@@ -1,6 +1,12 @@
 These are the release notes for the [Maps extension](README.md). For an overview of the different releases and which versions of PHP and MediaWiki they support, see the [platform compatibility tables](INSTALL.md#platform-compatibility-and-release-status).
 
 
+## Maps 4.0.3
+
+Under development
+
+* The `display_map` parser hook now correctly uses its `geoservice` parameter
+
 ## Maps 4.0.2
 
 Released on December 4th, 2016.

SemanticMaps/src/SM_GeoCoordsValue.php

@@ -149,8 +149,7 @@ private function findValueParts( $value ) {
 	 * @param string $coordinates
 	 */
 	private function tryParseAndSetDataItem( $coordinates ) {
-		$options = new \ValueParsers\ParserOptions();
-		$parser = new GeoCoordinateParser( $options );
+		$parser = new GeoCoordinateParser();
 
 		try {
 			$value = $parser->parse( $coordinates );

includes/Geocoders.php

@@ -121,23 +121,23 @@ public static function init() {
 	 * @since 0.7
 	 *
 	 * @param string $coordsOrAddress
-	 * @param string $geoservice
+	 * @param string $geoService
 	 * @param boolean $checkForCoords
 	 *
 	 * @return LatLongValue|false
 	 */
-	public static function attemptToGeocode( $coordsOrAddress, $geoservice = '', $checkForCoords = true ) {
+	public static function attemptToGeocode( $coordsOrAddress, $geoService = '', $checkForCoords = true ) {
 		if ( $checkForCoords ) {
-			$coordinateParser = new GeoCoordinateParser( new \ValueParsers\ParserOptions() );
+			$coordinateParser = new GeoCoordinateParser();
 
 			try {
 				return $coordinateParser->parse( $coordsOrAddress );
 			}
 			catch ( ParseException $parseException ) {
-				return self::geocode( $coordsOrAddress, $geoservice );
+				return self::geocode( $coordsOrAddress, $geoService );
 			}
 		} else {
-			return self::geocode( $coordsOrAddress, $geoservice );
+			return self::geocode( $coordsOrAddress, $geoService );
 		}
 	}
 

includes/Maps_DisplayMapRenderer.php

@@ -1,7 +1,8 @@
 <?php
+
 use Maps\Element;
-use Maps\Elements\Line;
 use Maps\Elements\Location;
+use ValueParsers\ParserOptions as ValueParserOptions;
 
 /**
  * Class handling the #display_map rendering.
@@ -12,64 +13,12 @@
  */
 class MapsDisplayMapRenderer {
 
-	/**
-	 * @since 2.0
-	 *
-	 * @var iMappingService
-	 */
-	protected $service;
+	private $service;
 
-	/**
-	 * Constructor.
-	 *
-	 * @param iMappingService $service
-	 */
-	public function __construct( iMappingService $service ) {
+	public function __construct( MapsMappingService $service ) {
 		$this->service = $service;
 	}
 
-	/**
-	 * Returns the HTML to display the map.
-	 *
-	 * @since 2.0
-	 *
-	 * @param array $params
-	 * @param Parser $parser
-	 * @param string $mapName
-	 *
-	 * @return string
-	 */
-	protected function getMapHTML( array $params, Parser $parser, $mapName ) {
-		return Html::rawElement(
-			'div',
-			[
-				'id' => $mapName,
-				'style' => "width: {$params['width']}; height: {$params['height']}; background-color: #cccccc; overflow: hidden;",
-				'class' => 'maps-map maps-' . $this->service->getName()
-			],
-			wfMessage( 'maps-loading-map' )->inContentLanguage()->escaped() .
-				Html::element(
-					'div',
-					[ 'style' => 'display:none', 'class' => 'mapdata' ],
-					FormatJson::encode( $this->getJSONObject( $params, $parser ) )
-				)
-		);
-	}
-
-	/**
-	 * Returns a PHP object to encode to JSON with the map data.
-	 *
-	 * @since 2.0
-	 *
-	 * @param array $params
-	 * @param Parser $parser
-	 *
-	 * @return mixed
-	 */
-	protected function getJSONObject( array $params, Parser $parser ) {
-		return $params;
-	}
-
 	/**
 	 * Handles the request from the parser hook by doing the work that's common for all
 	 * mapping services, calling the specific methods and finally returning the resulting output.
@@ -84,7 +33,7 @@ public final function renderMap( array $params, Parser $parser ) {
 
 		$mapName = $this->service->getMapId();
 
-		$output = $this->getMapHTML( $params, $parser, $mapName );
+		$output = $this->getMapHTML( $params, $mapName );
 
 		$configVars = Skin::makeVariablesScript( $this->service->getConfigVariables() );
 
@@ -99,17 +48,35 @@ public final function renderMap( array $params, Parser $parser ) {
 	}
 
 	/**
-	 * Converts the data in the coordinates parameter to JSON-ready objects.
-	 * These get stored in the locations parameter, and the coordinates on gets deleted.
-	 *
-	 * FIXME: complexity
+	 * Returns the HTML to display the map.
 	 *
-	 * @since 1.0
+	 * @param array $params
+	 * @param string $mapName
 	 *
-	 * @param array &$params
-	 * @param Parser $parser
+	 * @return string
 	 */
-	protected function handleMarkerData( array &$params, Parser $parser ) {
+	protected function getMapHTML( array $params, $mapName ) {
+		return Html::rawElement(
+			'div',
+			[
+				'id' => $mapName,
+				'style' => "width: {$params['width']}; height: {$params['height']}; background-color: #cccccc; overflow: hidden;",
+				'class' => 'maps-map maps-' . $this->service->getName()
+			],
+			wfMessage( 'maps-loading-map' )->inContentLanguage()->escaped() .
+			Html::element(
+				'div',
+				[ 'style' => 'display:none', 'class' => 'mapdata' ],
+				FormatJson::encode( $params )
+			)
+		);
+	}
+
+	/**
+	 * Converts the data in the coordinates parameter to JSON-ready objects.
+	 * These get stored in the locations parameter, and the coordinates on gets deleted.
+	 */
+	private function handleMarkerData( array &$params, Parser $parser ) {
 		if ( is_object( $params['centre'] ) ) {
 			$params['centre'] = $params['centre']->getJSONObject();
 		}
@@ -120,39 +87,60 @@ protected function handleMarkerData( array &$params, Parser $parser ) {
 			$params['wmsoverlay'] = $params['wmsoverlay']->getJSONObject();
 		}
 
+		$params['locations'] = $this->getLocationJson( $params, $parserClone );
+
+		unset( $params['coordinates'] );
+
+		$this->handleShapeData( $params, $parserClone );
+
+		if ( $params['mappingservice'] === 'openlayers' ) {
+			$params['layers'] = self::evilOpenLayersHack( $params['layers'] );
+		}
+	}
+
+	private function getLocationJson( array $params, $parserClone ) {
 		$iconUrl = MapsMapper::getFileUrl( $params['icon'] );
 		$visitedIconUrl = MapsMapper::getFileUrl( $params['visitedicon'] );
-		$params['locations'] = [];
-
-		/**
-		 * @var Location $location
-		 */
-		foreach ( $params['coordinates'] as $location ) {
-			$jsonObj = $location->getJSONObject( $params['title'], $params['label'], $iconUrl, '', '',$visitedIconUrl);
-
-			$jsonObj['title'] = $parserClone->parse( $jsonObj['title'], $parserClone->getTitle(), new ParserOptions() )->getText();
-			$jsonObj['text'] = $parserClone->parse( $jsonObj['text'], $parserClone->getTitle(), new ParserOptions() )->getText();
-			if ( isset( $jsonObj['inlineLabel'] ) ) {
-				$jsonObj['inlineLabel'] = strip_tags($parserClone->parse( $jsonObj['inlineLabel'], $parserClone->getTitle(), new ParserOptions() )->getText(),'<a><img>');
-			}
 
-			$hasTitleAndtext = $jsonObj['title'] !== '' && $jsonObj['text'] !== '';
-			$jsonObj['text'] = ( $hasTitleAndtext ? '<b>' . $jsonObj['title'] . '</b><hr />' : $jsonObj['title'] ) . $jsonObj['text'];
-			$jsonObj['title'] = strip_tags( $jsonObj['title'] );
+		$parser = new \Maps\LocationParser( new ValueParserOptions( [
+			'geoService' => $params['geoservice']
+		] ) );
+
+		$locationJsonObjects = [];
+
+		foreach ( $params['coordinates'] as $coordinatesOrAddress ) {
+			try {
+				$location = $parser->stringParse( $coordinatesOrAddress );
+			}
+			catch ( \Exception $ex ) {
+				// TODO: somehow report this to the user
+				continue;
+			}
 
-			$params['locations'][] = $jsonObj;
+			$locationJsonObjects[] = $this->getLocationJsonObject( $location, $params, $iconUrl, $visitedIconUrl, $parserClone );
 		}
 
-		unset( $params['coordinates'] );
+		return $locationJsonObjects;
+	}
 
-		$this->handleShapeData( $params, $parserClone );
+	private function getLocationJsonObject( Location $location, array $params, $iconUrl, $visitedIconUrl, Parser $parserClone ) {
+		$jsonObj = $location->getJSONObject( $params['title'], $params['label'], $iconUrl, '', '', $visitedIconUrl );
 
-		if ( $params['mappingservice'] === 'openlayers' ) {
-			$params['layers'] = self::evilOpenLayersHack( $params['layers'] );
+		$jsonObj['title'] = $parserClone->parse( $jsonObj['title'], $parserClone->getTitle(), new ParserOptions() )->getText();
+		$jsonObj['text'] = $parserClone->parse( $jsonObj['text'], $parserClone->getTitle(), new ParserOptions() )->getText();
+
+		if ( isset( $jsonObj['inlineLabel'] ) ) {
+			$jsonObj['inlineLabel'] = strip_tags($parserClone->parse( $jsonObj['inlineLabel'], $parserClone->getTitle(), new ParserOptions() )->getText(),'<a><img>');
 		}
+
+		$hasTitleAndtext = $jsonObj['title'] !== '' && $jsonObj['text'] !== '';
+		$jsonObj['text'] = ( $hasTitleAndtext ? '<b>' . $jsonObj['title'] . '</b><hr />' : $jsonObj['title'] ) . $jsonObj['text'];
+		$jsonObj['title'] = strip_tags( $jsonObj['title'] );
+
+		return $jsonObj;
 	}
 
-	protected function handleShapeData( array &$params, Parser $parserClone ) {
+	private function handleShapeData( array &$params, Parser $parserClone ) {
 		$textContainers = [
 			&$params['lines'] ,
 			&$params['polygons'] ,

includes/parserhooks/Maps_DisplayMap.php

@@ -47,18 +47,15 @@ protected function getParameterInfo( $type ) {
 		$params['mappingservice']['feature'] = 'display_map';
 
 		$params['coordinates'] = [
-			'type' => 'mapslocation',
+			'type' => 'string',
 			'aliases' => [ 'coords', 'location', 'address', 'addresses', 'locations', 'points' ],
-			'dependencies' => [ 'mappingservice', 'geoservice' ],
 			'default' => [],
 			'islist' => true,
 			'delimiter' => $type === ParserHook::TYPE_FUNCTION ? ';' : "\n",
 			'message' => 'maps-displaymap-par-coordinates',
 		];
 
-		$params = array_merge( $params, self::getCommonMapParams() );
-
-		return $params;
+		return array_merge( $params, self::getCommonMapParams() );
 	}
 
 	/**
@@ -184,22 +181,26 @@ protected function getDefaultParameters( $type ) {
 	 * @return string
 	 */
 	public function render( array $parameters ) {
-		// Get the instance of the service class.
-		$service = MapsMappingServices::getServiceInstance( $parameters['mappingservice'] );
-		
-		$mapClass = new MapsDisplayMapRenderer( $service );
+		$this->defaultMapZoom( $parameters );
+		$this->trackMap();
+
+		$renderer = new MapsDisplayMapRenderer( MapsMappingServices::getServiceInstance( $parameters['mappingservice'] ) );
 
+		return $renderer->renderMap( $parameters, $this->parser );
+	}
+
+	private function defaultMapZoom( &$parameters ) {
 		$fullParams = $this->validator->getParameters();
 
 		if ( array_key_exists( 'zoom', $fullParams ) && $fullParams['zoom']->wasSetToDefault() && count( $parameters['coordinates'] ) > 1 ) {
 			$parameters['zoom'] = false;
 		}
+	}
 
-		global $egMapsEnableCategory;
-		if ($egMapsEnableCategory) {
+	private function trackMap() {
+		if ( $GLOBALS['egMapsEnableCategory'] ) {
 			$this->parser->addTrackingCategory( 'maps-tracking-category' );
 		}
-		return $mapClass->renderMap( $parameters, $this->parser );
 	}
 
 	/**

includes/parsers/LocationParser.php

@@ -28,6 +28,7 @@ public function __construct( ParserOptions $options = null ) {
 		parent::__construct( $options );
 
 		$this->defaultOption( 'useaddressastitle', false );
+		$this->defaultOption( 'geoService', '' );
 	}
 
 	/**
@@ -38,7 +39,7 @@ public function __construct( ParserOptions $options = null ) {
 	 * @param string $value
 	 *
 	 * @return Location
-	 * @throws MWException
+	 * @throws ParseException
 	 */
 	public function stringParse( $value ) {
 		$separator = '~';
@@ -118,7 +119,7 @@ private function getExpandedLink( $link ) {
 	 */
 	private function stringToLatLongValue( $location ) {
 		if ( Geocoders::canGeocode() ) {
-			$latLongValue = Geocoders::attemptToGeocode( $location );
+			$latLongValue = Geocoders::attemptToGeocode( $location, $this->getOption( 'geoService' ) );
 
 			if ( $latLongValue === false ) {
 				throw new ParseException( 'Failed to parse or geocode' );

tests/Integration/parserhooks/DisplayMapTest.php

@@ -49,10 +49,6 @@ public function parametersProvider() {
 		return $this->arrayWrap( $paramLists );
 	}
 
-	public function testForSomeReasonPhpSegfaultsIfThereIsOneMethodLess() {
-		$this->assertTrue( (bool)'This is fucking weird' );
-	}
-
 	/**
 	 * @see ParserHookTest::processingProvider
 	 * @since 3.0
@@ -62,14 +58,14 @@ public function processingProvider() {
 		$argLists = [];
 
 		$values = [
-			'locations' => '4,2',
+			'locations' => '4,2; New York City; 13,37',
 			'width' => '420',
 			'height' => '420',
 			'service' => 'openlayers',
 		];
 
 		$expected = [
-			'coordinates' => [ new Location( new LatLongValue( 4, 2 ) ) ],
+			'coordinates' => [ '4,2', 'New York City', '13,37' ],
 			'width' => '420px',
 			'height' => '420px',
 			'mappingservice' => 'openlayers',