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

use querystring rather than mustache templates; support extra attributes in custom providers #17

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

andrewharvey
Copy link

@andrewharvey andrewharvey commented Sep 6, 2017

Hi @veltman. Thanks for building this repository. I've made two changes which I wanted to share, but I don't think they are ready to be merged. So please feel free to close this PR or add your comments on if you think the intent is a good direction and if it warrants further work, or if you're not keen on integrating these changes (that's absolutely fine).

  1. The first commit here uses querystring rather than mustache templates. I ran into a bug where if the mustache variable was in the query string and contained a ' (apostrophe) the URL wouldn't be created correctly based on the template. Using the querystring module fixed that for me. But adds significant complexity to the user interface. ~~~It also drops support for concatenating columns in the URL. (fixed in latest commit)~~~

  2. The second is for custom providers to return more than just lat, lng, such that any extra attributes returned are simply added to the output CSV.

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.

1 participant