Repository: googlefonts/glyphsLib
Pull request #932 (2023-07-29 12:51:50)

Accept `.glyphspackage` in GSFont constructor
Remove the file extension check, which should not be our concern, and use `parser.load()` which handles `.glyphspackage` for us.


Patch:
diff --git a/Lib/glyphsLib/classes.py b/Lib/glyphsLib/classes.py
index 643b2fed..8773ee65 100755
--- a/Lib/glyphsLib/classes.py
+++ b/Lib/glyphsLib/classes.py
@@ -23,8 +23,6 @@ from collections import OrderedDict
 from enum import IntEnum
 from io import StringIO
 
-import openstep_plist
-
 # renamed to avoid shadowing glyphsLib.types.Transform imported further below
 from fontTools.misc.transform import Identity, Transform as Affine
 from fontTools.pens.basePen import AbstractPen
@@ -34,7 +32,7 @@ from fontTools.pens.pointPen import (
     SegmentToPointPen,
 )
 
-from glyphsLib.parser import Parser
+from glyphsLib.parser import load, Parser
 from glyphsLib.pens import LayerPointPen
 from glyphsLib.types import (
     IndexPath,
@@ -4518,14 +4516,7 @@ class GSFont(GSBase):
 
         if path:
             path = os.fsdecode(os.fspath(path))
-            assert os.path.splitext(path)[-1] == ".glyphs", (
-                "Please supply a file path to a .glyphs file",
-            )
-
-            with open(path, "r", encoding="utf-8") as fp:
-                logger.info('Parsing "%s" file into <GSFont>', path)
-                p = Parser()
-                p.parse_into_object(self, openstep_plist.load(fp, use_numbers=True))
+            load(path, self)
             self.filepath = path
             for master in self.masters:
                 master.font = self
diff --git a/Lib/glyphsLib/parser.py b/Lib/glyphsLib/parser.py
index 60f9ccd4..85655443 100644
--- a/Lib/glyphsLib/parser.py
+++ b/Lib/glyphsLib/parser.py
@@ -131,15 +131,16 @@ def load_glyphspackage(package_dir):
     return data
 
 
-def load(file_or_path):
+def load(file_or_path, font=None):
     """Read a .glyphs file. 'file_or_path' should be a (readable) file
     object, a file name, or in the case of a .glyphspackage file, a
-    directory name.
-    Return a GSFont object.
+    directory name. 'font' is an existing object to parse into, or None.
+    Return a 'font' or a GSFont object.
     """
-    p = Parser(current_type=glyphsLib.classes.GSFont)
     logger.info("Parsing .glyphs file")
-    res = glyphsLib.classes.GSFont()
+    if font is None:
+        font = glyphsLib.classes.GSFont()
+    p = Parser(current_type=font.__class__)
     if hasattr(file_or_path, "read"):
         data = openstep_plist.load(file_or_path, use_numbers=True)
     elif os.path.isdir(file_or_path):
@@ -147,8 +148,8 @@ def load(file_or_path):
     else:
         fp = open(file_or_path, "r", encoding="utf-8")
         data = openstep_plist.load(fp, use_numbers=True)
-    p.parse_into_object(res, data)
-    return res
+    p.parse_into_object(font, data)
+    return font
 
 
 def loads(s):


Test patch:
diff --git a/tests/glyphs3_test.py b/tests/glyphs3_test.py
index 78fa8818..28917ceb 100644
--- a/tests/glyphs3_test.py
+++ b/tests/glyphs3_test.py
@@ -19,11 +19,7 @@ def test_glyphs3_italic_angle(datadir):
 
 
 def test_glyphspackage_load(datadir):
-    font1 = glyphsLib.load(str(datadir.join("GlyphsUnitTestSans3.glyphs")))
-    font1.DisplayStrings = ""  # glyphspackages, rather sensibly, don't store user state
-    font2 = glyphsLib.load(str(datadir.join("GlyphsUnitTestSans3.glyphspackage")))
-    names = [glyph.name for glyph in font2.glyphs]
-    assert names == [
+    expected = [
         "A",
         "Adieresis",
         "a",
@@ -36,6 +32,16 @@ def test_glyphspackage_load(datadir):
         "_part.shoulder",
         "_part.stem",  # Deliberately removed from glyph order file
     ]
+    font1 = glyphsLib.load(str(datadir.join("GlyphsUnitTestSans3.glyphs")))
+    font1.DisplayStrings = ""  # glyphspackages, rather sensibly, don't store user state
+    font2 = glyphsLib.load(str(datadir.join("GlyphsUnitTestSans3.glyphspackage")))
+    assert [glyph.name for glyph in font2.glyphs] == expected
+    assert glyphsLib.dumps(font1) == glyphsLib.dumps(font2)
+
+    font1 = glyphsLib.load(str(datadir.join("GlyphsUnitTestSans3.glyphs")))
+    font1.DisplayStrings = ""  # glyphspackages, rather sensibly, don't store user state
+    font2 = GSFont(str(datadir.join("GlyphsUnitTestSans3.glyphspackage")))
+    assert [glyph.name for glyph in font2.glyphs] == expected
     assert glyphsLib.dumps(font1) == glyphsLib.dumps(font2)