Closed iKenndac closed 9 months ago
Hi. Wow, impressive and interesting PR. I didn't know we could go with direct C bindings (it was quite some time ago).
Basically, I am very open to it, but I couldn't really help. I am running out of time almost everyday. I will keep following your work, and I'd be happy to merge once it is fully working.
Even if it is quite some past work to be removed, removing one layer is probably simpler / better in the long run.
I'd be curious to know for what application you intend to use SwiftAA on Linux.
@onekiloparsec No worries about not being able to help — I just wanted to check in and get your opinions on a couple of things, and make sure you'd be open to such a big change. It sounds like you're OK with the ObjCAA going away?
We're building an app called Photo Scout (https://photo-scout.app) that's a tool for photographers to be told when the conditions are right for a photo they want to take. There's a fair amount of astronomy maths that needs to be done for features like "Tell me when the sun is in this position in the sky" etc.
The Linux requirement comes from having a shared Swift "engine" that runs both in the app and in the backend (that's deployed on Linux) to power notifications. So far we've been finding calculations around the internet and porting them to Swift, but it's a bit haphazard and we're going to make a mistake at some point — so I figure it's a better use of our efforts to support a project like this.
Very interesting. I am happy if this library is useful to others, that's the whole point of OSS, I think. I originally created SwiftAA hoping it will help building a new version of my native application called iObserve. I never managed to develop this app, because I focused on the web version.
So it's great if it used by others.
I confirm I am OK to remove the ObjCAA layer. It was quite some work, but it has no real additional value, it only serves as a bridge.
Ok great, I'll continue tidying up and I'll ping you again when it's ready for review/merge.
@onekiloparsec Hi again! Sorry for the big delay.
I've performed some more work on this, and I think I'm very close to having this ready to merge. As of right now, I've:
Merged in the latest work from master
.
Fixed the build from every angle I can build it from: SPM on both Mac and Linux, CocoaPods (via pod lib lint
), and the Xcode project all build correctly now.
Fixed the build on watchOS and tvOS.
Removed the ObjCAA
target in favour of AABridge
, as discussed above. There's now a big hole for ObjCAA
to come back if it turns out someone relied on it.
Bumped the version to 3.0.0
. The Swift API is completely unchanged, but the removal of ObjCAA
technically makes this a breaking change, hence the major version bump.
If you're happy with this, I can update the README/docs to remove mentions of ObjCAA
and we can merge it in.
By the way,
Adds a platform-agnostic Color struct to use when AppKit or UIKit aren't available.
Maybe it would be a good idea to take this opportunity to rename the Color
struct/typealias. Not that it conflicts with anything but that it might cause confusion with SwiftUI's Color
. (In the case where someone uses SwiftAA in a SwiftUI-based project)
@vincentneo Since it's a typealias, it won't interfere. The struct is only defined when both UIKit and AppKit aren't available, and currently SwiftUI is very much tied to UIKit and AppKit. However, I changed it anyway.
As well as Color, SwiftAA declares a lot of types that interfere with other things (like Day
, Hour
, etc). There's a lot of import struct Time.Day
etc going on in my project already! Color has never been a problem though.
@onekiloparsec I've updated the README. I think this is ready for merge now.
Sorry for the delay. I can't test it right now, but it looks very good. Let's merge it.
@onekiloparsec Thank you for coming back to this, I appreciate it!
The app I was talking about above was launched in September and this branch is being used in production in both the iOS app and the Swift on the Server backend on Linux. It's been working great! https://photo-scout.app
I just had a look at your app/service. Really beautiful, we can see it's well crafted.
It is somewhat similar to what I am currently implementing myself for Arcsecond.io (but for a very different usage). :-)
This is an in-progress PR that allows SwiftAA to be used on Swift on Linux. The complexity here is with the ObjC bridge — and, unfortunately it seems that the only way to do this in a sane way is to get rid of it and use C bindings instead.
I'm opening this PR early to get some feedback on what you think. The removal of ObjCAA is the large breaking change. SwiftAA is unchanged other than some minor internal type name tweaks. AA+ is completely untouched.
Changes
Removes Objective-C++ bindings in favour of C ones. The main change here is that
KPCAADate
is implemented using the handle pattern rather than as anNSObject
subclass. All.mm
files have been renamed to.cpp
ones.Removes all imports of
Foundation.h
in the bridge C code.NSInteger
andNSUInteger
are redefined for sanity, butBOOL
is (somehow) more complicated, so all uses ofBOOL
have been changed tobool
.Guards some APIs (such as those requiring CoreLocation) behind appropriate checks. ObjC++
XCTestCase
subclasses too.Adds a platform-agnostic
Color
struct to use when AppKit or UIKit aren't available.Still To Do
Tidy up — some doc comments are now incorrect, etc.
Decide what to do with
ObjCAA
. Right now, it's still calledObjCAA
but it has no ObjC in it. It could be renamedCAA
or something, breaking the library for anyone that usesObjCAA
. Or, it could be renamed toAABridge
and exclusively be used by the Swift code, andObjCAA
can come back as it was for ObjC users.Tests
All tests continue to pass on macOS. On Linux, there are eight failures. However, the values are so close I think it's a side effect of the different platforms (but I'm not an expert).