Skip to content
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

Upconvert UFO v1 and v2 fontinfo and feature data #38

Merged
merged 5 commits into from
Jul 2, 2020
Merged

Conversation

madig
Copy link
Collaborator

@madig madig commented Apr 5, 2020

This does a bit of work.

ufoLib supports converting the versions up and down, but I think forcibly one-way upgrading the data to the latest supported version is the way to go for Norad.

The fontinfo error messages remain... rudimentary. I think they need to be revisited to return more useful error messages at a later point. I don't know how to get serde to include the field name in error messages if a field contains invalid data.

@madig madig force-pushed the fontinfo-v1 branch 2 times, most recently from bb0e216 to aa986c4 Compare May 3, 2020 21:07
@madig madig marked this pull request as ready for review June 14, 2020 22:00
Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

looks good, one little nit but overall I'm happy to trust you ;)

src/ufo.rs Outdated Show resolved Hide resolved
@madig
Copy link
Collaborator Author

madig commented Jul 1, 2020

This works, but strikes me as somehow unrusty.

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

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

Okay, one little suggestion but feel free to merge!

src/ufo.rs Show resolved Hide resolved
@madig madig merged commit 8330af3 into master Jul 2, 2020
@madig madig deleted the fontinfo-v1 branch July 2, 2020 21:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants