Merge pull request #25 from martinal/martinal/topic-add-merge-strategies

Add merge strategies sketch

nbdime/diff_format.py

@@ -245,6 +245,16 @@ def to_diffentry_dicts(di):  # TODO: Better name, validate_diff? as_diff?
         return di
 
 
+def as_dict_based_diff(di):
+    """Converting to dict-based diff format for dicts for convenience.
+
+    NB! Only one level, not recursive.
+
+    This step will be unnecessary if we change the diff format to work this way always.
+    """
+    return {e.key: e for e in di}
+
+
 def to_json_patch(d, path=""):
     """Convert nbdime diff object into the RFC6902 JSON Patch format.
 

nbdime/merging/generic.py

@@ -11,14 +11,13 @@
 from collections import namedtuple
 
 from ..diffing import diff
-from ..diff_format import Diff, SequenceDiff, MappingDiff
+from ..diff_format import Diff, SequenceDiff, MappingDiff, as_dict_based_diff
 from ..patching import patch
 
 
 # Set to true to enable some expensive debugging assertions
 DEBUGGING = 0
 
-
 # Sentinel to allow None value
 Missing = object()
 
@@ -31,8 +30,8 @@ def _merge_dicts(base, local, remote, base_local_diff, base_remote_diff):
 
     # Converting to dict-based diff format for dicts for convenience
     # This step will be unnecessary if we change the diff format to work this way always
-    base_local_diff = {e.key: e for e in base_local_diff}
-    base_remote_diff = {e.key: e for e in base_remote_diff}
+    base_local_diff = as_dict_based_diff(base_local_diff)
+    base_remote_diff = as_dict_based_diff(base_remote_diff)
 
     # Summary of diff entry cases with (#) references to below code
     # r\l | N/A   -   +   :   !

nbdime/merging/notebooks.py

@@ -8,12 +8,62 @@
 import nbformat
 
 from .generic import merge
+from ..diff_format import as_dict_based_diff
 
-def merge_notebooks(base, local, remote):
+
+# Strategies for handling conflicts  TODO: Implement these and refine further!
+generic_conflict_strategies = ("mergetool", "use-base", "use-local", "use-remote")
+source_conflict_strategies = generic_conflict_strategies + ("add-markers",)
+transient_conflict_strategies = generic_conflict_strategies + ("invalidate",)
+output_conflict_strategies = transient_conflict_strategies + ("use-all",)
+
+
+def autoresolve_notebook_conflicts(merged, local_conflict_diffs, remote_conflict_diffs, args):
+    assert isinstance(merged, dict)
+
+    # Converting to dict-based diff format for dicts for convenience
+    # This step will be unnecessary if we change the diff format to work this way always
+    lcd = as_dict_based_diff(local_conflict_diffs)
+    rcd = as_dict_based_diff(remote_conflict_diffs)
+    # FIXME: Step through nbformat docs and handle case by case
+
+
+    strategy = args.strategy
+    if strategy not in generic_conflict_strategies:
+        raise ValueError("Invalid strategy {}".format(strategy))
+
+    # TODO: We want to be a lot more sophisticated than this, e.g.
+    #   setting different strategies for source, output, metadata etc.
+    #   However this is illustrative and was quick and easy to implement.
+
+    if strategy == "mergetool":
+        pass
+    elif strategy == "use-base":
+        local_conflict_diffs = []
+        remote_conflict_diffs = []
+    elif strategy == "use-local":
+        merged = patch(merged, local_conflict_diffs)
+        local_conflict_diffs = []
+        remote_conflict_diffs = []
+    elif strategy == "use-remote":
+        merged = patch(merged, remote_conflict_diffs)
+        local_conflict_diffs = []
+        remote_conflict_diffs = []
+
+    return merged, local_conflict_diffs, remote_conflict_diffs
+
+
+def merge_notebooks(base, local, remote, args):
     """Merge changes introduced by notebooks local and remote from a shared ancestor base.
 
     Return new (partially) merged notebook and unapplied diffs from the local and remote side.
     """
+    # Execute a generic merge operation
     merged, local_conflict_diffs, remote_conflict_diffs = merge(base, local, remote)
     merged = nbformat.from_dict(merged)
+
+    # Try to resolve conflicts based on behavioural options
+    #merged, local_conflict_diffs, remote_conflict_diffs = \
+    #  autoresolve_notebook_conflicts(merged, local_conflict_diffs, remote_conflict_diffs, args)
+
     return merged, local_conflict_diffs, remote_conflict_diffs

nbdime/nbmergeapp.py

@@ -38,7 +38,12 @@ def main_merge(args):
     #l = split_lines(l)
     #r = split_lines(r)
 
-    m, lc, rc = merge_notebooks(b, l, r)
+    m, lc, rc = merge_notebooks(b, l, r, args)
+
+    if lc or rc:
+        print("Conflicts occured during merge operation.")
+    else:
+        print("Merge completed successfully with no conflicts.")
 
     if mfn:
         if lc or rc:
@@ -55,7 +60,7 @@ def main_merge(args):
             with open(mfn, "w") as mf:
                 nbformat.write(m, mf)
     else:
-        print(m)
+        # FIXME: Display conflicts in a useful way
         if lc or rc:
             print("Local conflicts:")
             print(lc)
@@ -76,6 +81,10 @@ def _build_arg_parser():
     add_diff_args(parser)
 
     # TODO: Define sensible strategy variables and implement
+    from .merging.notebooks import generic_conflict_strategies
+    parser.add_argument('-s', '--strategy',
+                        default="mergetool", choices=generic_conflict_strategies,
+                        help="Specify the merge strategy to use.")
     #parser.add_argument('-m', '--merge-strategy',
     #                    default="default", choices=("foo", "bar"),
     #                    help="Specify the merge strategy to use.")

nbdime/tests/test_merge_notebooks.py

@@ -59,7 +59,8 @@ def test_merge_cell_sources_neighbouring_inserts():
         "    return y + 2",
         ],
         ])
-    actual, lco, rco = merge_notebooks(sources_to_notebook(base), sources_to_notebook(local), sources_to_notebook(remote))
+    args = None
+    actual, lco, rco = merge_notebooks(sources_to_notebook(base), sources_to_notebook(local), sources_to_notebook(remote), args)
     assert not lco
     assert not rco
     assert actual == expected
@@ -106,7 +107,8 @@ def test_merge_cell_sources_separate_inserts():
         "print(f(7))",
         ],
         ])
-    actual, lco, rco = merge_notebooks(sources_to_notebook(base), sources_to_notebook(local), sources_to_notebook(remote))
+    args = None
+    actual, lco, rco = merge_notebooks(sources_to_notebook(base), sources_to_notebook(local), sources_to_notebook(remote), args)
     assert not lco
     assert not rco
     assert actual == expected