georust / gpx

Rust read/write support for GPS Exchange Format (GPX)
https://crates.io/crates/gpx
MIT License
102 stars 46 forks source link

Small Parser Improvements #24

Closed JDemler closed 4 years ago

JDemler commented 5 years ago

The parser failed to parse gpx files from gpsies.com. With these changes it is now possible to parse

I also added a testcase.

brendanashworth commented 5 years ago

@JDemler thanks for putting this together! I believe the purpose of your test case was to show it no longer errors, but I'd love it if we can also demonstrate via test case that the data is actually pulled into memory. Besides that this looks great and I can bring this in. 👍

JDemler commented 5 years ago

Added the requested testcase and fixed the formatting issues.

Another problem I ran over: currently the description in the metadata is parsed and written as tag with name description.

Both the 1.1 and 1.0 schema use desc though. I fixed the parser part in a branch. But as this is a breaking change I am not sure if it should be included in this PR. Maybe a seperate one?

brendanashworth commented 4 years ago

@JDemler thank you for putting this pull request together, sorry for taking forever to review it. I've merged it in https://github.com/georust/gpx/commit/5c484e972df9f539ff0ff23bb3cad4e5f47b2dee and will be releasing shortly.

The description to desc would be a breaking change, that might have been my mistake. Would you like to submit another PR for it? I think it can be released as a breaking change, say 1.0.0.

JDemler commented 4 years ago

Hey @brendanashworth , you can find the necessary changes for the description issue in https://github.com/CurrySoftware/gpx/tree/parser (the last 2 commits.)

It's only a 4-line change. Feel free to copy it :)