- Issue created by @tklawsuc
- 🇮🇹Italy itamair
thanks for reporting all this @tklawsuc
BUT outcomes of my QA and tests reproducing your use cases say that all this happen on singular cases (Issue 1)
and strangely depending on the Geocoder Provider that you use.
Issue 2 happens only with Nominatim ... and the address line 2 doesn't compromises correct Geocoding with others, such as Photon or Google Maps.The best approach that I would propose is contained into the attached patch,
that I feel could also benefit the related issue: https://www.drupal.org/project/geocoder/issues/3335292 🐛 Incorrect geocoding when suite number included in address Postponed: needs infoA new "geocoder_address_values_alter" hook (in the "geocoder_address" submodule) that might allow any custom module to change/alter the Address Values array (at your specific use case) before it is stringified by the AddressService: addressArrayToGeoString.
That can also do something like the following, to remove the "address_line2" value:
/** * Implements hook_geocoder_address_values_alter(). */ function [custom_module]_geocoder_address_values_alter(array &$values) { $values["address_line2"] = ''; }
Please Qa and test this attached patch.
I am pretty convinced to commit and add that into a new incoming Drupal geocoder 8.x-4.28 release ... - 🇨🇦Canada tklawsuc
@itamair this is a great solution to address the second issue. However it does not solve the first issue which we encounter more than the second. What if we pass additional contexts to the hook like newline_separator and linebreak_separator?
- 🇮🇹Italy itamair
ah ok ... better got it.
It is CommerceGuys\Addressing DefaultFormatter adding that "\n"
but it is the Geocoder AddressService here:
https://git.drupalcode.org/project/geocoder/-/blob/8.x-4.x/modules/geoco...
that is converting it into a space ... rather than something more clear separator for the Geocoder provider, such comma ...
right?Thus what about replacing both the "\n" and the "
" eventually into a comma instead (",")??
You propose ...Ok. I attache here an updated patch that adds these new replacements (in place of a space).
Could you carefully and extensively test that and eventually confirm this would improve the Geocoder AddressService on a general basis, in your feelings?
I think/assume that a comma (,) separator between different Address parts should be more clear with every Geocoder provider, isn't it?Let me know your outcomes with this new patch, please.
- 🇨🇦Canada tklawsuc
Unless there is a reason to use space instead of comma, I would make comma (and space) as the default for both separators as that will provide a clear separation between addessline and city. Some providers can't handle certain address without the comma before the city (as per my example
3615 S 31st St Temple, TX 76502 US
will not produce results on https://www.openstreetmap.org but if you use the comma it works). - 🇮🇹Italy itamair
thus ... could you QA and test the last attached patch and eventually mark this as RTBC eventually ... to approve (green flag) its commit into dev?
- 🇮🇹Italy itamair
Patch #4 committed into dev branch, will be part of next Geocoder release.
Automatically closed - issue fixed for 2 weeks with no activity.