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

[FlattenComponents] Don't throw weird errors when components are missing #669

Merged
merged 3 commits into from Oct 21, 2022

Conversation

simoncozens
Copy link
Contributor

If I use fontmake on a Glyphs file which contains components with missing base glyphs, I get errors like this:

INFO:ufo2ft.filters:Running FlattenComponentsFilter on GoogleSans_Persian-Regular
fontmake: Error: In 'master_ufo/XXX-Regular.designspace': Generating fonts from Designspace failed: 'flig2'

(Side note: there are many cases where fontmake eats the original exception like this, and produces weird and inscrutable errors. We should hunt down all of them.)

With this PR, the font compiles with warnings:

WARNING:ufo2ft.filters.flattenComponents:Could not find component 'flig2' used in 'f_f'

@simoncozens
Copy link
Contributor Author

simoncozens commented Oct 17, 2022

A second side note is that this happens even in glyphs which are set to non-exporting, which shouldn't happen anyway.

@anthrotype
Copy link
Member

Are we sure a WARNING is better than a (probably better phrased) error in this case? A missing component is probably bad and should be caught at build time. Warnings can be ignored.

this happens even in glyphs which are set to non-exporting, which shouldn't happen anyway.

this should be investigated further, because I think preprocessor already takes care of excluding the non-exported glyphs from the glyph set that is passed to the filters, so FlattenComponentsFilter should not even see those..

@simoncozens
Copy link
Contributor Author

Nope, I'm wrong - I was changing the exported glyphs of a Glyphs file but compiling the intermediate UFO. The glyphset issue is fine, so I'm happy to make this a hard error.

"""Returns a list of tuples (baseGlyph, transform) of nested component."""
if component.baseGlyph not in glyphSet:
raise ValueError(
"Could not find component '%s' used in '%s'"
Copy link
Member

Choose a reason for hiding this comment

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

nit: use f"" string interpolation instead old-style %

@simoncozens simoncozens merged commit 71ef6ca into main Oct 21, 2022
@khaledhosny khaledhosny deleted the flatten-component-error branch November 18, 2022 13:45
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.

None yet

2 participants