Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set optional parameters of http_build_query #28

Closed
wants to merge 1 commit into from

Conversation

sebalis
Copy link
Contributor

@sebalis sebalis commented Jul 14, 2021

If arg_separator is not set, the default value comes from arg_separator.output which is not always set to the value we need here, which is '&'. On Ubuntu the default is '&'. There is no need to adhere to any system defaults here because we know exactly in which context the URLs we build are used: for a backend request that is executed soon in the same PHP handler. The same parameters are used in the MapQuest provider.

If arg_separator is not set, the default value comes from arg_separator.output which is not always set to the value we need here, which is '&'. On Ubuntu the default is '&'. There is no need to adhere to any system defaults here because we know exactly in which context the URLs we build are used: for a backend request that is executed soon in the same PHP handler. The same parameters are used in the MapQuest provider.
@eileenmcnaughton
Copy link
Owner

@sebalis - Nominatim is a package that we include from https://github.com/geocoder-php/nominatim-provider - can you try submitting your fix to the upstream repo so we can update using composer?

@sebalis
Copy link
Contributor Author

sebalis commented Jul 14, 2021

Sure, I’ll see what I can do.

@sebalis
Copy link
Contributor Author

sebalis commented Jul 14, 2021

Just noticed that in my extended descripion the Ubuntu setting got mangled – the default there is &‌amp; and I hope it makes more sense now :-)

@sebalis
Copy link
Contributor Author

sebalis commented Jul 14, 2021

A bot there refers me to http://github.com/geocoder-php/Geocoder – is this going to be an endless cycle? :-)

@sebalis
Copy link
Contributor Author

sebalis commented Jul 14, 2021

Now at geocoder-php/Geocoder#1131 waiting for workflows to finish.

@sebalis
Copy link
Contributor Author

sebalis commented Jul 14, 2021

Do you need/want me to close this PR now, @eileenmcnaughton​?

@eileenmcnaughton
Copy link
Owner

@sebalis yeah maybe close this & open an issue to composer update once they merge it?

@sebalis
Copy link
Contributor Author

sebalis commented Jul 15, 2021

Of course, I’ll let you know. Thanks for now.

@sebalis sebalis closed this Jul 15, 2021
eileenmcnaughton added a commit that referenced this pull request Aug 23, 2021
@eileenmcnaughton eileenmcnaughton mentioned this pull request Aug 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants