From 1cfb67322777ee8506a08d7746947eefda454856 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Fri, 22 Jul 2022 15:31:53 +0200 Subject: [PATCH 1/3] Add feaIncludeDir option to allow override the include search path This is intended to fix https://github.com/googlefonts/glyphsLib/issues/797, by allowing fontmake to override the default include directory used for parsing a UFO features.fea; useful when the UFOs where generated from a .glyphs source whose features contain some `include` statements and they are saved in a different directory (or not saved to disk at all). fontmake will need to be modified to call ufo2ft with this new option when starting to build from a .glyphs input. --- Lib/ufo2ft/__init__.py | 1 + Lib/ufo2ft/featureCompiler.py | 27 ++++++++++++++++++++++----- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/Lib/ufo2ft/__init__.py b/Lib/ufo2ft/__init__.py index 4295caaaf..386c9e8cd 100644 --- a/Lib/ufo2ft/__init__.py +++ b/Lib/ufo2ft/__init__.py @@ -99,6 +99,7 @@ def call_postprocessor(otf, ufo, glyphSet, *, postProcessorClass, **kwargs): debugFeatureFile=None, notdefGlyph=None, colrLayerReuse=True, + feaIncludeDir=None, ) compileOTF_args = { diff --git a/Lib/ufo2ft/featureCompiler.py b/Lib/ufo2ft/featureCompiler.py index aa6d90de2..630e62565 100644 --- a/Lib/ufo2ft/featureCompiler.py +++ b/Lib/ufo2ft/featureCompiler.py @@ -24,17 +24,20 @@ logger = logging.getLogger(__name__) -def parseLayoutFeatures(font): +def parseLayoutFeatures(font, includeDir=None): """Parse OpenType layout features in the UFO and return a feaLib.ast.FeatureFile instance. + + includeDir is an optional directory path to search for included + feature files, if omitted the font.path is used. If the latter + is also not set, the feaLib Lexer uses the current working directory. """ featxt = font.features.text or "" if not featxt: return ast.FeatureFile() buf = StringIO(featxt) ufoPath = font.path - includeDir = None - if ufoPath is not None: + if includeDir is None and ufoPath is not None: # The UFO v3 specification says "Any include() statements must be relative to # the UFO path, not to the features.fea file itself". We set the `name` # attribute on the buffer to the actual feature file path, which feaLib will @@ -44,6 +47,7 @@ def parseLayoutFeatures(font): buf.name = os.path.join(ufoPath, "features.fea") includeDir = os.path.dirname(ufoPath) glyphNames = set(font.keys()) + includeDir = os.path.normpath(includeDir) if includeDir else None try: parser = Parser(buf, glyphNames, includeDir=includeDir) doc = parser.parse() @@ -157,7 +161,15 @@ class FeatureCompiler(BaseFeatureCompiler): CursFeatureWriter, ] - def __init__(self, ufo, ttFont=None, glyphSet=None, featureWriters=None, **kwargs): + def __init__( + self, + ufo, + ttFont=None, + glyphSet=None, + featureWriters=None, + feaIncludeDir=None, + **kwargs, + ): """ Args: featureWriters: a list of BaseFeatureWriter subclasses or @@ -177,9 +189,14 @@ def __init__(self, ufo, ttFont=None, glyphSet=None, featureWriters=None, **kwarg which gets replaced by either the UFO.lib writers or the default ones; thus one can insert additional writers either before or after these. + feaIncludeDir: a directory to be used as the include directory for + the feature file. If None, the include directory is set to the + parent directory of the UFO, provided the UFO has a path. """ BaseFeatureCompiler.__init__(self, ufo, ttFont, glyphSet) + self.feaIncludeDir = feaIncludeDir + self.initFeatureWriters(featureWriters) if kwargs.get("mtiFeatures") is not None: @@ -259,7 +276,7 @@ def setupFeatures(self): in a different way if desired. """ if self.featureWriters: - featureFile = parseLayoutFeatures(self.ufo) + featureFile = parseLayoutFeatures(self.ufo, self.feaIncludeDir) for writer in self.featureWriters: writer.write(self.ufo, featureFile, compiler=self) From 0246ce8ddd89fff3ddbb4db5d6dad5c1b1c32082 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 28 Jul 2022 16:59:01 +0200 Subject: [PATCH 2/3] add tests for feaIncludeDir --- tests/featureCompiler_test.py | 66 +++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/featureCompiler_test.py b/tests/featureCompiler_test.py index 1a413ce87..5f0505945 100644 --- a/tests/featureCompiler_test.py +++ b/tests/featureCompiler_test.py @@ -77,6 +77,29 @@ def test_include_not_found(self, FontClass, tmpdir, caplog): assert len(caplog.records) == 1 assert "change the file name in the include" in caplog.text + def test_include_dir(self, FontClass, tmp_path, caplog): + features_dir = tmp_path / "features" + features_dir.mkdir() + (features_dir / "test.fea").write_text( + dedent( + """\ + # hello world + """ + ), + encoding="utf-8", + ) + ufo = FontClass() + ufo.features.text = dedent( + """\ + include(test.fea) + """ + ) + ufo.save(tmp_path / "Test.ufo") + + fea = parseLayoutFeatures(ufo, features_dir) + + assert "# hello world" in str(fea) + class DummyFeatureWriter: tableTag = "GPOS" @@ -277,3 +300,46 @@ def test_buildTables_FeatureLibError(self, FontClass, caplog): finally: if tmpfile is not None: tmpfile.remove(ignore_errors=True) + + def test_setupFeatures_custom_feaIncludeDir(self, FontClass, tmp_path): + (tmp_path / "family.fea").write_text( + """\ + feature liga { + sub f f by f_f; + } liga; + """ + ) + ufo = FontClass() + ufo.newGlyph("a") + ufo.newGlyph("v") + ufo.newGlyph("f") + ufo.newGlyph("f_f") + ufo.kerning.update({("a", "v"): -40}) + ufo.features.text = dedent( + """\ + include(family.fea); + """ + ) + compiler = FeatureCompiler(ufo, feaIncludeDir=str(tmp_path)) + + compiler.setupFeatures() + + assert compiler.features == dedent( + """\ + feature liga { + sub f f by f_f; + } liga; + + + lookup kern_Common { + lookupflag IgnoreMarks; + pos a v -40; + } kern_Common; + + feature kern { + script DFLT; + language dflt; + lookup kern_Common; + } kern; + """ + ) From 6a4a59d94c1ca67421428e579fbb7714c200c5ad Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 28 Jul 2022 17:21:35 +0200 Subject: [PATCH 3/3] also test feaIncludeDir param in top-level compile method --- tests/integration_test.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/integration_test.py b/tests/integration_test.py index fc1a8bb6e..214480dc1 100644 --- a/tests/integration_test.py +++ b/tests/integration_test.py @@ -2,6 +2,7 @@ import io import os import sys +from pathlib import Path import pytest from fontTools.pens.boundsPen import BoundsPen @@ -99,6 +100,17 @@ def test_included_features(self, FontClass): ttf = compileTTF(ufo) expectTTX(ttf, "Bug108.ttx", tables=self._layoutTables) + def test_included_features_with_custom_include_dir(self, FontClass, tmp_path): + ufo = FontClass(getpath("Bug108.ufo")) + features_dir = tmp_path / "features" + features_dir.mkdir() + (features_dir / "foobarbaz.fea").write_text( + Path(getpath("Bug108_included.fea")).read_text() + ) + ufo.features.text = "include(features/foobarbaz.fea);" + ttf = compileTTF(ufo, feaIncludeDir=tmp_path) + expectTTX(ttf, "Bug108.ttx", tables=self._layoutTables) + def test_mti_features(self, FontClass): """Checks handling of UFOs with embdedded MTI/Monotype feature files https://github.com/googlei18n/fontmake/issues/289