[fdiff.diff] add support for table inclusion and exclusion filters in unified diffs

Author: chrissimpkins

lib/fdiff/diff.py

@@ -7,16 +7,66 @@
 from fdiff.thirdparty.fdifflib import unified_diff
 
 
-def u_diff(filepath_a, filepath_b, context_lines=3):
+def u_diff(
+    filepath_a, filepath_b, context_lines=3, include_tables=None, exclude_tables=None
+):
+    """Performs a unified diff on a TTX serialized data format dump of font binary data using
+    a modified version of the Python standard libary difflib module.
+
+    filepath_a: (string) pre-file path
+    filepath_b: (string) post-file path
+    context_lines: (int) number of context lines to include in the diff (default=3)
+    include_tables: (list of str) Python list of OpenType tables to include in the diff
+    exclude_tables: (list of str) Python list of OpentType tables to exclude from the diff
+
+    include_tables and exclude_tables are mutually exclusive arguments.  Only one should
+    be defined
+
+    :returns: Generator of ordered diff line strings that include newline line endings
+    :raises: KeyError if include_tables or exclude_tables includes a mis-specified table
+    that is not included in filepath_a OR filepath_b"""
     tt_left = TTFont(filepath_a)
     tt_right = TTFont(filepath_b)
 
+    # Validation: include_tables request should be for tables that are in one of
+    # the two fonts. This otherwise silently passes with exit status code 0 which
+    # could lead to the interpretation of no diff between two files when the table
+    # entry is incorrectly defined or is a typo.  Let's be conservative and consider
+    # this an error, force user to use explicit definitions that include tables in
+    # one of the two files, and understand that the diff request was for one or more
+    # tables that are not present.
+    if include_tables is not None:
+        for table in include_tables:
+            if table not in tt_left and table not in tt_right:
+                raise KeyError(
+                    f"'{table}' table was not identified for inclusion in either font"
+                )
+
+    # Validation: exclude_tables request should be for tables that are in one of
+    # the two fonts.  Mis-specified OT table definitions could otherwise result
+    # in the presence of a table in the diff when the request was to exclude it.
+    # For example, when an "OS/2" table request is entered as "OS2".
+    if exclude_tables is not None:
+        for table in exclude_tables:
+            if table not in tt_left and table not in tt_right:
+                raise KeyError(
+                    f"'{table}' table was not identified for exclusion in either font"
+                )
+
     fromdate = get_file_modtime(filepath_a)
     todate = get_file_modtime(filepath_b)
 
     with tempfile.TemporaryDirectory() as tmpdirname:
-        tt_left.saveXML(os.path.join(tmpdirname, "left.ttx"))
-        tt_right.saveXML(os.path.join(tmpdirname, "right.ttx"))
+        tt_left.saveXML(
+            os.path.join(tmpdirname, "left.ttx"),
+            tables=include_tables,
+            skipTables=exclude_tables,
+        )
+        tt_right.saveXML(
+            os.path.join(tmpdirname, "right.ttx"),
+            tables=include_tables,
+            skipTables=exclude_tables,
+        )
 
         with open(os.path.join(tmpdirname, "left.ttx")) as ff:
             fromlines = ff.readlines()

tests/test_diff.py

@@ -9,6 +9,9 @@
 ROBOTO_UDIFF_EXPECTED_PATH = os.path.join("tests", "testfiles", "roboto_udiff_expected.txt")
 ROBOTO_UDIFF_COLOR_EXPECTED_PATH = os.path.join("tests", "testfiles", "roboto_udiff_color_expected.txt")
 ROBOTO_UDIFF_1CONTEXT_EXPECTED_PATH = os.path.join("tests", "testfiles", "roboto_udiff_1context_expected.txt")
+ROBOTO_UDIFF_HEADONLY_EXPECTED_PATH = os.path.join("tests", "testfiles", "roboto_udiff_headonly_expected.txt")
+ROBOTO_UDIFF_HEADPOSTONLY_EXPECTED_PATH = os.path.join("tests", "testfiles", "roboto_udiff_headpostonly_expected.txt")
+ROBOTO_UDIFF_EXCLUDE_HEADPOST_EXPECTED_PATH = os.path.join("tests", "testfiles", "roboto_udiff_ex_headpost_expected.txt")
 
 # Setup: define the expected diff text for unified diff
 with open(ROBOTO_UDIFF_EXPECTED_PATH, "r") as robo_udiff:
@@ -22,6 +25,18 @@
 with open(ROBOTO_UDIFF_1CONTEXT_EXPECTED_PATH, "r") as robo_udiff_contextlines:
     ROBOTO_UDIFF_1CONTEXT_EXPECTED = robo_udiff_contextlines.read()
 
+# Setup: define the expected diff text for head table only diff
+with open(ROBOTO_UDIFF_HEADONLY_EXPECTED_PATH, "r") as robo_udiff_headonly:
+    ROBOTO_UDIFF_HEADONLY_EXPECTED = robo_udiff_headonly.read()
+
+# Setup: define the expected diff text for head and post table only diff
+with open(ROBOTO_UDIFF_HEADPOSTONLY_EXPECTED_PATH, "r") as robo_udiff_headpostonly:
+    ROBOTO_UDIFF_HEADPOSTONLY_EXPECTED = robo_udiff_headpostonly.read()
+
+# Setup: define the expected diff text for head and post table only diff
+with open(ROBOTO_UDIFF_EXCLUDE_HEADPOST_EXPECTED_PATH, "r") as robo_udiff_ex_headpost:
+    ROBOTO_UDIFF_EXCLUDE_HEADPOST_EXPECTED = robo_udiff_ex_headpost.read()
+
 
 def test_unified_diff_default():
     res = u_diff(ROBOTO_BEFORE_PATH, ROBOTO_AFTER_PATH)
@@ -61,5 +76,68 @@ def test_unified_diff_context_lines_1():
             assert line == expected_string_list[x]
 
 
+def test_unified_diff_head_table_only():
+    res = u_diff(ROBOTO_BEFORE_PATH, ROBOTO_AFTER_PATH, include_tables=["head"])
+    res_string = "".join(res)
+    res_string_list = res_string.split("\n")
+    expected_string_list = ROBOTO_UDIFF_HEADONLY_EXPECTED.split("\n")
+
+    # have to handle the tests for the top two file path lines
+    # differently than the rest of the comparisons because
+    # the time is defined using local platform settings
+    # which makes tests fail on remote CI testing services vs.
+    # my local testing platform...
+    for x, line in enumerate(res_string_list):
+        # treat top two lines of the diff as comparison of first 10 chars only
+        if x in (0, 1):
+            assert line[0:9] == expected_string_list[x][0:9]
+        else:
+            assert line == expected_string_list[x]
+
+
+def test_unified_diff_headpost_table_only():
+    res = u_diff(ROBOTO_BEFORE_PATH, ROBOTO_AFTER_PATH, include_tables=["head", "post"])
+    res_string = "".join(res)
+    res_string_list = res_string.split("\n")
+    expected_string_list = ROBOTO_UDIFF_HEADPOSTONLY_EXPECTED.split("\n")
+
+    # have to handle the tests for the top two file path lines
+    # differently than the rest of the comparisons because
+    # the time is defined using local platform settings
+    # which makes tests fail on remote CI testing services vs.
+    # my local testing platform...
+    for x, line in enumerate(res_string_list):
+        # treat top two lines of the diff as comparison of first 10 chars only
+        if x in (0, 1):
+            assert line[0:9] == expected_string_list[x][0:9]
+        else:
+            assert line == expected_string_list[x]
+
+
+def test_unified_diff_exclude_headpost_tables():
+    res = u_diff(ROBOTO_BEFORE_PATH, ROBOTO_AFTER_PATH, exclude_tables=["head", "post"])
+    res_string = "".join(res)
+    res_string_list = res_string.split("\n")
+    expected_string_list = ROBOTO_UDIFF_EXCLUDE_HEADPOST_EXPECTED.split("\n")
+
+    # have to handle the tests for the top two file path lines
+    # differently than the rest of the comparisons because
+    # the time is defined using local platform settings
+    # which makes tests fail on remote CI testing services vs.
+    # my local testing platform...
+    for x, line in enumerate(res_string_list):
+        # treat top two lines of the diff as comparison of first 10 chars only
+        if x in (0, 1):
+            assert line[0:9] == expected_string_list[x][0:9]
+        else:
+            assert line == expected_string_list[x]
+
+
+def test_unified_diff_include_with_bad_table_definition():
+    with pytest.raises(KeyError):
+        u_diff(ROBOTO_BEFORE_PATH, ROBOTO_AFTER_PATH, include_tables=["bogus"])
 
 
+def test_unified_diff_exclude_with_bad_table_definition():
+    with pytest.raises(KeyError):
+        u_diff(ROBOTO_BEFORE_PATH, ROBOTO_AFTER_PATH, exclude_tables=["bogus"])