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

Add support for waypoints and routes #3

Merged
merged 5 commits into from
Apr 12, 2022

Conversation

s3cur3
Copy link
Contributor

@s3cur3 s3cur3 commented Apr 12, 2022

I've tried to mirror the existing structure here (adding a field to the Gpx struct, and new structs for each of the types).

I'm using it at work and it's handled all the test data we've thrown at it so far. 😄

Comment on lines +88 to +96
defp optional_string(xml, element_type) do
el = xpath(xml, ~x"./#{element_type}")

defp optinal_string(maybe_string) when maybe_string == "", do: nil
defp optinal_string(maybe_string), do: maybe_string
if is_nil(el) do
nil
else
xpath(el, ~x"./text()"s)
end
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was getting an exception thrown when trying to query ./whatever/text() when the element in question didn't exist, so I moved to checking for the element, then asking for its text.

{:sweet_xml, "~> 0.6.0"}
{:sweet_xml, "~> 0.7.0"}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this library's purposes, I think 0.6.6 that we were using previously is actually compatible with 0.7.3, so we could potentially specify a broader supported range here, albeit at the cost of additional testing. 0.6.x is incompatible with a lot of libraries in the wild, including ExAws.

@caspg
Copy link
Collaborator

caspg commented Apr 12, 2022

Nice one. Thanks for this PR.

@caspg caspg merged commit 7b5c744 into velomapa:master Apr 12, 2022
@s3cur3
Copy link
Contributor Author

s3cur3 commented Apr 12, 2022

My pleasure, and thank you! 🙏

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.

2 participants