Distinguish value records format A from format B with zero values

Fixes https://github.com/fonttools/fonttools/issues/848.

Lib/fontTools/feaLib/ast.py

@@ -973,7 +973,8 @@ class SubtableStatement(Statement):
 
 
 class ValueRecord(Expression):
-    def __init__(self, location, vertical, xPlacement, yPlacement, xAdvance, yAdvance,
+    def __init__(self, location, vertical,
+                 xPlacement, yPlacement, xAdvance, yAdvance,
                  xPlaDevice, yPlaDevice, xAdvDevice, yAdvDevice):
         Expression.__init__(self, location)
         self.xPlacement, self.yPlacement = (xPlacement, yPlacement)
@@ -1008,10 +1009,10 @@ class ValueRecord(Expression):
             vertical = self.vertical
 
         # Try format A, if possible.
-        if x == 0 and y == 0:
+        if x is None and y is None:
-            if xAdvance == 0 and vertical:
+            if xAdvance is None and vertical:
                 return str(yAdvance)
-            elif yAdvance == 0 and not vertical:
+            elif yAdvance is None and not vertical:
                 return str(xAdvance)
 
         # Try format B, if possible.

Lib/fontTools/feaLib/parser.py

@@ -1024,10 +1024,12 @@ class Parser(object):
         if self.next_token_type_ is Lexer.NUMBER:
             number, location = self.expect_number_(), self.cur_token_location_
             if vertical:
-                val = self.ast.ValueRecord(location, vertical, 0, 0, 0, number,
+                val = self.ast.ValueRecord(location, vertical,
+                                           None, None, None, number,
                                            None, None, None, None)
             else:
-                val = self.ast.ValueRecord(location, vertical, 0, 0, number, 0,
+                val = self.ast.ValueRecord(location, vertical,
+                                           None, None, number, None,
                                            None, None, None, None)
             return val
         self.expect_symbol_("<")

Tests/feaLib/data/GPOS_1.fea

@@ -26,7 +26,6 @@ feature kern {
     # single adjustment positionings, provided the re-definition is using
     # the same value.  We replicate this behavior.
     pos four 400;
-#test-fea2fea: pos four 400;
     pos four <0 0 400 0>;
     pos nine -100;
 } kern;

Tests/feaLib/data/spec6h_iii_1.fea

@@ -5,6 +5,5 @@
 languagesystem DFLT dflt;
 
 feature test {
-#test-fea2fea: pos [quoteleft quotedblleft] [Y T]' 20 [quoteright quotedblright];
     pos [quoteleft quotedblleft] [Y T]' <0 0 20 0> [quoteright quotedblright];
 } test;

Tests/feaLib/parser_test.py

@@ -529,7 +529,7 @@ class ParserTest(unittest.TestCase):
         [look] = liga.statements
         [foo] = look.statements
         self.assertEqual(foo.value.xAdvance, 123)
-        self.assertEqual(foo.value.yAdvance, 0)
+        self.assertIsNone(foo.value.yAdvance)
 
     def test_lookup_block_with_vertical_valueRecordDef(self):
         doc = self.parse("feature vkrn {"
@@ -540,7 +540,7 @@ class ParserTest(unittest.TestCase):
         [vkrn] = doc.statements
         [look] = vkrn.statements
         [foo] = look.statements
-        self.assertEqual(foo.value.xAdvance, 0)
+        self.assertIsNone(foo.value.xAdvance)
         self.assertEqual(foo.value.yAdvance, 123)
 
     def test_lookup_reference(self):
@@ -1248,26 +1248,54 @@ class ParserTest(unittest.TestCase):
     def test_valuerecord_format_a_horizontal(self):
         doc = self.parse("feature liga {valueRecordDef 123 foo;} liga;")
         value = doc.statements[0].statements[0].value
-        self.assertEqual(value.xPlacement, 0)
+        self.assertIsNone(value.xPlacement)
-        self.assertEqual(value.yPlacement, 0)
+        self.assertIsNone(value.yPlacement)
         self.assertEqual(value.xAdvance, 123)
-        self.assertEqual(value.yAdvance, 0)
+        self.assertIsNone(value.yAdvance)
         self.assertIsNone(value.xPlaDevice)
         self.assertIsNone(value.yPlaDevice)
         self.assertIsNone(value.xAdvDevice)
         self.assertIsNone(value.yAdvDevice)
+        self.assertEqual(value.makeString(vertical=False), "123")
 
     def test_valuerecord_format_a_vertical(self):
         doc = self.parse("feature vkrn {valueRecordDef 123 foo;} vkrn;")
         value = doc.statements[0].statements[0].value
-        self.assertEqual(value.xPlacement, 0)
+        self.assertIsNone(value.xPlacement)
-        self.assertEqual(value.yPlacement, 0)
+        self.assertIsNone(value.yPlacement)
-        self.assertEqual(value.xAdvance, 0)
+        self.assertIsNone(value.xAdvance)
         self.assertEqual(value.yAdvance, 123)
         self.assertIsNone(value.xPlaDevice)
         self.assertIsNone(value.yPlaDevice)
         self.assertIsNone(value.xAdvDevice)
         self.assertIsNone(value.yAdvDevice)
+        self.assertEqual(value.makeString(vertical=True), "123")
+
+    def test_valuerecord_format_a_zero_horizontal(self):
+        doc = self.parse("feature liga {valueRecordDef 0 foo;} liga;")
+        value = doc.statements[0].statements[0].value
+        self.assertIsNone(value.xPlacement)
+        self.assertIsNone(value.yPlacement)
+        self.assertEqual(value.xAdvance, 0)
+        self.assertIsNone(value.yAdvance)
+        self.assertIsNone(value.xPlaDevice)
+        self.assertIsNone(value.yPlaDevice)
+        self.assertIsNone(value.xAdvDevice)
+        self.assertIsNone(value.yAdvDevice)
+        self.assertEqual(value.makeString(vertical=False), "0")
+
+    def test_valuerecord_format_a_zero_vertical(self):
+        doc = self.parse("feature vkrn {valueRecordDef 0 foo;} vkrn;")
+        value = doc.statements[0].statements[0].value
+        self.assertIsNone(value.xPlacement)
+        self.assertIsNone(value.yPlacement)
+        self.assertIsNone(value.xAdvance)
+        self.assertEqual(value.yAdvance, 0)
+        self.assertIsNone(value.xPlaDevice)
+        self.assertIsNone(value.yPlaDevice)
+        self.assertIsNone(value.xAdvDevice)
+        self.assertIsNone(value.yAdvDevice)
+        self.assertEqual(value.makeString(vertical=True), "0")
 
     def test_valuerecord_format_a_vertical_contexts_(self):
         for tag in "vkrn vpal vhal valt".split():
@@ -1289,6 +1317,20 @@ class ParserTest(unittest.TestCase):
         self.assertIsNone(value.yPlaDevice)
         self.assertIsNone(value.xAdvDevice)
         self.assertIsNone(value.yAdvDevice)
+        self.assertEqual(value.makeString(vertical=False), "<1 2 3 4>")
+
+    def test_valuerecord_format_b_zero(self):
+        doc = self.parse("feature liga {valueRecordDef <0 0 0 0> foo;} liga;")
+        value = doc.statements[0].statements[0].value
+        self.assertEqual(value.xPlacement, 0)
+        self.assertEqual(value.yPlacement, 0)
+        self.assertEqual(value.xAdvance, 0)
+        self.assertEqual(value.yAdvance, 0)
+        self.assertIsNone(value.xPlaDevice)
+        self.assertIsNone(value.yPlaDevice)
+        self.assertIsNone(value.xAdvDevice)
+        self.assertIsNone(value.yAdvDevice)
+        self.assertEqual(value.makeString(vertical=False), "<0 0 0 0>")
 
     def test_valuerecord_format_c(self):
         doc = self.parse(
@@ -1310,6 +1352,9 @@ class ParserTest(unittest.TestCase):
         self.assertEqual(value.yPlaDevice, ((11, 111), (12, 112)))
         self.assertIsNone(value.xAdvDevice)
         self.assertEqual(value.yAdvDevice, ((33, -113), (44, -114), (55, 115)))
+        self.assertEqual(value.makeString(vertical=False),
+                         "<1 2 3 4 <device 8 88> <device 11 111, 12 112>"
+                         " <device NULL> <device 33 -113, 44 -114, 55 115>>")
 
     def test_valuerecord_format_d(self):
         doc = self.parse("feature test {valueRecordDef <NULL> foo;} test;")