Merge pull request #432 from anthrotype/generate-gdef-option

add --generate-GDEF option

Lib/glyphsLib/__init__.py

@@ -89,6 +89,7 @@ def build_masters(
     minimize_glyphs_diffs=False,
     normalize_ufos=False,
     create_background_layers=False,
+    generate_GDEF=True,
 ):
     """Write and return UFOs from the masters and the designspace defined in a
     .glyphs file.
@@ -118,6 +119,7 @@ def build_masters(
         propagate_anchors=propagate_anchors,
         instance_dir=instance_dir,
         minimize_glyphs_diffs=minimize_glyphs_diffs,
+        generate_GDEF=generate_GDEF,
     )
 
     ufos = []

Lib/glyphsLib/builder/__init__.py

@@ -29,6 +29,7 @@ def to_ufos(
     propagate_anchors=True,
     ufo_module=defcon,
     minimize_glyphs_diffs=False,
+    generate_GDEF=True,
 ):
     """Take a GSFont object and convert it into one UFO per master.
 
@@ -39,13 +40,17 @@ def to_ufos(
 
     If family_name is provided, the master UFOs will be given this name and
     only instances with this name will be returned.
+
+    If generate_GDEF is True, write a `table GDEF {...}` statement in the
+    UFO's features.fea, containing GlyphClassDef and LigatureCaretByPos.
     """
     builder = UFOBuilder(
         font,
         ufo_module=ufo_module,
         family_name=family_name,
         propagate_anchors=propagate_anchors,
         minimize_glyphs_diffs=minimize_glyphs_diffs,
+        generate_GDEF=generate_GDEF,
     )
 
     result = list(builder.masters)
@@ -62,6 +67,7 @@ def to_designspace(
     propagate_anchors=True,
     ufo_module=defcon,
     minimize_glyphs_diffs=False,
+    generate_GDEF=True,
 ):
     """Take a GSFont object and convert it into a Designspace Document + UFOS.
     The UFOs are available as the attribute `font` of each SourceDescriptor of
@@ -81,6 +87,9 @@ def to_designspace(
 
     If family_name is provided, the master UFOs will be given this name and
     only instances with this name will be returned.
+
+    If generate_GDEF is True, write a `table GDEF {...}` statement in the
+    UFO's features.fea, containing GlyphClassDef and LigatureCaretByPos.
     """
     builder = UFOBuilder(
         font,
@@ -90,6 +99,7 @@ def to_designspace(
         propagate_anchors=propagate_anchors,
         use_designspace=True,
         minimize_glyphs_diffs=minimize_glyphs_diffs,
+        generate_GDEF=generate_GDEF,
     )
     return builder.designspace
 

Lib/glyphsLib/builder/builders.py

@@ -56,6 +56,7 @@ def __init__(
         propagate_anchors=True,
         use_designspace=False,
         minimize_glyphs_diffs=False,
+        generate_GDEF=True,
     ):
         """Create a builder that goes from Glyphs to UFO + designspace.
 
@@ -76,6 +77,8 @@ def __init__(
         minimize_glyphs_diffs -- set to True to store extra info in UFOs
                                  in order to get smaller diffs between .glyphs
                                  .glyphs files when going glyphs->ufo->glyphs.
+        generate_GDEF -- set to False to skip writing a `table GDEF {...}` in
+                         the UFO features.
         """
         self.font = font
         self.ufo_module = ufo_module
@@ -84,6 +87,7 @@ def __init__(
         self.propagate_anchors = propagate_anchors
         self.use_designspace = use_designspace
         self.minimize_glyphs_diffs = minimize_glyphs_diffs
+        self.generate_GDEF = generate_GDEF
 
         # The set of (SourceDescriptor + UFO)s that will be built,
         # indexed by master ID, the same order as masters in the source GSFont.

Lib/glyphsLib/builder/features.py

@@ -90,7 +90,12 @@ def _to_ufo_features(self, master, ufo):
     # results, we would need anchor propagation or user intervention. Glyphs.app
     # only generates it on generating binaries.
     gdef_str = None
-    if not self.minimize_glyphs_diffs:
+    if self.generate_GDEF:
+        if re.search(r"^\s*table\s+GDEF\s+{", prefix_str, flags=re.MULTILINE):
+            raise ValueError(
+                "The features already contain a `table GDEF {...}` statement. "
+                "Either delete it or set generate_GDEF to False."
+            )
         gdef_str = _build_gdef(ufo)
 
     # make sure feature text is a unicode string, for defcon

Lib/glyphsLib/cli.py

@@ -80,6 +80,14 @@ def main(args=None):
             "full control over all anchors."
         ),
     )
+    group.add_argument(
+        "--generate-GDEF",
+        action="store_true",
+        help=(
+            "write a `table GDEF {...}` statement in the UFO features "
+            "containing `GlyphClassDef` and `LigatureCaretByPos` statements"
+        ),
+    )
     group.add_argument(
         "-N",
         "--normalize-ufos",
@@ -170,6 +178,7 @@ def glyphs2ufo(options):
         propagate_anchors=options.propagate_anchors,
         normalize_ufos=options.normalize_ufos,
         create_background_layers=options.create_background_layers,
+        generate_GDEF=options.generate_GDEF,
     )
 
 

tests/builder/features_test.py

@@ -23,6 +23,8 @@
 
 from glyphsLib import to_glyphs, to_ufos, classes
 
+import pytest
+
 
 def make_font(features):
     ufo = defcon.Font()
@@ -390,15 +392,10 @@ def test_roundtrip_feature_prefix_with_only_a_comment():
     assert prefix_r.code == "#include(../family.fea)"
 
 
-def test_roundtrip_preserve_gdef(tmpdir):
-    """Test that the GDEF table is preserved unchanged regardless of
-    minimize_ufo_diffs.
-
-    See https://github.com/googlei18n/fontmake/issues/457.
-    """
-
+@pytest.fixture
+def ufo_with_GDEF():
     ufo = defcon.Font()
-    gdef_example = dedent(
+    gdef = dedent(
         """\
         table GDEF {
         GlyphClassDef
@@ -409,22 +406,34 @@ def test_roundtrip_preserve_gdef(tmpdir):
         } GDEF;
         """
     )
-    ufo.features.text = gdef_example
+    ufo.features.text = gdef
+    return ufo, gdef
 
-    font = to_glyphs([ufo], minimize_ufo_diffs=True)
+
+def test_roundtrip_existing_GDEF(tmpdir, ufo_with_GDEF):
+    """Test that an existing GDEF table in UFO is preserved unchanged and
+    no extra GDEF table is generated upon roundtripping to UFO when
+    `generate_GDEF` is False.
+    """
+    ufo, gdef = ufo_with_GDEF
+    font = to_glyphs([ufo])
     filename = os.path.join(str(tmpdir), "font.glyphs")
     font.save(filename)
     font = classes.GSFont(filename)
-    rtufo, = to_ufos(font)
+    rtufo, = to_ufos(font, generate_GDEF=False)
+
+    assert rtufo.features.text == gdef
 
-    assert rtufo.features.text == gdef_example
 
-    font = to_glyphs([rtufo], minimize_ufo_diffs=False)
+def test_generate_GDEF_already_exists(tmpdir, ufo_with_GDEF):
+    ufo = ufo_with_GDEF[0]
+    font = to_glyphs([ufo])
+    filename = os.path.join(str(tmpdir), "font.glyphs")
     font.save(filename)
     font = classes.GSFont(filename)
-    rtufo, = to_ufos(font)
 
-    assert rtufo.features.text == gdef_example
+    with pytest.raises(ValueError, match="features already contain a `table GDEF"):
+        to_ufos(font, generate_GDEF=True)
 
 
 def test_groups_remain_at_top(tmpdir):

tests/test_helpers.py

@@ -122,9 +122,12 @@ def _normalize(self, font):
     def assertUFORoundtrip(self, font):
         self._normalize(font)
         expected = write_to_lines(font)
-        # Don't propagate anchors when intending to round-trip
+        # Don't propagate anchors nor generate GDEF when intending to round-trip
         designspace = to_designspace(
-            font, propagate_anchors=False, minimize_glyphs_diffs=True
+            font,
+            propagate_anchors=False,
+            minimize_glyphs_diffs=True,
+            generate_GDEF=False,
         )
 
         # Check that round-tripping in memory is the same as writing on disk