Fix replace when replacement text matches source text multiple times (#…

…16258)

* Revert "Alternate description for disabled filters"

This reverts commit 059fb7e.

* Revert "Revert "Alternate description for disabled filters""

This reverts commit 27ed1d0.

* New test cases for replacement text that matches source text multiple times

* WIP: Advances CodeMirror highlight

* Simplifies highlight moving logic

* Explicitly ignores promise

* Reinstates prior code

* Moves to next cell if we're at the end of the current one

* WIP: Fixes double-match problem, breaks no-match case

* Simplifies loop logic, avoiding if case

* Removes commented out code

* Reinstates highlight advancement

* Reset current search provider's position before switching to another

* Promise lint fix

galata/test/jupyterlab/notebook-replace.test.ts

@@ -182,4 +182,80 @@ test.describe('Notebook Search and Replace', () => {
     await page.click('button:has-text("Replace")');
     await page.locator('text=1/4').waitFor();
   });
+
+  test('Replace with a string containing the query string twice', async ({
+    page
+  }) => {
+    // Create a small test notebook
+    await page.notebook.createNew();
+    await page.notebook.setCell(0, 'code', 'test\ntest');
+    await page.notebook.addCell('code', 'test\ntest');
+
+    await page.keyboard.press('Control+f');
+    await page.fill('[placeholder="Find"]', 'test');
+
+    await page.click('button[title="Show Replace"]');
+    await page.fill('[placeholder="Replace"]', 'testtest');
+
+    // TODO: Next Match press count should be one less
+    // (the -/4 state should not be necessary).
+    await page.locator('text=-/4').waitFor();
+    await page.click('button[title^="Next Match"]', {
+      clickCount: 3
+    });
+
+    await page.locator('text=1/4').waitFor();
+    await page.click('button:has-text("Replace")');
+
+    // We should move to the first match after the first replacement.
+    await page.locator('text=3/5').waitFor();
+    await page.click('button:has-text("Replace")');
+
+    // At this point we should be in the second cell
+    await page.locator('text=5/6').waitFor();
+    await page.click('button:has-text("Replace")');
+
+    await page.locator('text=7/7').waitFor();
+
+    await page.click('button:has-text("Replace")');
+    await page.locator('text=1/8').waitFor();
+  });
+
+  test('Replace with a string containing the query string three times', async ({
+    page
+  }) => {
+    // Create a small test notebook
+    await page.notebook.createNew();
+    await page.notebook.setCell(0, 'code', 'test\ntest');
+    await page.notebook.addCell('code', 'test\ntest');
+
+    await page.keyboard.press('Control+f');
+    await page.fill('[placeholder="Find"]', 'test');
+
+    await page.click('button[title="Show Replace"]');
+    await page.fill('[placeholder="Replace"]', 'testtesttest');
+
+    // TODO: Next Match press count should be one less
+    // (the -/4 state should not be necessary).
+    await page.locator('text=-/4').waitFor();
+    await page.click('button[title^="Next Match"]', {
+      clickCount: 3
+    });
+
+    await page.locator('text=1/4').waitFor();
+    await page.click('button:has-text("Replace")');
+
+    // We should move to the first match after the first replacement.
+    await page.locator('text=4/6').waitFor();
+    await page.click('button:has-text("Replace")');
+
+    // At this point we should be in the second cell
+    await page.locator('text=7/8').waitFor();
+    await page.click('button:has-text("Replace")');
+
+    await page.locator('text=10/10').waitFor();
+
+    await page.click('button:has-text("Replace")');
+    await page.locator('text=1/12').waitFor();
+  });
 });

packages/codemirror/src/searchprovider.ts

@@ -278,7 +278,7 @@ export abstract class EditorSearchProvider<
         this.currentIndex = this.cmHandler.currentIndex;
       } else {
         // Note: the loop logic is only used in single-editor (e.g. file editor)
-        // provider sub-classes, notebook has it's own loop logic and ignores
+        // provider sub-classes, notebook has its own loop logic and ignores
         // `currentIndex` as set here.
         this.currentIndex = loop ? 0 : null;
       }
@@ -375,6 +375,7 @@ export abstract class EditorSearchProvider<
                   nextMatchFound = true;
                   break;
                 }
+
                 // Move the highlight forward from the previous match, not looping.
                 void this.highlightNext(false, { from: 'previous-match' });
               }

packages/notebook/src/searchprovider.ts

@@ -466,7 +466,7 @@ export class NotebookSearchProvider extends SearchProvider<NotebookPanel> {
       );
       if (searchEngine.currentMatchIndex === null) {
         // switch to next cell
-        await this.highlightNext(loop);
+        await this.highlightNext(loop, { from: 'previous-match' });
       }
     }
 
@@ -658,7 +658,30 @@ export class NotebookSearchProvider extends SearchProvider<NotebookPanel> {
       }
     }
 
+    // If we're looking for the next match after the previous match,
+    // and we've reached the end of the current cell, start at the next one, if possible
+    const from = options?.from ?? '';
+    const atEndOfCurrentCell =
+      from === 'previous-match' &&
+      this._searchProviders[this._currentProviderIndex].currentMatchIndex ===
+        null;
+
     const startIndex = this._currentProviderIndex;
+    // If we need to move to the next cell or loop, reset the position of the current search provider.
+    if (atEndOfCurrentCell) {
+      void this._searchProviders[this._currentProviderIndex].clearHighlight();
+    }
+
+    // If we're at the end of the last cell in the provider list and we need to loop, do so
+    if (
+      loop &&
+      atEndOfCurrentCell &&
+      this._currentProviderIndex + 1 >= this._searchProviders.length
+    ) {
+      this._currentProviderIndex = 0;
+    } else {
+      this._currentProviderIndex += atEndOfCurrentCell ? 1 : 0;
+    }
     do {
       const searchEngine = this._searchProviders[this._currentProviderIndex];