-
Notifications
You must be signed in to change notification settings - Fork 50
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 Linux #55
Conversation
Sources/Polyline/CoreLocation.swift
Outdated
public let longitude: CLLocationDegrees | ||
} | ||
|
||
public struct CLLocation { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Polyline might not be the only library in a Linux application that needs to work with coordinates. If the application also installs Turf, both will have CLLocations and CLLocationCoordinate2Ds. It won’t result in compilation errors, but having two partial implementations of Core Location’s API can be confusing.
Instead, how about taking the “CL” prefix off these types and using them internally instead of Core Location types? For the public API, we can continue to provide Core Location–compatible methods (as long as Core Location can be imported), but behind the scenes they’d be pass-throughs to non–Core Location implementations. It’d be like the MapKit convenience methods.
Or perhaps we could define a protocol that both this class and the real CLLocation could conform to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Different concrete type internally would have an impact on the performance since it would have to be transformed yet another time. The protocol approach, perhaps with a couple of generic functions, sounds viable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took the simplest possible approach of making the public API use a uniquely named type that on Apple platforms is aliased to CLLocationCoordinate2D
but on Linux platforms is a concrete struct definition. This is similar to the workarounds often found when deploying backwards to an older version of iOS that lacks certain Apple APIs. It should have no performance implications. The only drawbacks I can think of are that users have to dig a little deeper to realize they can use certain methods with CLLocationCoordinate2D
, and that Linux clients will need to implement their own class to wrap CLLocationCoordinate2D
in the unlikely event that they need one.
Rewrote the Core Location compatibility shim for Linux with a single type, LocationCoordinate2D, that uses only standard library types and avoids any naming collision with Core Location types. Added a single type alias so that Apple platforms can use CLLocationCoordinate2D in place of the compatibility shim. Excluded anything involving CLLocation from Linux; clients can provide their own class to wrap LocationCoordinate2D if desired.
@@ -0,0 +1,12 @@ | |||
#!/bin/bash | |||
|
|||
if [[ $TRAVIS_OS_NAME == 'osx' ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Travis never seems to get here. Travis is effectively only building the library on Linux and never testing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in #66.
|
||
#if !os(watchOS) | ||
#if canImport(MapKit) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On watchOS, MapKit can be imported, but its symbols are marked as unavailable, triggering the following compiler error:
/path/to/Polyline/Sources/Polyline/Polyline.swift:67:28: 'MKPolyline' is unavailable in watchOS
/MapKit.MKPolyline:2:12: 'MKPolyline' has been explicitly marked unavailable here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in #67.
Added support for Linux
canImport
CLLocationCoordinate2D
withLocationCoordinate2D
, which is an alias forCLLocationCoordinate2D
on Apple platforms@1ec5 @tomtaylor