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

Waypoint name can be an empty string #70

Closed
cedeber opened this issue May 21, 2022 · 3 comments · Fixed by #71
Closed

Waypoint name can be an empty string #70

cedeber opened this issue May 21, 2022 · 3 comments · Fixed by #71

Comments

@cedeber
Copy link
Contributor

cedeber commented May 21, 2022

Hello.

I found out that I have a GPX file with a waypoint name which is empty:

<wpt lat="47.940954" lon="7.158881">
  <ele>0</ele>
  <name><![CDATA[]]></name>
  <desc><![CDATA[]]></desc>
  <sym>3</sym>
</wpt>

Looking at the GPX spec: http://www.topografix.com/GPX/1/1/#type_wptType this seems to be valid.

GPX does not place restrictions on the length of this field or the characters contained in it.

But this crate makes it mandatory (allow_empty: false here):

"name" => waypoint.name = Some(string::consume(context, "name", false)?),

I believe we should allow it empty, don't you think?
Thanks

@lnicola
Copy link
Member

lnicola commented May 21, 2022

Sounds good, do you want to file a PR?

@cedeber
Copy link
Contributor Author

cedeber commented May 21, 2022

Here we go. I didn't find any test specific for the waypoint. Do you already have something or do I add one?

@lnicola
Copy link
Member

lnicola commented May 21, 2022

I'm not sure, I'm not from around 😅.

bors bot added a commit that referenced this issue May 27, 2022
71: Waypoint name can be an empty string r=urschrei a=cedeber

Fixes #70




Co-authored-by: Cédric Eberhardt <hello+code@cedeber.fr>
Co-authored-by: Stephan Hügel <shugel@tcd.ie>
bors bot added a commit that referenced this issue May 27, 2022
71: Waypoint name can be an empty string r=urschrei a=cedeber

Fixes #70




Co-authored-by: Cédric Eberhardt <hello+code@cedeber.fr>
Co-authored-by: Stephan Hügel <shugel@tcd.ie>
@bors bors bot closed this as completed in 205b893 May 27, 2022
bors bot added a commit that referenced this issue May 27, 2022
72: Allow empty elevation tags r=urschrei a=ebcrowder

Similar to #70, I have a GPX file that has self-closing `elevation` tags, which of course, contain no value. This appears to be valid per the GPX spec at https://www.topografix.com/GPX/1/1/#type_ptType.

The GPX file was generated by a Wahoo bike computer.

This change edits the parsing logic to use a default `f64` value of `0.00` for these cases. I didn't see a precedence for handling similar optional values in this lib, so let me know if I should handle this differently in accordance with the project's goals. 

- [x] I agree to follow the project's [code of conduct](https://github.com/georust/geo/blob/master/CODE_OF_CONDUCT.md).
- [x] I added an entry to `CHANGELOG.md` if knowledge of this change could be valuable to users.
---



Co-authored-by: Eric Crowder <eric@ebcrowder.dev>
bors bot added a commit that referenced this issue May 27, 2022
72: Allow empty elevation tags r=michaelkirk a=ebcrowder

Similar to #70, I have a GPX file that has self-closing `elevation` tags, which of course, contain no value. This appears to be valid per the GPX spec at https://www.topografix.com/GPX/1/1/#type_ptType.

The GPX file was generated by a Wahoo bike computer.

This change edits the parsing logic to use a default `f64` value of `0.00` for these cases. I didn't see a precedence for handling similar optional values in this lib, so let me know if I should handle this differently in accordance with the project's goals. 

- [x] I agree to follow the project's [code of conduct](https://github.com/georust/geo/blob/master/CODE_OF_CONDUCT.md).
- [x] I added an entry to `CHANGELOG.md` if knowledge of this change could be valuable to users.
---



Co-authored-by: Eric Crowder <eric@ebcrowder.dev>
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 a pull request may close this issue.

2 participants