search

geocoder-php / Geocoder

The most featured Geocoder library written in PHP.
https://geocoder-php.org
MIT License
3.94k stars 517 forks source link

Add name for PhotonAddress #1106

Closed Persaeus closed 3 years ago

Persaeus commented 3 years ago

This PR adds "name" for the photon provider.

Two new methods are added to PhotonAddress: getName() and withName($name)

jbelien commented 3 years ago

Thanks @nihilsen ! 👍

Could you add a new test for that field ?

Persaeus commented 3 years ago

Tests added.

It occurred to me that the Photon API doesn't always provide a "name" property that we can use as the display name.

That's not very useful, so in those cases I think it would be appropriate to construct a placeholder display name using the address components. This produces a display name similar to that given by the Nominatim provider.

jbelien commented 3 years ago

It occurred to me that the Photon API doesn't always provide a "name" property that we can use as the display name.

That's not very useful, so in those cases I think it would be appropriate to construct a placeholder display name using the address components. This produces a display name similar to that given by the Nominatim provider.

No, we do not "touch" the result returned by an API. If there is no name property fill, the name should stay null. Please revert commit 4b91de7 and add a test for a request that returns a name property.

Thanks.

Persaeus commented 3 years ago

No, we do not "touch" the result returned by an API.

Got it. That seems reasonable.

Persaeus commented 3 years ago

Instead of displayName, could you rename it to name, getName(), withName() ?

You got it.

Tests separated.

jbelien commented 3 years ago

Thanks @nihilsen !