From 1b1b52500c911bd1665e7c1b5d815bdaae2a049a Mon Sep 17 00:00:00 2001 From: moyogo Date: Thu, 21 Jan 2016 14:03:20 +0000 Subject: [PATCH] [voltLib] handle duplicate or case insensitive duplicate names --- Lib/fontTools/voltLib/parser.py | 40 ++++++++++- Lib/fontTools/voltLib/parser_test.py | 99 ++++++++++++++++++++++++++-- 2 files changed, 129 insertions(+), 10 deletions(-) diff --git a/Lib/fontTools/voltLib/parser.py b/Lib/fontTools/voltLib/parser.py index 860a94f3f..daa0ecd45 100644 --- a/Lib/fontTools/voltLib/parser.py +++ b/Lib/fontTools/voltLib/parser.py @@ -25,7 +25,8 @@ class Parser(object): def __init__(self, path): self.doc_ = ast.VoltFile() self.groups_ = SymbolTable() - self.anchors_ = SymbolTable() + self.anchors_ = {} # dictionary of SymbolTable() keyed by glyph + self.lookups_ = SymbolTable() self.next_token_type_, self.next_token_ = (None, None) self.next_token_location_ = None with open(path, "r") as f: @@ -43,6 +44,7 @@ class Parser(object): if self.next_token_type_ is not None: raise VoltLibError("Expected the end of the file", self.cur_token_location_) + self.groups_.expand() return self.doc_ else: raise VoltLibError( @@ -93,8 +95,11 @@ class Parser(object): enum = self.parse_enum_() self.expect_keyword_("END_GROUP") if self.groups_.resolve(name) is not None: - raise VoltLibError('Glyph group "%s" already defined' % name, - location) + raise VoltLibError( + 'Glyph group "%s" already defined, ' + 'group names are case insensitive' % name, + location + ) def_group = ast.GroupDefinition(location, name, enum) self.groups_.define(name, def_group) return def_group @@ -156,6 +161,12 @@ class Parser(object): assert self.is_cur_keyword_("DEF_LOOKUP") location = self.cur_token_location_ name = self.expect_string_() + if self.lookups_.resolve(name) is not None: + raise VoltLibError( + 'Lookup "%s" already defined, ' + 'lookup names are case insensitive' % name, + location + ) process_base = True if self.next_token_ == "PROCESS_BASE": self.advance_lexer_() @@ -211,6 +222,7 @@ class Parser(object): def_lookup = ast.LookupDefinition( location, name, process_base, process_marks, direction, reversal, comments, context, sub, pos) + self.lookups_.define(name, def_lookup) return def_lookup def parse_context_(self): @@ -357,6 +369,14 @@ class Parser(object): gid = self.expect_number_() self.expect_keyword_("GLYPH") glyph_name = self.expect_name_() + # check for duplicate anchor names on this glyph + if (glyph_name in self.anchors_ + and self.anchors_[glyph_name].resolve(name) is not None): + raise VoltLibError( + 'Anchor "%s" already defined, ' + 'anchor names are case insensitive' % name, + location + ) self.expect_keyword_("COMPONENT") component = self.expect_number_() if self.next_token_ == "LOCKED": @@ -369,6 +389,9 @@ class Parser(object): self.expect_keyword_("END_ANCHOR") anchor = ast.AnchorDefinition(location, name, gid, glyph_name, component, locked, pos) + if glyph_name not in self.anchors_: + self.anchors_[glyph_name] = SymbolTable() + self.anchors_[glyph_name].define(name, anchor) return anchor def parse_adjust_by_(self): @@ -532,6 +555,17 @@ class SymbolTable(parser.SymbolTable): def __init__(self): parser.SymbolTable.__init__(self) + def resolve(self, name, case_insensitive=True): + for scope in reversed(self.scopes_): + item = scope.get(name) + if item: + return item + if case_insensitive: + for key in scope: + if key.lower() == name.lower(): + return scope[key] + return None + # TODO # add expanding ranges def expand(self): diff --git a/Lib/fontTools/voltLib/parser_test.py b/Lib/fontTools/voltLib/parser_test.py index 2a2ab34b8..666ace4f5 100644 --- a/Lib/fontTools/voltLib/parser_test.py +++ b/Lib/fontTools/voltLib/parser_test.py @@ -187,11 +187,26 @@ class ParserTest(unittest.TestCase): def test_group_duplicate(self): self.assertRaisesRegex( - VoltLibError, 'Glyph group "dup" already defined', - self.parse, 'DEF_GROUP "dup"\n' + VoltLibError, + 'Glyph group "dupe" already defined, ' + 'group names are case insensitive', + self.parse, 'DEF_GROUP "dupe"\n' 'ENUM GLYPH "a" GLYPH "b" END_ENUM\n' 'END_GROUP\n' - 'DEF_GROUP "dup"\n' + 'DEF_GROUP "dupe"\n' + 'ENUM GLYPH "x" END_ENUM\n' + 'END_GROUP\n' + ) + + def test_group_duplicate_case_insensitive(self): + self.assertRaisesRegex( + VoltLibError, + 'Glyph group "Dupe" already defined, ' + 'group names are case insensitive', + self.parse, 'DEF_GROUP "dupe"\n' + 'ENUM GLYPH "a" GLYPH "b" END_ENUM\n' + 'END_GROUP\n' + 'DEF_GROUP "Dupe"\n' 'ENUM GLYPH "x" END_ENUM\n' 'END_GROUP\n' ) @@ -303,6 +318,48 @@ class ParserTest(unittest.TestCase): "kern", ["kern1", "kern2"])) + def test_lookup_duplicate(self): + with self.assertRaisesRegex( + VoltLibError, + 'Lookup "dupe" already defined, ' + 'lookup names are case insensitive', + ): + [lookup1, lookup2] = self.parse( + 'DEF_LOOKUP "dupe"\n' + 'AS_SUBSTITUTION\n' + 'SUB GLYPH "a"\n' + 'WITH GLYPH "a.alt"\n' + 'END_SUB\n' + 'END_SUBSTITUTION\n' + 'DEF_LOOKUP "dupe"\n' + 'AS_SUBSTITUTION\n' + 'SUB GLYPH "b"\n' + 'WITH GLYPH "b.alt"\n' + 'END_SUB\n' + 'END_SUBSTITUTION\n' + ).statements + + def test_lookup_duplicate_insensitive_case(self): + with self.assertRaisesRegex( + VoltLibError, + 'Lookup "Dupe" already defined, ' + 'lookup names are case insensitive', + ): + [lookup1, lookup2] = self.parse( + 'DEF_LOOKUP "dupe"\n' + 'AS_SUBSTITUTION\n' + 'SUB GLYPH "a"\n' + 'WITH GLYPH "a.alt"\n' + 'END_SUB\n' + 'END_SUBSTITUTION\n' + 'DEF_LOOKUP "Dupe"\n' + 'AS_SUBSTITUTION\n' + 'SUB GLYPH "b"\n' + 'WITH GLYPH "b.alt"\n' + 'END_SUB\n' + 'END_SUBSTITUTION\n' + ).statements + def test_substitution_empty(self): with self.assertRaisesRegex( VoltLibError, @@ -653,16 +710,44 @@ class ParserTest(unittest.TestCase): ) def test_def_anchor(self): - [anchor] = self.parse( + [anchor1, anchor2, anchor3] = self.parse( + 'DEF_ANCHOR "top" ON 120 GLYPH a ' + 'COMPONENT 1 AT POS DX 250 DY 450 END_POS END_ANCHOR\n' 'DEF_ANCHOR "MARK_top" ON 120 GLYPH acutecomb ' - 'COMPONENT 1 AT POS DX 0 DY 450 END_POS END_ANCHOR' + 'COMPONENT 1 AT POS DX 0 DY 450 END_POS END_ANCHOR\n' + 'DEF_ANCHOR "bottom" ON 120 GLYPH a ' + 'COMPONENT 1 AT POS DX 250 DY 0 END_POS END_ANCHOR\n' ).statements self.assertEqual( - (anchor.name, anchor.gid, anchor.glyph_name, anchor.component, - anchor.locked, anchor.pos), + (anchor1.name, anchor1.gid, anchor1.glyph_name, anchor1.component, + anchor1.locked, anchor1.pos), + ("top", 120, "a", 1, + False, (None, 250, 450, {}, {}, {})) + ) + self.assertEqual( + (anchor2.name, anchor2.gid, anchor2.glyph_name, anchor2.component, + anchor2.locked, anchor2.pos), ("MARK_top", 120, "acutecomb", 1, False, (None, 0, 450, {}, {}, {})) ) + self.assertEqual( + (anchor3.name, anchor3.gid, anchor3.glyph_name, anchor3.component, + anchor3.locked, anchor3.pos), + ("bottom", 120, "a", 1, + False, (None, 250, 0, {}, {}, {})) + ) + + def test_def_anchor_duplicate(self): + self.assertRaisesRegex( + VoltLibError, + 'Anchor "dupe" already defined, ' + 'anchor names are case insensitive', + self.parse, + 'DEF_ANCHOR "dupe" ON 120 GLYPH a ' + 'COMPONENT 1 AT POS DX 250 DY 450 END_POS END_ANCHOR\n' + 'DEF_ANCHOR "dupe" ON 120 GLYPH a ' + 'COMPONENT 1 AT POS DX 250 DY 450 END_POS END_ANCHOR\n' + ) def test_anchor_adjust_device(self): [anchor] = self.parse(