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

[review] Add support for roundtripping Glyphs/UFO custom name table entries. #827

Merged

Conversation

bramstein
Copy link
Contributor

We have some Glyphs source files with custom name table entries that weren't preserved when converting to UFO. This PR parses the Glyphs <nameid> <platform_id>? <encoding_id>? <language_id>?; <string> format and creates UFO openTypeNameRecords (and vice versa).

I added a new test that covers all the different combinations the Glyphs name table entry format supports. In the case the Glyphs name table entry only specifies a name identifier and string default values are used for the other parameters. The default parameters are not removed when converting from UFO -> Glyphs, so roundtripping a Glyphs will not be literally identical but functionally identical.

This is a partial fix for #633.

@anthrotype
Copy link
Member

Thanks for the PR! Can you please rebase on main branch (or merge origin/main into yours), that should fix the setup error you're seeing in the "lint" job.

@bramstein bramstein force-pushed the bs-name-table-entries branch from e4d7a31 to ccc662f Compare November 7, 2022 12:12
Copy link
Member

@anthrotype anthrotype left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

tests/builder/custom_params_test.py Show resolved Hide resolved
@anthrotype
Copy link
Member

regression tests took ages, I'm not sure what's going on there. But if anything related to this PR has changed that made them fail, then it's for good and they should automatically fix themselves after merging the PR to main. If not, we'll take it from there.
Thanks!

@anthrotype anthrotype merged commit b116f98 into googlefonts:main Nov 7, 2022
@IvanUkhov IvanUkhov deleted the bs-name-table-entries branch February 25, 2023 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants