Repository: jupyter/nbdime (language: python)

Issue:
CM6: Several conflict gutter markers placed on the same line
When trying to run example6, the following issue has been observed in the merge editor: several conflict gutter markers are produced at the same line (case of line 6), not placed exactly at the same location.

![Screenshot from 2023-10-02 15-24-57](https://github.com/jupyter/nbdime/assets/99649086/a4f0386b-9d08-4119-b893-4d59fa23c7fa)



Discussion / hints:
I was able to reproduce it, specifically, it looks that there are two copies of the marker in line 5:

![image](https://github.com/jupyter/nbdime/assets/5832902/a2748516-9b6e-470e-a10b-7d57300bd7fc)

and the second copy overhangs to line 6.
Well, I am not sure if the issue is in the gutter code or in the chunking algorithm. In this case we get 6 conflict chunks:

![image](https://github.com/jupyter/nbdime/assets/5832902/5026caf4-84f8-4d01-9d28-6f3f3fb06413)

And two of them have `remoteFrom: 4` (so start at line 5, and the second of these leads to overhanging into line 6).

The also seem essentially identical:

| First copy | Second copy |
|--------|--------|
| ![Screenshot from 2023-10-20 22-54-47](https://github.com/jupyter/nbdime/assets/5832902/cdb4aace-942b-4667-a969-46b5fe05422e) |![Screenshot from 2023-10-20 22-54-38](https://github.com/jupyter/nbdime/assets/5832902/117f921a-f967-44c0-92b5-d7861f58ab6f) | 

Is there any case when duplicate chunks are expected, or is this an error in chunking code?
> Is there any case when duplicate chunks are expected, or is this an error in chunking code?

@vidartf is this what you meant by "In certain cases overlapping chunks are not being considered properly overlapping." in issue https://github.com/jupyter/nbdime/issues/677? In that case I think this issue just becomes a duplicate of #677.
There does seem to be a regression here compared to CM5 version. See this example diffing the files in ui-test/data/merge_test1:

CM5:
![image](https://github.com/jupyter/nbdime/assets/510760/5e97217c-d23a-40e3-adbb-4c125803b54a)

CM6:
![image](https://github.com/jupyter/nbdime/assets/510760/2634c8d1-abfc-4c53-98eb-0e35124c6939)

Notice the extra arrow on the base version on line 5. You can also see the same for CM6 here: 

https://github.com/jupyter/nbdime/blob/554c2d34882c77ce48e85586f5db8fce93ed80d1/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-take-a-snapshot-at-opening-1-linux.png

Yes, but it is not a bug in CM6 but a bug in how the chunks are calculated as per my comments above.
Of course it still should be fixed, which may be best approached by writing a test case, but it may require more thought/context to understand why the chunking algorithm sometimes leads to duplicate chunks; or we could just add a simple workaround of de-duplicating them without worrying about the algorithm being occasionally wrong.
To clarify, I am using CM5/CM6 as a shorthand for "current stable release" vs "current RC".
(and while there are for sure some bugs in the current stable version's mergeview chunking algorithm, the one I point out above is a new bug in the RC version)

Fix patch:
diff --git a/.prettierignore b/.prettierignore
index 373b6698..4e18772b 100644
--- a/.prettierignore
+++ b/.prettierignore
@@ -6,4 +6,5 @@
 **/.ipynb_checkpoints
 **/coverage
 CHANGELOG.md
-package-lock.json
\ No newline at end of file
+package-lock.json
+nbdime/tests/files/inline-conflict--decisions.json
diff --git a/packages/nbdime/src/common/mergeview.ts b/packages/nbdime/src/common/mergeview.ts
index 8e0fb10c..799c2838 100644
--- a/packages/nbdime/src/common/mergeview.ts
+++ b/packages/nbdime/src/common/mergeview.ts
@@ -295,21 +295,21 @@ const removeGutterMarkerEffect = StateEffect.define<{ type: string }>({
 class MergeMarker extends GutterMarker {
   constructor(options: { symbol: string; className: string; block: boolean }) {
     super();
-    this._symbol = options.symbol;
-    this._className = options.className;
-    this._block = options.block;
-  }
-  get isBlock() {
-    return this._block;
+    this.symbol = options.symbol;
+    this.className = options.className;
+    this.block = options.block;
   }
   toDOM() {
-    let pickerMarker = elt('div', this._symbol);
-    pickerMarker.className = this._className;
+    let pickerMarker = elt('div', this.symbol);
+    pickerMarker.className = this.className;
     return pickerMarker;
   }
-  private _symbol: string;
-  private _className: string;
-  private _block: boolean;
+  eq(other: GutterMarker): boolean {
+    return other === this;
+  }
+  readonly symbol: string;
+  readonly className: string;
+  readonly block: boolean;
 }
 
 /**
@@ -332,7 +332,18 @@ const gutterMarkerField = StateField.define<RangeSet<MergeMarker>>({
               : e.value.block
               ? conflictBlockMarker
               : conflictMarker;
-          gutters = gutters.update({ add: [marker.range(e.value.from)] });
+          // check for overlap (duplicates) with same type
+          let overlap = false;
+          gutters.between(e.value.from, e.value.from, (from, to, value) => {
+            if (from === e.value.from && value.eq(marker)) {
+              overlap = true;
+              return false;
+            }
+            return;
+          });
+          if (!overlap) {
+            gutters = gutters.update({ add: [marker.range(e.value.from)] });
+          }
         }
       }
       if (e.is(removeGutterMarkerEffect)) {
@@ -1417,7 +1428,7 @@ export class MergeView extends Panel {
         class: 'cm-gutter',
         markers: view => {
           return view.state.field(gutterMarkerField).update({
-            filter: (_from, _to, value: MergeMarker) => !value.isBlock,
+            filter: (_from, _to, value: MergeMarker) => !value.block,
           });
         },
         widgetMarker: (
@@ -1430,7 +1441,7 @@ export class MergeView extends Panel {
           }
           const markers = view.state.field(gutterMarkerField).update({
             filter: (from, _to, value: MergeMarker) =>
-              value.isBlock && block.from === from,
+              value.block && block.from === from,
           });
           if (markers.size > 1) {
             throw Error('More than one block gutter widget matched');
diff --git a/packages/nbdime/src/merge/model/cell.ts b/packages/nbdime/src/merge/model/cell.ts
index dfa1d798..555fb428 100644
--- a/packages/nbdime/src/merge/model/cell.ts
+++ b/packages/nbdime/src/merge/model/cell.ts
@@ -269,7 +269,12 @@ export class CellMergeModel extends ObjectMergeModel<
    */
   get agreedCell(): boolean {
     // TODO: Also check other fields?
-    return this.agreedSource && this.agreedMetadata && this.agreedOutputs && this.agreedIds;
+    return (
+      this.agreedSource &&
+      this.agreedMetadata &&
+      this.agreedOutputs &&
+      this.agreedIds
+    );
   }
 
   /**
diff --git a/packages/nbdime/src/styles/merge.css b/packages/nbdime/src/styles/merge.css
index efc1cda8..7c48ff41 100644
--- a/packages/nbdime/src/styles/merge.css
+++ b/packages/nbdime/src/styles/merge.css
@@ -253,13 +253,13 @@
   .cm-merge-m-chunk-end-either-empty::after
   + .cm-linewidget
   .cm-merge-spacer {
-    content: '';
-    width: 100%;
-    height: 1px;
-    background: var(--jp-merge-either-color1);
-    display: block;
-    position: absolute;
-    left: 0px;
+  content: '';
+  width: 100%;
+  height: 1px;
+  background: var(--jp-merge-either-color1);
+  display: block;
+  position: absolute;
+  left: 0px;
 }
 
 .jp-Notebook-merge .cm-central-editor .cm-line .cm-merge-m-deleted,
diff --git a/packages/webapp/src/app/merge.ts b/packages/webapp/src/app/merge.ts
index 173c0b38..1fa2441e 100644
--- a/packages/webapp/src/app/merge.ts
+++ b/packages/webapp/src/app/merge.ts
@@ -183,23 +183,26 @@ function onPopState(e: PopStateEvent) {
  */
 function onMergeRequestCompleted(data: any) {
   let layoutWork = showMerge(data);
-  layoutWork.then(() => {
-    toggleSpinner(false);
-    markUnchangedRanges();
-  }, reason => {
-    console.log("Failed to show merge result");
-    let root = document.getElementById('nbdime-root');
-    if (!root) {
-      throw new Error('Missing root element "nbidme-root"');
-    }
-    const pre = document.createElement('pre');
-    pre.innerText = reason || "Error occurred displaying merge!";
-    pre.classList.add("jp-mergeapp-error");
-    // we might have a partial render, so leave that in case it is useful
-    root.prepend(pre);
-    mergeWidget = null;
-    toggleSpinner(false);
-  });
+  layoutWork.then(
+    () => {
+      toggleSpinner(false);
+      markUnchangedRanges();
+    },
+    reason => {
+      console.log('Failed to show merge result');
+      let root = document.getElementById('nbdime-root');
+      if (!root) {
+        throw new Error('Missing root element "nbidme-root"');
+      }
+      const pre = document.createElement('pre');
+      pre.innerText = reason || 'Error occurred displaying merge!';
+      pre.classList.add('jp-mergeapp-error');
+      // we might have a partial render, so leave that in case it is useful
+      root.prepend(pre);
+      mergeWidget = null;
+      toggleSpinner(false);
+    },
+  );
 }
 
 /**


Test patch:
diff --git a/ui-tests/tests/nbdime-merge-test1.spec.ts b/ui-tests/tests/nbdime-merge-test1.spec.ts
index 0935cd33..2f4544ee 100644
--- a/ui-tests/tests/nbdime-merge-test1.spec.ts
+++ b/ui-tests/tests/nbdime-merge-test1.spec.ts
@@ -11,7 +11,7 @@ test.describe('merge test1', () => {
   });
 
   test('take a snapshot at opening', async ({ page }) => {
-    await expect.soft(page.getByText('➭')).toHaveCount(12);
+    await expect.soft(page.getByText('➭')).toHaveCount(11);
 
     await expect.soft(page.locator('#nbdime-header-base')).toHaveText('Base');
 
@@ -40,7 +40,7 @@ test.describe('merge test1', () => {
       .locator('.cm-central-editor')
       .nth(1) // This select the cell; 0 being the notebook metadata
       .locator('.jp-Merge-gutter-picker')
-      .nth(2)
+      .nth(1)
       .click();
     await page.getByText('⚠').click();
     expect(await page.locator('#main').screenshot()).toMatchSnapshot();
diff --git a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-central-version-for-conflict-1-linux.png b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-central-version-for-conflict-1-linux.png
index ab4322f2..6750a8d9 100644
Binary files a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-central-version-for-conflict-1-linux.png and b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-central-version-for-conflict-1-linux.png differ
diff --git a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-central-version-for-conflict-1-win32.png b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-central-version-for-conflict-1-win32.png
index e7dabd62..aa523049 100644
Binary files a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-central-version-for-conflict-1-win32.png and b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-central-version-for-conflict-1-win32.png differ
diff --git a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-left-version-for-conflict-1-linux.png b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-left-version-for-conflict-1-linux.png
index 68f5c76d..2f65d617 100644
Binary files a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-left-version-for-conflict-1-linux.png and b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-left-version-for-conflict-1-linux.png differ
diff --git a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-left-version-for-conflict-1-win32.png b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-left-version-for-conflict-1-win32.png
index 8fdacdfb..5d2b050e 100644
Binary files a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-left-version-for-conflict-1-win32.png and b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-left-version-for-conflict-1-win32.png differ
diff --git a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-right-version-for-conflict-1-linux.png b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-right-version-for-conflict-1-linux.png
index 8d4c14c3..cb484099 100644
Binary files a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-right-version-for-conflict-1-linux.png and b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-right-version-for-conflict-1-linux.png differ
diff --git a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-right-version-for-conflict-1-win32.png b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-right-version-for-conflict-1-win32.png
index d99e67d4..993c518b 100644
Binary files a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-right-version-for-conflict-1-win32.png and b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-choose-right-version-for-conflict-1-win32.png differ
diff --git a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-take-a-snapshot-at-opening-1-linux.png b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-take-a-snapshot-at-opening-1-linux.png
index 949aae7e..57beb790 100644
Binary files a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-take-a-snapshot-at-opening-1-linux.png and b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-take-a-snapshot-at-opening-1-linux.png differ
diff --git a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-take-a-snapshot-at-opening-1-win32.png b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-take-a-snapshot-at-opening-1-win32.png
index 09b45ab1..fa631915 100644
Binary files a/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-take-a-snapshot-at-opening-1-win32.png and b/ui-tests/tests/nbdime-merge-test1.spec.ts-snapshots/merge-test1-take-a-snapshot-at-opening-1-win32.png differ
diff --git a/ui-tests/tests/nbdime-merge-test2.spec.ts b/ui-tests/tests/nbdime-merge-test2.spec.ts
index b8b77976..ddea8e25 100644
--- a/ui-tests/tests/nbdime-merge-test2.spec.ts
+++ b/ui-tests/tests/nbdime-merge-test2.spec.ts
@@ -11,7 +11,7 @@ test.beforeEach(async ({ page }) => {
 /* notebooks of same length with 0 conflict*/
 test.describe('merge test2 ', () => {
   test('take a snapshot at opening', async ({ page }) => {
-    await expect.soft(page.getByText('➭')).toHaveCount(12);
+    await expect.soft(page.getByText('➭')).toHaveCount(11);
     expect(await page.locator('#main').screenshot()).toMatchSnapshot();
   });
 
@@ -30,7 +30,7 @@ test.describe('merge test2 ', () => {
       .locator('.cm-central-editor')
       .nth(1) // This select the cell; 0 being the notebook metadata
       .locator('.jp-Merge-gutter-picker')
-      .nth(2)
+      .nth(1)
       .click();
     expect(await page.locator('#main').screenshot()).toMatchSnapshot();
   });
diff --git a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-central-version-1-linux.png b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-central-version-1-linux.png
index c1128624..b1e39b21 100644
Binary files a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-central-version-1-linux.png and b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-central-version-1-linux.png differ
diff --git a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-central-version-1-win32.png b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-central-version-1-win32.png
index 1e93170a..dcf517d8 100644
Binary files a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-central-version-1-win32.png and b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-central-version-1-win32.png differ
diff --git a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-left-version-1-linux.png b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-left-version-1-linux.png
index e3167406..6963048c 100644
Binary files a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-left-version-1-linux.png and b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-left-version-1-linux.png differ
diff --git a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-left-version-1-win32.png b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-left-version-1-win32.png
index 4c86a6d7..85a88f77 100644
Binary files a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-left-version-1-win32.png and b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-left-version-1-win32.png differ
diff --git a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-right-version-1-linux.png b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-right-version-1-linux.png
index e3167406..6963048c 100644
Binary files a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-right-version-1-linux.png and b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-right-version-1-linux.png differ
diff --git a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-right-version-1-win32.png b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-right-version-1-win32.png
index 4c86a6d7..85a88f77 100644
Binary files a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-right-version-1-win32.png and b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-choose-right-version-1-win32.png differ
diff --git a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-take-a-snapshot-at-opening-1-linux.png b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-take-a-snapshot-at-opening-1-linux.png
index e3167406..6963048c 100644
Binary files a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-take-a-snapshot-at-opening-1-linux.png and b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-take-a-snapshot-at-opening-1-linux.png differ
diff --git a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-take-a-snapshot-at-opening-1-win32.png b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-take-a-snapshot-at-opening-1-win32.png
index 4c86a6d7..85a88f77 100644
Binary files a/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-take-a-snapshot-at-opening-1-win32.png and b/ui-tests/tests/nbdime-merge-test2.spec.ts-snapshots/merge-test2-take-a-snapshot-at-opening-1-win32.png differ
diff --git a/ui-tests/tests/nbdime-merge-test3.spec.ts b/ui-tests/tests/nbdime-merge-test3.spec.ts
index ea2f52bf..6757b059 100644
--- a/ui-tests/tests/nbdime-merge-test3.spec.ts
+++ b/ui-tests/tests/nbdime-merge-test3.spec.ts
@@ -11,7 +11,7 @@ test.beforeEach(async ({ page }) => {
 /* 2 cells with merge conflict */
 test.describe('merge test3', () => {
   test('should warn for remaining conflicts', async ({ page }) => {
-    await expect.soft(page.getByText('➭')).toHaveCount(26);
+    await expect.soft(page.getByText('➭')).toHaveCount(25);
 
     await page.getByRole('button', { name: 'Download' }).click();