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

Why do we include space, nb space and some control chars in extended subsets? #14

Closed
m4rc1e opened this issue Apr 12, 2024 · 5 comments · Fixed by #15
Closed

Why do we include space, nb space and some control chars in extended subsets? #14

m4rc1e opened this issue Apr 12, 2024 · 5 comments · Fixed by #15

Comments

@m4rc1e
Copy link
Contributor

m4rc1e commented Apr 12, 2024

These character are already covered in the non-extended subsets so I don't see the point in duplicating them?

@m4rc1e
Copy link
Contributor Author

m4rc1e commented Apr 25, 2024

@garretrieger would you be opposed to me removing the following glyphs from every extended subset, https://github.com/googlefonts/nam-files/blob/main/Lib/gfsubsets/data/greek-ext_unique-glyphs.nam#L5-L8.

My issue is that gftools add-font will include the greek-ext subset for all families because our threshold for including an extended subset is only requiring 1% of characters. greek-ext only has 241 chars so 4/241=0.0165%. If I remove these glyphs, it won't trigger. If we can't remove these glyphs, I guess I could discount them from the set in gftools add-font.

@garretrieger
Copy link
Contributor

Removing codepoints from a subset that have been included in pushed fonts can be risky, so if the only problem these are causing is with the tools then I'd prefer we try updating the tools first before resorting to subset changes.

@garretrieger
Copy link
Contributor

I think it would be reasonable to just ignore those four characters for the purpose of subset selection as they occur frequently in various subsets and don't really signal any specific script.

@vv-monsalve
Copy link

vv-monsalve commented May 9, 2024

My issue is that gftools add-font will include the greek-ext subset for all families

I think this is also the case for cyrillic-ext see gftools#829

Edit: At a revision of the Latin-ext_unique-glyphs.nam, we do want it on standard fonts as it covers what is now required under Latin Core. However, the central question about whether characters like space or nb space should be repeated on this subset remains valid.

@m4rc1e
Copy link
Contributor Author

m4rc1e commented May 9, 2024

I'll make a fix for this today. Plan is to simply make the mod to the tools.

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 a pull request may close this issue.

3 participants