Fix the ogd diff function (#56)

* Improve odg diff mechanism
* Add text diff by default, with options for data and internal variants
* Fixup bugs and add testing

Author: sveinse

setup.cfg

@@ -31,7 +31,7 @@ python_requires = >=3.10, <4
 install_requires =
     jsonschema
     colorama
-    deepdiff<8.0.0
+    deepdiff
 
 [options.packages.find]
 where = src

src/objdictgen/__main__.py

@@ -23,16 +23,15 @@
 import logging
 import sys
 from dataclasses import dataclass, field
-from pprint import pformat
-from typing import TYPE_CHECKING, Callable, Generator, Sequence, TypeVar
+from typing import Callable, Sequence, TypeVar
 
 from colorama import Fore, Style, init
 
 import objdictgen
 from objdictgen import jsonod
 from objdictgen.node import Node
-from objdictgen.printing import format_node
-from objdictgen.typing import TDiffEntries, TDiffNodes, TPath
+from objdictgen.printing import format_diff_nodes, format_node
+from objdictgen.typing import TPath
 
 T = TypeVar('T')
 
@@ -88,38 +87,6 @@ def open_od(fname: TPath|str, validate=True, fix=False) -> "Node":
         raise
 
 
-def print_diffs(diffs: TDiffNodes, show=False):
-    """ Print the differences between two object dictionaries"""
-
-    def _pprint(text: str):
-        for line in pformat(text).splitlines():
-            print("       ", line)
-
-    def _printlines(entries: TDiffEntries):
-        for chtype, change, path in entries:
-            if 'removed' in chtype:
-                print(f"<<<     {path} only in LEFT")
-                if show:
-                    _pprint(change.t1)
-            elif 'added' in chtype:
-                print(f"    >>> {path} only in RIGHT")
-                if show:
-                    _pprint(change.t2)
-            elif 'changed' in chtype:
-                print(f"<< - >> {path} value changed from '{change.t1}' to '{change.t2}'")
-            else:
-                print(f"{Fore.RED}{chtype} {path} {change}{Style.RESET_ALL}")
-
-    rest = diffs.pop('', None)
-    if rest:
-        print(f"{Fore.GREEN}Changes:{Style.RESET_ALL}")
-        _printlines(rest)
-
-    for index in sorted(diffs):
-        print(f"{Fore.GREEN}Index 0x{index:04x} ({index}){Style.RESET_ALL}")
-        _printlines(diffs[index])
-
-
 @debug_wrapper()
 def main(debugopts: DebugOpts, args: Sequence[str]|None = None):
     """ Main command dispatcher """
@@ -144,6 +111,7 @@ def main(debugopts: DebugOpts, args: Sequence[str]|None = None):
     opt_debug = dict(action='store_true', help="Debug: enable tracebacks on errors")
     opt_od = dict(metavar='od', default=None, help="Object dictionary")
     opt_novalidate = dict(action='store_true', help="Don't validate input files")
+    opt_nocolor = dict(action='store_true', help="Disable colored output")
 
     parser.add_argument('--version', action='version', version='%(prog)s ' + objdictgen.__version__)
     parser.add_argument('--no-color', action='store_true', help="Disable colored output")
@@ -183,11 +151,13 @@ def main(debugopts: DebugOpts, args: Sequence[str]|None = None):
     """, aliases=['compare'])
     subp.add_argument('od1', **opt_od)  # type: ignore[arg-type]
     subp.add_argument('od2', **opt_od)  # type: ignore[arg-type]
+    subp.add_argument('--show', action="store_true", help="Show difference data")
     subp.add_argument('--internal', action="store_true", help="Diff internal object")
+    subp.add_argument('--data', action="store_true", help="Show difference as data")
+    subp.add_argument('--raw', action="store_true", help="Show raw difference")
+    subp.add_argument('--no-color', **opt_nocolor)  # type: ignore[arg-type]
     subp.add_argument('--novalidate', **opt_novalidate)  # type: ignore[arg-type]
-    subp.add_argument('--show', action="store_true", help="Show difference data")
     subp.add_argument('-D', '--debug', **opt_debug)  # type: ignore[arg-type]
-    subp.add_argument('--no-color', action='store_true', help="Disable colored output")
 
     # -- EDIT --
     subp = subparser.add_parser('edit', help="""
@@ -210,9 +180,9 @@ def main(debugopts: DebugOpts, args: Sequence[str]|None = None):
     subp.add_argument('--short', action="store_true", help="Do not list sub-index")
     subp.add_argument('--unused', action="store_true", help="Include unused profile parameters")
     subp.add_argument('--internal', action="store_true", help="Show internal data")
-    subp.add_argument('-D', '--debug', **opt_debug)  # type: ignore[arg-type]
+    subp.add_argument('--no-color', **opt_nocolor)  # type: ignore[arg-type]
     subp.add_argument('--novalidate', **opt_novalidate)  # type: ignore[arg-type]
-    subp.add_argument('--no-color', action='store_true', help="Disable colored output")
+    subp.add_argument('-D', '--debug', **opt_debug)  # type: ignore[arg-type]
 
     # -- NETWORK --
     subp = subparser.add_parser('network', help="""
@@ -306,22 +276,22 @@ def main(debugopts: DebugOpts, args: Sequence[str]|None = None):
 
     # -- DIFF command --
     elif opts.command in ("diff", "compare"):
+
         od1 = open_od(opts.od1, validate=not opts.novalidate)
         od2 = open_od(opts.od2, validate=not opts.novalidate)
 
-        diffs = jsonod.diff_nodes(
-            od1, od2, asdict=not opts.internal,
-            validate=not opts.novalidate,
-        )
+        lines = list(format_diff_nodes(od1, od2, data=opts.data, raw=opts.raw,
+                             internal=opts.internal, show=opts.show))
+
+        for line in lines:
+            print(line)
 
-        if diffs:
-            errcode = 1
+        errcode = 1 if lines else 0
+        if errcode:
             print(f"{objdictgen.ODG_PROGRAM}: '{opts.od1}' and '{opts.od2}' differ")
         else:
-            errcode = 0
             print(f"{objdictgen.ODG_PROGRAM}: '{opts.od1}' and '{opts.od2}' are equal")
 
-        print_diffs(diffs, show=opts.show)
         if errcode:
             parser.exit(errcode)
 

src/objdictgen/jsonod.py

@@ -17,6 +17,8 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301
 # USA
 
+from __future__ import annotations
+
 import copy
 import json
 import logging
@@ -36,7 +38,7 @@
 from objdictgen.typing import (TDiffNodes, TIndexEntry, TODJson, TODObjJson,
                                TODObj, TODSubObj, TODSubObjJson, TODValue, TParamEntry, TPath, TProfileMenu)
 from objdictgen.utils import (copy_in_order, exc_amend, maybe_number,
-                              str_to_int)
+                              str_to_int, strip_brackets)
 
 T = TypeVar('T')
 M = TypeVar('M', bound=Mapping)
@@ -173,6 +175,10 @@ class ValidationError(Exception):
 # Copied from https://github.com/NickolaiBeloguzov/jsonc-parser/blob/master/jsonc_parser/parser.py#L11-L39
 RE_JSONC = re.compile(r"(\".*?(?<!\\)\"|\'.*?\')|(\s*/\*.*?\*/\s*|\s*//[^\r\n]*$)", re.MULTILINE | re.DOTALL)
 
+# Regexs to handle parsing of diffing the JSON
+RE_DIFF_ROOT = re.compile(r"^(root(\[.*?\]))(.*)")
+RE_DIFF_INDEX = re.compile(r"\['dictionary'\]\[(\d+)\](.*)")
+
 
 def remove_jsonc(text: str) -> str:
     """ Remove jsonc annotations """
@@ -1394,67 +1400,64 @@ def _validate_dictionary(index, obj):
             raise
 
 
-def diff_nodes(node1: "Node", node2: "Node", asdict=True, validate=True) -> TDiffNodes:
+def diff(node1: Node, node2: Node, internal=False) -> TDiffNodes:
     """Compare two nodes and return the differences."""
 
     diffs: dict[int|str, list] = {}
 
-    if asdict:
-        jd1 = node_todict(node1, sort=True, validate=validate)
-        jd2 = node_todict(node2, sort=True, validate=validate)
+    if internal:
 
-        dt = datetime.isoformat(datetime.now())
-        jd1['$date'] = jd2['$date'] = dt
+        # Simply diff the python data structure for the nodes
+        diff = deepdiff.DeepDiff(node1.__dict__, node2.__dict__, exclude_paths=[
+            "IndexOrder"
+        ], view='tree')
 
-        # DeepDiff does not have typing, but the diff object is a dict-like object
-        #   DeepDiff[str, deepdiff.model.PrettyOrderedSet].
-        # PrettyOrderedSet is a list-like object
-        #   PrettyOrderedSet[deepdiff.model.DiffLevel]
+    else:
 
-        diff = deepdiff.DeepDiff(jd1, jd2, exclude_paths=[
-            "root['dictionary']"
-        ], view='tree')
+        # Don't use rich format for diffing, as it will contain comments which confuse the output
+        jd1 = node_todict(node1, sort=True, rich=False, internal=True)
+        jd2 = node_todict(node2, sort=True, rich=False, internal=True)
 
-        chtype: str
-        for chtype, changes in diff.items():
-            change: deepdiff.model.DiffLevel
-            for change in changes:
-                path: str = change.path(force='fake')  # pyright: ignore[reportAssignmentType]
-                entries = diffs.setdefault('', [])
-                entries.append((chtype, change, path.replace('root', '')))
-
-        diff = deepdiff.DeepDiff(jd1['dictionary'], jd2['dictionary'], view='tree', group_by='index')
-
-        res = re.compile(r"root\[('0x[0-9a-fA-F]+'|\d+)\]")
-
-        for chtype, changes in diff.items():
-            for change in changes:
-                path = change.path(force='fake')  # pyright: ignore[reportAssignmentType]
-                m = res.search(path)
-                if m:
-                    num = str_to_int(m.group(1).strip("'"))
-                    entries = diffs.setdefault(num, [])
-                    entries.append((chtype, change, path.replace(m.group(0), '')))
-                else:
-                    entries = diffs.setdefault('', [])
-                    entries.append((chtype, change, path.replace('root', '')))
+        # Convert the dictionary list to a dict to ensure the order of the objects
+        jd1["dictionary"] = {obj["index"]: obj for obj in jd1["dictionary"]}
+        jd2["dictionary"] = {obj["index"]: obj for obj in jd2["dictionary"]}
 
-    else:
-        diff = deepdiff.DeepDiff(node1.__dict__, node2.__dict__, exclude_paths=[
-            "root.IndexOrder"
-        ], view='tree')
+        # Diff the two nodes in json object format
+        diff = deepdiff.DeepDiff(jd1, jd2, view='tree')
+
+    # Iterate over the changes
+    for chtype, changes in diff.items():
+        for change in changes:
+            path = change.path()
+
+            # Match the root[<obj>]... part of the path
+            m = RE_DIFF_ROOT.match(path)
+            if not m:
+                raise ValueError(f"Unexpected path '{path}' in compare")
 
-        res = re.compile(r"root\.(Profile|Dictionary|ParamsDictionary|UserMapping|DS302)\[(\d+)\]")
+            # Path is the display path, root the categorization
+            path = m[2] + m[3]
+            root = m[2]
+
+            if not internal:
+                if m[1] == "root['dictionary']":
+                    # Extract the index from the path
+                    m = RE_DIFF_INDEX.match(path)
+                    root = f"Index {m[1]}"
+                    path = m[2]
 
-        for chtype, changes in diff.items():
-            for change in changes:
-                path = change.path(force='fake')  # pyright: ignore[reportAssignmentType]
-                m = res.search(path)
-                if m:
-                    entries = diffs.setdefault(int(m.group(2)), [])
-                    entries.append((chtype, change, path.replace(m.group(0), m.group(1))))
                 else:
-                    entries = diffs.setdefault('', [])
-                    entries.append((chtype, change, path.replace('root.', '')))
+                    root = "Header fields"
+
+            # Append the change to the list of changes
+            entries = diffs.setdefault(strip_brackets(root), [])
+            entries.append((chtype, change, strip_brackets(path)))
+
+    # Ensure the Index entries are sorted correctly
+    def _sort(text):
+        if text.startswith("Index "):
+            return f"zz 0x{int(text[6:]):04x}"
+        return text
 
-    return diffs
+    # Sort the entries
+    return {k: diffs[k] for k in sorted(diffs, key=_sort)}