-
Notifications
You must be signed in to change notification settings - Fork 12
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 UfoDataRequest #53
Conversation
I don't understand what this does from the PR text or the code :O please elaborate. |
@madig I don't understand why you don't understand 😓 If you call If you call my above example, it will only parse the font's |
Aha! Now I understand the purpose, though I still don't know how the code above arrives at the fontinfo.plist data. It will be some time until I can study this. Maybe @cmyr has a quick take... |
Right here: Line 181 in 99b565e
This part has not changed. All I did was not fetch fields of the |
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 think this is a reasonable addition? We may want to revisit at some point in the future, but this seems pretty straight-forward.
I have some naming and API suggestions, but overall I have no problem with this being added. Thanks!
This suddenly reminds me of |
04423ba
to
e5d66e1
Compare
Switch to #[non_exhaustive] Make UfoDataRequest Copy Builder pattern for UfoDataRequest Better documentation for UfoDataRequest Co-authored-by: Colin Rofls (@cmyr) UfoDataRequest→DataRequest Add tests
e5d66e1
to
a2a4f61
Compare
Switch to #[non_exhaustive] Make UfoDataRequest Copy Builder pattern for UfoDataRequest Better documentation for UfoDataRequest Co-authored-by: Colin Rofls (@cmyr) UfoDataRequest→DataRequest Add tests
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.
Okay a few more little notes but this is basically good to go I think.
@ctrlcctrlv what's the status of this? It felt like we were very close to merging, would still be happy to get it in... |
No worries, happy to take a look whenever it feels ready to you :) |
I'd say it's ready. The failing test isn't my doing, the service seems down. |
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.
Thanks!
This makes it so I can do this in MFEQ's Qmetadata:
That MFEQ module will never need all the glyph and kerning parsing this tool does; so all it does is slow things down substantially, especially with the 125MB UFO for TT2020 Style F. I know Rust maintainers are rather picky, so if you don't like this idea, just let me know, I'm happy to maintain my own Norad version, I already maintain a personal
.glif
parser because I need Skia-friendly points.