Merge pull request #491 from dotmesh-io/addrange-removerange-conflict…

…-take-2 Handle addrange/removerange conflicts by splitting.
jupyter · Aug 7, 2019 · cdadb44 · cdadb44
2 parents ec065a7 + 0810fa3
commit cdadb44
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 0 deletions.
diff --git a/packages/nbdime/src/merge/model/notebook.ts b/packages/nbdime/src/merge/model/notebook.ts
@@ -313,6 +313,27 @@ function splitCellChunks(mergeDecisions: MergeDecision[]): MergeDecision[] {
           'remote', // Check for custom action first?
           md.conflict,
         ));
+      } else if (hasEntries(md.remoteDiff) && hasEntries(md.localDiff)) {
+        const ops = [md.remoteDiff[0].op, md.localDiff[0].op].sort();
+        if (ops.join(',') === 'addrange,removerange') {
+          // Insertion and deletions on the same index are simply split
+          // but both keep the conflict status
+
+          // Just do local first (alt. do add first)
+          let lmd = new MergeDecision(md);
+          lmd.action = 'local';
+          lmd.localDiff = md.localDiff.slice();
+          lmd.remoteDiff = null;
+          output.push(lmd);
+
+          let rmd = new MergeDecision(md);
+          rmd.action = 'remote';
+          rmd.localDiff = null;
+          rmd.remoteDiff = md.remoteDiff.slice();
+          output.push(rmd);
+        } else {
+          output.push(md);  // deepCopy?
+        }
       } else {
         output.push(md);  // deepCopy?
       }

diff --git a/packages/nbdime/test/src/merge/model.spec.ts b/packages/nbdime/test/src/merge/model.spec.ts
@@ -525,6 +525,58 @@ describe('merge', () => {
           expect(stripSource(d.remoteDiff)).to.eql([opAddRange(1, 'line #2\n')]);
         });
 
+        it('should split an ADDRANGE/REMOVERANGE conflict', () => {
+          const lineAdd = opPatch('source', [opAddRange(1, 'line #2\n')]);
+          let cdecs: MergeDecision[] = [new MergeDecision(
+            ['cells'],
+            [opAddRange(0, [cell1])],
+            [opRemoveRange(0, 1)],
+            'local_then_remote',
+            true
+          )];
+          let model = new NotebookMergeModel(notebook, cdecs);
+
+          expect(model.decisions.length).to.be(2);
+          let d = model.decisions[0];
+          expect(d.action).to.be('local');
+          expect(d.conflict).to.be(true);
+          expect(stripSource(d.localDiff)).to.eql([opAddRange(0, [cell1])]);
+          expect(d.remoteDiff).to.be(null);
+
+          d = model.decisions[1];
+          expect(d.action).to.be('remote');
+          expect(d.conflict).to.be(true);
+          expect(d.localDiff).to.be(null);
+          expect(stripSource(d.remoteDiff)).to.eql([opRemoveRange(0, 1)]);
+
+        });
+
+        it('should split an REMOVERANGE/ADDRANGE conflict', () => {
+          const lineAdd = opPatch('source', [opAddRange(1, 'line #2\n')]);
+          let cdecs: MergeDecision[] = [new MergeDecision(
+            ['cells'],
+            [opRemoveRange(0, 1)],
+            [opAddRange(0, [cell1])],
+            'local_then_remote',
+            true
+          )];
+          let model = new NotebookMergeModel(notebook, cdecs);
+
+          expect(model.decisions.length).to.be(2);
+          let d = model.decisions[0];
+          expect(d.action).to.be('remote');
+          expect(d.conflict).to.be(true);
+          expect(d.localDiff).to.be(null);
+          expect(stripSource(d.remoteDiff)).to.eql([opAddRange(0, [cell1])]);
+
+          d = model.decisions[1];
+          expect(d.action).to.be('local');
+          expect(d.conflict).to.be(true);
+          expect(stripSource(d.localDiff)).to.eql([opRemoveRange(0, 1)]);
+          expect(d.remoteDiff).to.be(null);
+
+        });
+
       });
 
     });