Repository: googlefonts/glyphsLib
Pull request #886 (2023-03-22 09:40:22)

Load all .glyphspackage glyphs
In https://github.com/googlefonts/glyphsLib/issues/846#issuecomment-1478023500, Georg said

> But was in really intended to load the .glif file based on the name? I thought that you should read all files and map them based off the name in the file?

This PR does that: loads all files in the `glyphspackage` directory and then reorders them based on the order in `order.plist`.


Patch:
diff --git a/Lib/glyphsLib/parser.py b/Lib/glyphsLib/parser.py
index e1c7e490..60f9ccd4 100644
--- a/Lib/glyphsLib/parser.py
+++ b/Lib/glyphsLib/parser.py
@@ -14,7 +14,7 @@
 
 
 from collections import OrderedDict
-from fontTools.misc.filenames import userNameToFileName
+from pathlib import Path
 import glyphsLib
 import logging
 import openstep_plist
@@ -101,17 +101,32 @@ class Parser:
 
 
 def load_glyphspackage(package_dir):
-    infofile = os.path.join(package_dir, "fontinfo.plist")
-    orderfile = os.path.join(package_dir, "order.plist")
-    glyphorder = openstep_plist.load(open(orderfile, "r", encoding="utf-8"))
-    data = openstep_plist.load(open(infofile, "r", encoding="utf-8"), use_numbers=True)
+    package = Path(package_dir)
+    infofile = package / "fontinfo.plist"
+    orderfile = package / "order.plist"
+
+    glyphorder = []
+    if orderfile.exists():
+        with open(orderfile, "r", encoding="utf-8") as order_fh:
+            glyphorder = openstep_plist.load(order_fh)
+
+    with open(infofile, "r", encoding="utf-8") as info_fh:
+        data = openstep_plist.load(info_fh, use_numbers=True)
     data["glyphs"] = []
-    for g in glyphorder:
-        glyphname = userNameToFileName(g) + ".glyph"
-        glyphfile = os.path.join(package_dir, "glyphs", glyphname)
-        data["glyphs"].append(
-            openstep_plist.load(open(glyphfile, "r"), use_numbers=True)
-        )
+    for glyphfile in (package / "glyphs").glob("*.glyph"):
+        with open(glyphfile, "r") as fh:
+            glyph = openstep_plist.load(fh, use_numbers=True)
+        data["glyphs"].append(glyph)
+    # Sort according to glyphorder
+
+    def sort_key(glyph):
+        glyphname = glyph["glyphname"]
+        if glyphname in glyphorder:
+            return (0, glyphorder.index(glyphname))
+        else:
+            return (1, glyphname)
+
+    data["glyphs"] = list(sorted(data["glyphs"], key=sort_key))
 
     return data
 


Test patch:
diff --git a/tests/data/GlyphsUnitTestSans3.glyphspackage/order.plist b/tests/data/GlyphsUnitTestSans3.glyphspackage/order.plist
index 4023bd54..c2ae3b7f 100644
--- a/tests/data/GlyphsUnitTestSans3.glyphspackage/order.plist
+++ b/tests/data/GlyphsUnitTestSans3.glyphspackage/order.plist
@@ -8,6 +8,5 @@ m,
 n,
 a.sc,
 dieresis,
-_part.shoulder,
-_part.stem
-)
\ No newline at end of file
+_part.shoulder
+)
diff --git a/tests/glyphs3_test.py b/tests/glyphs3_test.py
index cbb167f8..6336bfa7 100644
--- a/tests/glyphs3_test.py
+++ b/tests/glyphs3_test.py
@@ -22,6 +22,20 @@ 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 == [
+        "A",
+        "Adieresis",
+        "a",
+        "adieresis",
+        "h",
+        "m",
+        "n",
+        "a.sc",
+        "dieresis",
+        "_part.shoulder",
+        "_part.stem",  # Deliberately removed from glyph order file
+    ]
     assert glyphsLib.dumps(font1) == glyphsLib.dumps(font2)