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

Support all string properties of wptType tags #13

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

Conversation

joukewitteveen
Copy link

Waypoints (wpt), route points (rtept), and track points (trkpt) share the same type (wptType) in the GPX schema. It makes sense to centralize the code that generates these tags.

Doing so makes it possible to implement more of the supported children. I implemented support for all string based child tags. This is especially nice in conjunction with mapbox/togeojson, which reads those tags.

@joukewitteveen joukewitteveen force-pushed the master branch 2 times, most recently from 3713bda to c00bae2 Compare October 19, 2018 09:22
@joukewitteveen
Copy link
Author

I extended this PR with support for links. With this, most of a waypoint survives a pass through togeojson followed by togpx!

I also included a fix for an older typo. This should have been a separate PR, but given the low activity on this project I decided to just stick it in here.

@joukewitteveen
Copy link
Author

Any ideas on this PR?

Note that it is meant to be independent of #11. While this PR adds support for more waypoint properties, it does not expose any transformers. It also does nothing to make togpx output GPX routes. In that sense, this PR is far less ambitious. When those features are desired, #11 can just as well be applied after this one. It would need to be rebased, though.

tags: { name: "name" },
name: "not_name"
tags: { id: "name" },
ref: "not_name"
Copy link
Author

Choose a reason for hiding this comment

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

This reflects a change in property precedence when configuring a name.
Before this pull request, the options were, in order of decreasing precedence:

  1. inferred from tags (tags.name, tags.id, tags.ref)
  2. inferred from bare properties (name, id, ref)

After this pull request, the options would be, in order of decreasing precedence:

  1. explicit in a bare property (name)
  2. inferred from tags (tags.name, tags.id, tags.ref)
  3. inferred from bare properties (name, id, ref)

Note that the occurrence of name in the third option is superfluous.

Because of this change in precedence, I suggest that the next version would be 0.6.0, if this PR is accepted.

@joukewitteveen
Copy link
Author

Ping?

Support all string properties of the GPX wptType type. For the name
and description tag, the user callback is used as a fallback. The
callbacks are always used for tags that are not of the wptType type.
GPX elements may contain multiple links, each potentially containing a
text element and a type element. Take these from a "links" array in
the properties of a feature if present and fallback to the featureLink
callback otherwise.
@joukewitteveen joukewitteveen force-pushed the master branch 2 times, most recently from e5c43e6 to 05c12e9 Compare January 4, 2019 20:26
@joukewitteveen
Copy link
Author

I force-pushed a new version that does away with the gpx_str variable (in the "streamlining" commit). This variable is currently leaked into the global scope.

@joukewitteveen
Copy link
Author

Ping?

@joukewitteveen
Copy link
Author

Ping? I would still like to see this improved support for a round-trip via togeojson added.

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