-
Notifications
You must be signed in to change notification settings - Fork 19
should glifLib ignore glyph "name" attribute, and use the one from contents.plist instead? #34
Comments
Pity that this answer from my previous self didn't get any reply. I think this is still an issue, or a discrepancy between the wording of the UFO spec and its reference implementation (ufoLib). |
I checked again the GLIF spec for all UFO formats from 1 to 3 and they all say that name attribute should be ignored when parsing GLIF
I just checked the defcon code, and I can see that Layer.loadGlyph is setting the name attribute immediately before calling GlyphSet.readGlyph method. I think in fontTools.ufoLib we should change GlyphSet.readGlyph so that it no longer reads/sets the name attribute on the glyph object. The reason I'm insisting on this is that, in fontTools.ufoLib, I would like to make the name attribute of Glyph objects read-only, ie. it should only be possible to set once it in the Glyph(name="hello") constructor, and then access it via a read-only 'name' property, however attempting to modify it would raise AttributeError. The rationale is that the parent object (Layer) is the one that contains the mapping between names and respective Glyph objects; making the name attribute read/write means that each Glyph objects must maintain a (circular) reference to their parent Layer, which I really wish to avoid. |
What you proposed seems fine to me. I want to drop the |
actually, since the UFOReader uses a |
Maybe we should change it just for clarity in the future? Whatever you think is best. I'm forgetful so I like to leave breadcrumbs. |
The UFO Specification says that:
However, in
glifLib
we do use thename
attribute of theglyph
element, and set theglyphObject.name
with it:https://github.com/unified-font-object/ufoLib/blob/master/Lib/ufoLib/glifLib.py#L972-L977
Shouldn't it be ignored, and the glyph name from the contents.plist be used instead to avoid mismatches?
The text was updated successfully, but these errors were encountered: