Repository: googlefonts/glyphsLib
Pull request #863 (2023-02-23 13:31:17)

Reduce the ordering difference with respect to Glyphs
When using `glyphsLib` for manipulating Glyphs files, it produces a lot of unrelated changes due to its serialization order that is different from the one used by Glyphs. This PR is an attempt to reduce the different.

anthrotype: LGTM (pending tests fixups)

Patch:
diff --git a/Lib/glyphsLib/classes.py b/Lib/glyphsLib/classes.py
index 7cb13674..ba9c08d2 100755
--- a/Lib/glyphsLib/classes.py
+++ b/Lib/glyphsLib/classes.py
@@ -2567,10 +2567,10 @@ class GSComponent(GSBase):
         if self.smartComponentValues:
             writer.writeKeyValue("piece", self.smartComponentValues)
         if writer.format_version > 2:
+            writer.writeObjectKeyValue(self, "rotation", keyName="angle", default=0)
             writer.writeObjectKeyValue(
                 self, "position", keyName="pos", default=Point(0, 0)
             )
-            writer.writeObjectKeyValue(self, "rotation", keyName="angle", default=0)
             writer.writeObjectKeyValue(self, "name", keyName="ref")
             if self.scale != (1, 1):
                 writer.writeKeyValue("scale", Point(list(self.scale)))
@@ -2806,10 +2806,16 @@ GSAnchor._add_parsers(
 
 class GSHint(GSBase):
     def _serialize_to_plist(self, writer):
-        writer.writeObjectKeyValue(self, "horizontal", "if_true")
-
+        if writer.format_version > 2:
+            writer.writeObjectKeyValue(self, "name", condition="if_true")
+        for field in ["horizontal", "options"]:
+            writer.writeObjectKeyValue(self, field, condition="if_true")
         for field in ["origin", "place"]:
             writer.writeObjectKeyValue(self, field)
+        if writer.format_version > 2:
+            writer.writeObjectKeyValue(self, "stem", self.stem != -2)
+        for field in ["other1", "other2", "scale"]:
+            writer.writeObjectKeyValue(self, field)
         if writer.format_version == 2:
             writer.writeObjectKeyValue(self, "target")
         elif self.target:
@@ -2819,12 +2825,11 @@ class GSHint(GSBase):
             else:
                 writer.writeValue(self.target)
             writer.file.write(";\n")
-        for field in ["other1", "other2", "scale"]:
-            writer.writeObjectKeyValue(self, field)
-        writer.writeObjectKeyValue(self, "stem", self.stem != -2)
-
-        for field in ["type", "name", "options", "settings"]:
+        for field in ["settings", "type"]:
             writer.writeObjectKeyValue(self, field, condition="if_true")
+        if writer.format_version == 2:
+            writer.writeObjectKeyValue(self, "stem", self.stem != -2)
+            writer.writeObjectKeyValue(self, "name", condition="if_true")
 
     _defaultsForName = {
         # TODO: (jany) check defaults in glyphs
@@ -3006,8 +3011,8 @@ GSHint._add_parsers(
 class GSFeature(GSBase):
     def _serialize_to_plist(self, writer):
         writer.writeObjectKeyValue(self, "automatic", "if_true")
-        writer.writeObjectKeyValue(self, "disabled", "if_true")
         writer.writeObjectKeyValue(self, "code", True)
+        writer.writeObjectKeyValue(self, "disabled", "if_true")
         if writer.format_version == 3:
             writer.writeObjectKeyValue(self, "labels", "if_true")
             writer.writeKeyValue("tag", self.name)
@@ -3061,8 +3066,8 @@ GSFeature._add_parsers(
 class GSClass(GSFeature):
     def _serialize_to_plist(self, writer):
         writer.writeObjectKeyValue(self, "automatic", "if_true")
-        writer.writeObjectKeyValue(self, "disabled", "if_true")
         writer.writeObjectKeyValue(self, "code", True)
+        writer.writeObjectKeyValue(self, "disabled", "if_true")
         writer.writeKeyValue("name", self.name)
 
     pass
@@ -3573,8 +3578,8 @@ class GSLayer(GSBase):
         writer.writeObjectKeyValue(self, "layerId", "if_true")
         if writer.format_version == 2:
             writer.writeObjectKeyValue(self, "metricLeft", keyName="leftMetricsKey")
-            writer.writeObjectKeyValue(self, "metricWidth", keyName="widthMetricsKey")
             writer.writeObjectKeyValue(self, "metricRight", keyName="rightMetricsKey")
+            writer.writeObjectKeyValue(self, "metricWidth", keyName="widthMetricsKey")
         else:
             writer.writeObjectKeyValue(self, "metricLeft")
             writer.writeObjectKeyValue(self, "metricRight")
@@ -4046,6 +4051,7 @@ class GSGlyph(GSBase):
         writer.writeObjectKeyValue(self, "layers", "if_true")
         if writer.format_version > 2:
             writer.writeObjectKeyValue(self, "metricLeft", "if_true")
+            writer.writeObjectKeyValue(self, "metricRight", "if_true")
             writer.writeObjectKeyValue(self, "metricWidth", "if_true")
         else:
             writer.writeObjectKeyValue(self, "leftKerningGroup", "if_true")
@@ -4055,17 +4061,14 @@ class GSGlyph(GSBase):
             writer.writeObjectKeyValue(
                 self, "metricWidth", "if_true", keyName="widthMetricsKey"
             )
-            writer.writeObjectKeyValue(
-                self, "metricVertWidth", "if_true", keyName="vertWidthMetricsKey"
-            )
-        writer.writeObjectKeyValue(self, "note")
-        if writer.format_version > 2:
-            writer.writeObjectKeyValue(self, "metricRight", "if_true")
-        else:
             writer.writeObjectKeyValue(self, "rightKerningGroup", "if_true")
             writer.writeObjectKeyValue(
                 self, "metricRight", "if_true", keyName="rightMetricsKey"
             )
+            writer.writeObjectKeyValue(
+                self, "metricVertWidth", "if_true", keyName="vertWidthMetricsKey"
+            )
+        writer.writeObjectKeyValue(self, "note")
         writer.writeObjectKeyValue(self, "topKerningGroup", "if_true")
         writer.writeObjectKeyValue(self, "topMetricsKey", "if_true")
         writer.writeObjectKeyValue(self, "bottomKerningGroup", "if_true")


Test patch:
diff --git a/tests/writer_test.py b/tests/writer_test.py
index 9a504616..2b98a5e2 100644
--- a/tests/writer_test.py
+++ b/tests/writer_test.py
@@ -678,9 +678,9 @@ class WriterTest(unittest.TestCase, test_helpers.AssertLinesEqual):
             leftKerningGroup = A;
             leftMetricsKey = A;
             widthMetricsKey = A;
-            note = "Stunning one-bedroom A with renovated acute accent";
             rightKerningGroup = A;
             rightMetricsKey = A;
+            note = "Stunning one-bedroom A with renovated acute accent";
             unicode = 00C1;
             script = latin;
             category = Letter;
@@ -849,8 +849,8 @@ class WriterTest(unittest.TestCase, test_helpers.AssertLinesEqual):
             );
             layerId = L1;
             leftMetricsKey = A;
-            widthMetricsKey = A;
             rightMetricsKey = A;
+            widthMetricsKey = A;
             name = "{125, 100}";
             paths = (
             {
@@ -1120,13 +1120,13 @@ rememberToDownloadARealRemindersApp = 1;}"',
                 """\
             {
             horizontal = 1;
+            options = 128;
             origin = "{0, 0}";
-            target = "{0, 1}";
             other1 = "{0, 2}";
             other2 = "{1, 0}";
+            target = "{0, 1}";
             type = 16;
             name = "My favourite hint";
-            options = 128;
             }
         """
             ),
@@ -1176,14 +1176,14 @@ rememberToDownloadARealRemindersApp = 1;}"',
             dedent(
                 """\
              {
+             name = "My favourite hint";
              horizontal = 1;
+             options = 128;
              origin = (0,0);
-             target = (0,1);
              other1 = (0,2);
              other2 = (1,0);
+             target = (0,1);
              type = 16;
-             name = "My favourite hint";
-             options = 128;
              }
          """,
             ),