From 60f2c741c3f15361707c19d8381ab33ed512e796 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 15 Jan 2018 18:38:27 +0000 Subject: [PATCH 1/4] CapturingLogHandler: match the fully formatted log message in assertRegex and not the raw 'msg' attribute which still has the '%' formatting placeholders --- Lib/fontTools/misc/loggingTools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/fontTools/misc/loggingTools.py b/Lib/fontTools/misc/loggingTools.py index 4ed788e99..0e509fefa 100644 --- a/Lib/fontTools/misc/loggingTools.py +++ b/Lib/fontTools/misc/loggingTools.py @@ -467,7 +467,7 @@ class CapturingLogHandler(logging.Handler): import re pattern = re.compile(regexp) for r in self.records: - if pattern.search(r.msg): + if pattern.search(r.getMessage()): return True assert 0, "Pattern '%s' not found in logger records" % regexp From 27d40f5160d946c8643d71280bbc3bf6b9dc1438 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 15 Jan 2018 18:43:10 +0000 Subject: [PATCH 2/4] [feaLib.builder] don't error when specific kern pairs conflict Fixes #1147 --- Lib/fontTools/feaLib/builder.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/Lib/fontTools/feaLib/builder.py b/Lib/fontTools/feaLib/builder.py index 4ec7ea9aa..0dd0c283e 100644 --- a/Lib/fontTools/feaLib/builder.py +++ b/Lib/fontTools/feaLib/builder.py @@ -9,6 +9,10 @@ from fontTools.otlLib import builder as otl from fontTools.ttLib import newTable, getTableModule from fontTools.ttLib.tables import otBase, otTables import itertools +import logging + + +log = logging.getLogger(__name__) def addOpenTypeFeatures(font, featurefile): @@ -1424,15 +1428,20 @@ class PairPosBuilder(LookupBuilder): key = (glyph1, glyph2) oldValue = self.glyphPairs.get(key, None) if oldValue is not None: + # the Feature File spec explicitly allows specific pairs generated + # by an 'enum' rule to be overridden by preceding single pairs; + # we emit a warning and use the previously defined value otherLoc = self.locations[key] - raise FeatureLibError( - 'Already defined position for pair %s %s at %s:%d:%d' - % (glyph1, glyph2, otherLoc[0], otherLoc[1], otherLoc[2]), - location) - val1, _ = makeOpenTypeValueRecord(value1, pairPosContext=True) - val2, _ = makeOpenTypeValueRecord(value2, pairPosContext=True) - self.glyphPairs[key] = (val1, val2) - self.locations[key] = location + log.warning( + 'Already defined position for pair %s %s at %s:%d:%d; ' + 'choosing the first value', + glyph1, glyph2, otherLoc[0], otherLoc[1], otherLoc[2]) + val1, val2 = oldValue + else: + val1, _ = makeOpenTypeValueRecord(value1, pairPosContext=True) + val2, _ = makeOpenTypeValueRecord(value2, pairPosContext=True) + self.glyphPairs[key] = (val1, val2) + self.locations[key] = location def add_subtable_break(self, location): self.pairs.append((self.SUBTABLE_BREAK_, self.SUBTABLE_BREAK_, From a4e1d4a2bf5eef8c3ebed24cf1ed1a551d06efef Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 15 Jan 2018 18:45:42 +0000 Subject: [PATCH 3/4] [builder_test] test_pairPos_redefinition_warning --- Tests/feaLib/builder_test.py | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/Tests/feaLib/builder_test.py b/Tests/feaLib/builder_test.py index ff82b2fc4..d649a7bdc 100644 --- a/Tests/feaLib/builder_test.py +++ b/Tests/feaLib/builder_test.py @@ -1,6 +1,7 @@ from __future__ import print_function, division, absolute_import from __future__ import unicode_literals from fontTools.misc.py23 import * +from fontTools.misc.loggingTools import CapturingLogHandler from fontTools.feaLib.builder import Builder, addOpenTypeFeatures, \ addOpenTypeFeaturesFromString from fontTools.feaLib.error import FeatureLibError @@ -13,6 +14,7 @@ import os import shutil import sys import tempfile +import logging import unittest @@ -196,16 +198,29 @@ class BuilderTest(unittest.TestCase): " sub f_f_i by f f i;" "} test;") - def test_pairPos_redefinition(self): - self.assertRaisesRegex( - FeatureLibError, - r"Already defined position for pair A B " - "at .*:2:[0-9]+", # :2: = line 2 - self.build, - "feature test {\n" - " pos A B 123;\n" # line 2 - " pos A B 456;\n" - "} test;\n") + def test_pairPos_redefinition_warning(self): + # https://github.com/fonttools/fonttools/issues/1147 + logger = logging.getLogger("fontTools.feaLib.builder") + with CapturingLogHandler(logger, "WARNING") as captor: + # the pair "yacute semicolon" is redefined in the enum pos + font = self.build( + "@Y_LC = [y yacute ydieresis];" + "@SMALL_PUNC = [comma semicolon period];" + "feature kern {" + " pos yacute semicolon -70;" + " enum pos @Y_LC semicolon -80;" + " pos @Y_LC @SMALL_PUNC -100;" + "} kern;") + + captor.assertRegex("Already defined position for pair yacute semicolon") + + # the first definition prevails: yacute semicolon -70 + st = font["GPOS"].table.LookupList.Lookup[0].SubTable[0] + self.assertEqual(st.Coverage.glyphs[2], "yacute") + self.assertEqual(st.PairSet[2].PairValueRecord[0].SecondGlyph, + "semicolon") + self.assertEqual(vars(st.PairSet[2].PairValueRecord[0].Value1), + {"XAdvance": -70}) def test_singleSubst_multipleSubstitutionsForSameGlyph(self): self.assertRaisesRegex( From 95030cb78837526daec8ab3266d55e404993d6a9 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Mon, 15 Jan 2018 19:50:34 +0000 Subject: [PATCH 4/4] minor: unused variables --- Lib/fontTools/feaLib/builder.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/fontTools/feaLib/builder.py b/Lib/fontTools/feaLib/builder.py index 0dd0c283e..43f6af1ef 100644 --- a/Lib/fontTools/feaLib/builder.py +++ b/Lib/fontTools/feaLib/builder.py @@ -1436,7 +1436,6 @@ class PairPosBuilder(LookupBuilder): 'Already defined position for pair %s %s at %s:%d:%d; ' 'choosing the first value', glyph1, glyph2, otherLoc[0], otherLoc[1], otherLoc[2]) - val1, val2 = oldValue else: val1, _ = makeOpenTypeValueRecord(value1, pairPosContext=True) val2, _ = makeOpenTypeValueRecord(value2, pairPosContext=True)