-
Notifications
You must be signed in to change notification settings - Fork 737
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Support for fonts with multiple descriptors #156
Support for fonts with multiple descriptors #156
Conversation
9cf9545
to
9a822c7
Compare
Perfect, nice one 👌 Could you just please add an entry in the CHANGELOG.md to mention that change and credit yourself? Thanks! |
Done. Thanks! Looks like there's now a conflict though. Do you prefer that I rebase my feature branch or make a merge commit, or will you make the merge on your side when pulling in the PR? |
If you can rebase your PR on top of |
248ae29
to
03c9fa7
Compare
Mmmmh idk what happened with Travis, it seems your rebase didn't properly trigger a new build like it should've. Maybe pushing a dummy new commit might help re-triggering the travis hook?! |
@@ -35,16 +35,18 @@ public func == (lhs: Font, rhs: Font) -> Bool { | |||
// MARK: CTFont | |||
|
|||
extension CTFont { | |||
static func parseFontInfo(fileURL: NSURL) -> Font? { | |||
static func parseFontInfo(fileURL: NSURL) -> [Font]? { |
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.
Now that this method returns an array and not a single font, maybe we could return a non-optional array instead (and return an empty array on any guard
failure)?
Less code, I like it! And so does Travis 👍 |
🎉 |
I ran into an issue where I tried to use the font "Avenir" which contains descriptors for "Light", "Medium", "Heavy", etc., and the output from
swiftgen
only contained the descriptor for "Book."The intent of this change is to support all of the descriptors from a given font, rather than just the first.