From b6ec12bad0aab5301821980b9b8f6da59511d0ba Mon Sep 17 00:00:00 2001
From: Khaled Hosny <khaledhosny@eglug.org>
Date: Tue, 22 Jan 2019 23:37:17 +0200
Subject: [PATCH] [voltLib] Distinguish different PROCESS_MARKS

PROCESS_MARKS followed by a group name is used for markAttachmentType
lookup flag, while followed by MARK_GLYPH_SET is used for
useMarkFilteringSet. The code parsed both correctly but did not
distinguish between the two in the generated AST as it should since they
compile to different lookup flags.
---
 Lib/fontTools/voltLib/ast.py    |  6 ++++--
 Lib/fontTools/voltLib/parser.py |  7 ++++---
 Tests/voltLib/parser_test.py    | 19 ++++++++++++++++++-
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/Lib/fontTools/voltLib/ast.py b/Lib/fontTools/voltLib/ast.py
index 46cffb9e7..de626bae4 100644
--- a/Lib/fontTools/voltLib/ast.py
+++ b/Lib/fontTools/voltLib/ast.py
@@ -170,12 +170,14 @@ class FeatureDefinition(Statement):
 
 
 class LookupDefinition(Statement):
-    def __init__(self, name, process_base, process_marks, direction,
-                 reversal, comments, context, sub, pos, location=None):
+    def __init__(self, name, process_base, process_marks, mark_glyph_set,
+                 direction, reversal, comments, context, sub, pos,
+                 location=None):
         Statement.__init__(self, location)
         self.name = name
         self.process_base = process_base
         self.process_marks = process_marks
+        self.mark_glyph_set = mark_glyph_set
         self.direction = direction
         self.reversal = reversal
         self.comments = comments
diff --git a/Lib/fontTools/voltLib/parser.py b/Lib/fontTools/voltLib/parser.py
index cd2f3a09e..194f273ac 100644
--- a/Lib/fontTools/voltLib/parser.py
+++ b/Lib/fontTools/voltLib/parser.py
@@ -203,11 +203,12 @@ class Parser(object):
             self.advance_lexer_()
             process_base = False
         process_marks = True
+        mark_glyph_set = None
         if self.next_token_ == "PROCESS_MARKS":
             self.advance_lexer_()
             if self.next_token_ == "MARK_GLYPH_SET":
                 self.advance_lexer_()
-                process_marks = self.expect_string_()
+                mark_glyph_set = self.expect_string_()
             elif self.next_token_type_ == Lexer.STRING:
                 process_marks = self.expect_string_()
             elif self.next_token_ == "ALL":
@@ -249,8 +250,8 @@ class Parser(object):
                 "Got %s" % (as_pos_or_sub),
                 location)
         def_lookup = ast.LookupDefinition(
-            name, process_base, process_marks, direction, reversal,
-            comments, context, sub, pos, location=location)
+            name, process_base, process_marks, mark_glyph_set, direction,
+            reversal, comments, context, sub, pos, location=location)
         self.lookups_.define(name, def_lookup)
         return def_lookup
 
diff --git a/Tests/voltLib/parser_test.py b/Tests/voltLib/parser_test.py
index 7ad468605..5288aed0f 100644
--- a/Tests/voltLib/parser_test.py
+++ b/Tests/voltLib/parser_test.py
@@ -665,7 +665,7 @@ class ParserTest(unittest.TestCase):
             (lookup.name, process_marks),
             ("SomeSub", False))
 
-    def test_substitution_process_marks(self):
+    def test_substitution_mark_attachment(self):
         [group, lookup] = self.parse(
             'DEF_GROUP "SomeMarks" ENUM GLYPH "acutecmb" GLYPH "gravecmb" '
             'END_ENUM END_GROUP\n'
@@ -683,6 +683,23 @@ class ParserTest(unittest.TestCase):
             (lookup.name, process_marks),
             ("SomeSub", "SomeMarks"))
 
+    def test_substitution_mark_glyph_set(self):
+        [group, lookup] = self.parse(
+            'DEF_GROUP "SomeMarks" ENUM GLYPH "acutecmb" GLYPH "gravecmb" '
+            'END_ENUM END_GROUP\n'
+            'DEF_LOOKUP "SomeSub" PROCESS_BASE '
+            'PROCESS_MARKS MARK_GLYPH_SET "SomeMarks" \n'
+            'DIRECTION RTL\n'
+            'AS_SUBSTITUTION\n'
+            'SUB GLYPH "A"\n'
+            'WITH GLYPH "A.c2sc"\n'
+            'END_SUB\n'
+            'END_SUBSTITUTION'
+        ).statements
+        self.assertEqual(
+            (lookup.name, lookup.mark_glyph_set),
+            ("SomeSub", "SomeMarks"))
+
     def test_substitution_process_all_marks(self):
         [group, lookup] = self.parse(
             'DEF_GROUP "SomeMarks" ENUM GLYPH "acutecmb" GLYPH "gravecmb" '