Revert "Support public.skipExportGlyphs lib key for storing export fl…

…ags (#519)"

This reverts commit 198c4bb.

We need to release a bugfix v3.3.1 release after reverting #451, but
this is a major new feature that changes the way fontmake --subset option
behaves. Better done if a separate release. I will re-revert it back to
master after releasing v3.3.1.

Lib/glyphsLib/builder/builders.py

@@ -208,17 +208,6 @@ def masters(self):
             for layer in ufo.layers:
                 self.to_ufo_layer_lib(layer)
 
-        # Sanitize skip list and write it to both Designspace- and UFO-level lib keys.
-        # The latter is unnecessary when using e.g. the `ufo2ft.compile*FromDS`
-        # functions, but the data may take a different path. Writing it everywhere can
-        # save on surprises/logic in other software.
-        skip_export_glyphs = self._designspace.lib.get("public.skipExportGlyphs")
-        if skip_export_glyphs is not None:
-            skip_export_glyphs = sorted(set(skip_export_glyphs))
-            self._designspace.lib["public.skipExportGlyphs"] = skip_export_glyphs
-            for source in self._sources.values():
-                source.font.lib["public.skipExportGlyphs"] = skip_export_glyphs
-
         self.to_ufo_features()  # This depends on the glyphOrder key
         self.to_ufo_groups()
         self.to_ufo_kerning()
@@ -680,25 +669,6 @@ def _fake_designspace(self, ufos):
             source.path = ufo.path
             source.location = ufo_to_location[ufo]
             designspace.addSource(source)
-
-        # UFO-level skip list lib keys are usually ignored, except when we don't have a
-        # Designspace file to start from. If they exist in the UFOs, promote them to a
-        # Designspace-level lib key. However, to avoid accidents, expect the list to
-        # exist in none or be the same in all UFOs.
-        if any("public.skipExportGlyphs" in ufo.lib for ufo in ufos):
-            skip_export_glyphs = {
-                frozenset(ufo.lib.get("public.skipExportGlyphs", [])) for ufo in ufos
-            }
-            if len(skip_export_glyphs) == 1:
-                designspace.lib["public.skipExportGlyphs"] = sorted(
-                    next(iter(skip_export_glyphs))
-                )
-            else:
-                raise ValueError(
-                    "The `public.skipExportGlyphs` list of all UFOs must either not "
-                    "exist or be the same in every UFO."
-                )
-
         return designspace
 
     # Implementation is split into one file per feature

Lib/glyphsLib/builder/features.py

@@ -96,9 +96,7 @@ def _to_ufo_features(self, master, ufo):
                 "The features already contain a `table GDEF {...}` statement. "
                 "Either delete it or set generate_GDEF to False."
             )
-        gdef_str = _build_gdef(
-            ufo, self._designspace.lib.get("public.skipExportGlyphs")
-        )
+        gdef_str = _build_gdef(ufo)
 
     # make sure feature text is a unicode string, for defcon
     full_text = (
@@ -107,7 +105,7 @@ def _to_ufo_features(self, master, ufo):
     ufo.features.text = full_text if full_text.strip() else ""
 
 
-def _build_gdef(ufo, skipExportGlyphs=None):
+def _build_gdef(ufo):
     """Build a GDEF table statement (GlyphClassDef and LigatureCaretByPos).
 
     Building GlyphClassDef requires anchor propagation or user care to work as
@@ -134,12 +132,6 @@ def _build_gdef(ufo, skipExportGlyphs=None):
     subCategory_key = GLYPHLIB_PREFIX + "subCategory"
 
     for glyph in ufo:
-        # Do not generate any entries for non-export glyphs, as looking them up on
-        # compilation will fail.
-        if skipExportGlyphs is not None:
-            if glyph.name in skipExportGlyphs:
-                continue
-
         has_attaching_anchor = False
         for anchor in glyph.anchors:
             name = anchor.name

Lib/glyphsLib/builder/glyph.py

@@ -65,9 +65,7 @@ def to_ufo_glyph(self, ufo_glyph, layer, glyph):
 
     export = glyph.export
     if not export:
-        if "public.skipExportGlyphs" not in self._designspace.lib:
-            self._designspace.lib["public.skipExportGlyphs"] = []
-        self._designspace.lib["public.skipExportGlyphs"].append(glyph.name)
+        ufo_glyph.lib[GLYPHLIB_PREFIX + "Export"] = export
 
     # FIXME: (jany) next line should be an API of GSGlyph?
     glyphinfo = glyphsLib.glyphdata.get_glyph(ufo_glyph.name)
@@ -149,23 +147,18 @@ def to_glyphs_glyph(self, ufo_glyph, ufo_layer, master):
 
     if ufo_glyph.unicodes:
         glyph.unicodes = ["{:04X}".format(c) for c in ufo_glyph.unicodes]
-    glyph.note = ufo_glyph.note or ""
+    note = ufo_glyph.note
+    if note is not None:
+        glyph.note = note
     if GLYPHLIB_PREFIX + "lastChange" in ufo_glyph.lib:
         last_change = ufo_glyph.lib[GLYPHLIB_PREFIX + "lastChange"]
         # We cannot be strict about the dateformat because it's not an official
         # UFO field mentioned in the spec so it could happen to have a timezone
         glyph.lastChange = from_loose_ufo_time(last_change)
     if ufo_glyph.markColor:
         glyph.color = _to_glyphs_color(ufo_glyph.markColor)
-
-    # The export flag can be stored in the glyph's lib key (for upgrading legacy
-    # sources) or the Designspace-level public.skipExportGlyphs lib key (canonical
-    # place to store the information). The UFO level lib key is ignored.
     if GLYPHLIB_PREFIX + "Export" in ufo_glyph.lib:
         glyph.export = ufo_glyph.lib[GLYPHLIB_PREFIX + "Export"]
-    if ufo_glyph.name in self.designspace.lib.get("public.skipExportGlyphs", []):
-        glyph.export = False
-
     ps_names_key = PUBLIC_PREFIX + "postscriptNames"
     if (
         ps_names_key in ufo_glyph.font.lib

tests/builder/builder_test.py

@@ -836,111 +836,17 @@ def test_supplementary_layers(self):
     def test_glyph_lib_Export(self):
         font = generate_minimal_font()
         glyph = add_glyph(font, "a")
+
         self.assertEqual(glyph.export, True)
 
         ufo = to_ufos(font)[0]
-        ds = to_designspace(font)
 
         self.assertNotIn(GLYPHLIB_PREFIX + "Export", ufo["a"].lib)
-        self.assertNotIn("public.skipExportGlyphs", ufo.lib)
-        self.assertNotIn("public.skipExportGlyphs", ds.lib)
-
-        font2 = to_glyphs(ds)
-        glyph2 = font2.glyphs[0]
-        self.assertEqual(glyph2.name, "a")
-        self.assertEqual(glyph2.export, True)
-
-        glyph2.export = False
-        ufo = to_ufos(font2)[0]
-        ds = to_designspace(font2)
 
-        self.assertNotIn(GLYPHLIB_PREFIX + "Export", ufo["a"].lib)
-        self.assertEqual(ufo.lib["public.skipExportGlyphs"], ["a"])
-        self.assertEqual(ds.lib["public.skipExportGlyphs"], ["a"])
-
-    def test_glyph_lib_Export_mixed(self):
-        font = generate_minimal_font()
-        add_glyph(font, "a")
-        add_glyph(font, "b")
-        add_glyph(font, "c")
-        add_glyph(font, "d")
-        ds = to_designspace(font)
-        ufo = ds.sources[0].font
-
-        ufo["a"].lib[GLYPHLIB_PREFIX + "Export"] = False
-        ufo.lib["public.skipExportGlyphs"] = ["b"]
-        ds.lib["public.skipExportGlyphs"] = ["c"]
-
-        font2 = to_glyphs(ds)
-        ds2 = to_designspace(font2)
-        ufo2 = ds2.sources[0].font
-
-        self.assertNotIn(GLYPHLIB_PREFIX + "Export", ufo2["a"].lib)
-        self.assertNotIn(GLYPHLIB_PREFIX + "Export", ufo2["b"].lib)
-        self.assertNotIn(GLYPHLIB_PREFIX + "Export", ufo2["c"].lib)
-        self.assertNotIn(GLYPHLIB_PREFIX + "Export", ufo2["d"].lib)
-        self.assertEqual(ufo2.lib["public.skipExportGlyphs"], ["a", "c"])
-        self.assertEqual(ds2.lib["public.skipExportGlyphs"], ["a", "c"])
-
-        font3 = to_glyphs(ds2)
-        self.assertEqual(font3.glyphs["a"].export, False)
-        self.assertEqual(font3.glyphs["b"].export, True)
-        self.assertEqual(font3.glyphs["c"].export, False)
-        self.assertEqual(font3.glyphs["d"].export, True)
-
-        ufos3 = to_ufos(font2)
-        ufo3 = ufos3[0]
-        self.assertNotIn(GLYPHLIB_PREFIX + "Export", ufo2["a"].lib)
-        self.assertNotIn(GLYPHLIB_PREFIX + "Export", ufo2["b"].lib)
-        self.assertNotIn(GLYPHLIB_PREFIX + "Export", ufo2["c"].lib)
-        self.assertNotIn(GLYPHLIB_PREFIX + "Export", ufo2["d"].lib)
-        self.assertEqual(ufo3.lib["public.skipExportGlyphs"], ["a", "c"])
-
-    def test_glyph_lib_Export_GDEF(self):
-        font = generate_minimal_font()
-        add_glyph(font, "a")
-        add_glyph(font, "d")
-        add_anchor(font, "d", "top", 100, 100)
-
-        ds = to_designspace(font)
-        ufo = ds.sources[0].font
-        self.assertIn(
-            "GlyphClassDef[d]", ufo.features.text.replace("\n", "").replace(" ", "")
-        )
-
-        font.glyphs["d"].export = False
-        ds2 = to_designspace(font)
-        ufo2 = ds2.sources[0].font
-        self.assertEqual(ufo2.features.text, "")
-
-    def test_glyph_lib_Export_fake_designspace(self):
-        font = generate_minimal_font()
-        master = GSFontMaster()
-        master.ascender = 0
-        master.capHeight = 0
-        master.descender = 0
-        master.id = "id"
-        master.xHeight = 0
-        font.masters.append(master)
-        add_glyph(font, "a")
-        add_glyph(font, "b")
-        ds = to_designspace(font)
-
-        ufos = [source.font for source in ds.sources]
-
-        font2 = to_glyphs(ufos)
-        ds2 = to_designspace(font2)
-        self.assertNotIn("public.skipExportGlyphs", ds2.lib)
-
-        ufos[0].lib["public.skipExportGlyphs"] = ["a"]
-
-        with self.assertRaises(ValueError):
-            to_glyphs(ufos)
+        glyph.export = False
+        ufo = to_ufos(font)[0]
 
-        ufos[1].lib["public.skipExportGlyphs"] = ["a"]
-        font3 = to_glyphs(ufos)
-        self.assertEqual(font3.glyphs["a"].export, False)
-        self.assertEqual(font3.glyphs["b"].export, True)
+        self.assertEqual(ufo["a"].lib[GLYPHLIB_PREFIX + "Export"], False)
 
     def test_glyph_lib_metricsKeys(self):
         font = generate_minimal_font()