Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix search coming back in notebook and editor #15443

Merged
merged 13 commits into from
Dec 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
58 changes: 58 additions & 0 deletions galata/test/jupyterlab/notebook-search-highlight.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Copyright (c) Jupyter Development Team.
// Distributed under the terms of the Modified BSD License.
import { expect, galata, test } from '@jupyterlab/galata';
import * as path from 'path';

const TEST_FILENAME = 'search_highlight_notebook.ipynb';
const TEST_NEEDLE = 'come';

test.use({ tmpPath: 'test-search' });

test.beforeAll(async ({ request, tmpPath }) => {
const contents = galata.newContentsHelper(request);
await contents.uploadFile(
path.resolve(__dirname, `./notebooks/${TEST_FILENAME}`),
`${tmpPath}/${TEST_FILENAME}`
);
});

test.beforeEach(async ({ page, tmpPath }) => {
await page.notebook.openByPath(`${tmpPath}/${TEST_FILENAME}`);
await page.notebook.activate(TEST_FILENAME);
});

const HIGHLIGHTS_LOCATOR = '.cm-searching';

test('Open and close Search dialog, then add new code cell', async ({
page
}) => {
// search for our needle
await page.evaluate(async searchText => {
await window.jupyterapp.commands.execute('documentsearch:start', {
searchText
});
}, TEST_NEEDLE);

// wait for the search to complete
await page.waitForSelector('text=1/21');
expect(await page.locator(HIGHLIGHTS_LOCATOR).count()).toBeGreaterThanOrEqual(
4
);

// cancel search
await page.keyboard.press('Escape');

// expect the highlights to have gone
expect(await page.locator(HIGHLIGHTS_LOCATOR).count()).toEqual(0);

// insert a new code cell
await page.evaluate(async () =>
window.jupyterapp.commands.execute('notebook:insert-cell-below')
);

// wait an arbitrary amount of extra time
// and expect the highlights to be still gone
// regression-testing against #14871
await new Promise(resolve => setTimeout(resolve, 1000));
expect(await page.locator(HIGHLIGHTS_LOCATOR).count()).toEqual(0);
});
110 changes: 110 additions & 0 deletions galata/test/jupyterlab/notebooks/search_highlight_notebook.ipynb
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
{
"cells": [
{
"cell_type": "markdown",
"id": "e86bd77e-7e48-4941-8cb9-72410d8256aa",
"metadata": {},
"source": [
"# regular notebook (not myst)"
]
},
{
"cell_type": "markdown",
"id": "d06cb0c2-483b-4781-b283-8212ea667823",
"metadata": {},
"source": [
"## a subtitle\n",
"\n",
"Where does it come from?\n",
"Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of \"de Finibus Bonorum et Malorum\" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, \"Lorem ipsum dolor sit amet..\", comes from a line in section 1.10.32.\n"
]
},
{
"cell_type": "markdown",
"id": "8541f40d-4f71-48fc-92d4-708063b476ec",
"metadata": {},
"source": [
"## another one\n",
"\n",
"Where does it come from?\n",
"Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of \"de Finibus Bonorum et Malorum\" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, \"Lorem ipsum dolor sit amet..\", comes from a line in section 1.10.32.\n"
]
},
{
"cell_type": "markdown",
"id": "d896a643-8998-48cf-8a4f-ec2965656e73",
"metadata": {},
"source": [
"## yet another\n",
"\n",
"Where does it come from?\n",
"Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of \"de Finibus Bonorum et Malorum\" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, \"Lorem ipsum dolor sit amet..\", comes from a line in section 1.10.32.\n"
]
},
{
"cell_type": "markdown",
"id": "a089ea8f-02b8-4b09-af17-aca38782b3c7",
"metadata": {},
"source": [
"## let's change\n",
"\n",
"Where does it come from?\n",
"Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of \"de Finibus Bonorum et Malorum\" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, \"Lorem ipsum dolor sit amet..\", comes from a line in section 1.10.32.\n"
]
},
{
"cell_type": "markdown",
"id": "6f9ad439-3dc0-410c-9d98-2a9356f839a3",
"metadata": {},
"source": [
"## to a less predictible\n",
"\n",
"Where does it come from?\n",
"Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of \"de Finibus Bonorum et Malorum\" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, \"Lorem ipsum dolor sit amet..\", comes from a line in section 1.10.32.\n"
]
},
{
"cell_type": "markdown",
"id": "a783fb23-b79c-40c7-b8fe-be597130a3ae",
"metadata": {},
"source": [
"## pattern in picking names\n",
"\n",
"Where does it come from?\n",
"Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of \"de Finibus Bonorum et Malorum\" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, \"Lorem ipsum dolor sit amet..\", comes from a line in section 1.10.32.\n"
]
},
{
"cell_type": "markdown",
"id": "4e2ae1be-078e-4bf6-974b-6ea6750a74a9",
"metadata": {},
"source": [
"## the last one\n",
"\n",
"Where does it come from?\n",
"Contrary to popular belief, Lorem Ipsum is not simply random text. It has roots in a piece of classical Latin literature from 45 BC, making it over 2000 years old. Richard McClintock, a Latin professor at Hampden-Sydney College in Virginia, looked up one of the more obscure Latin words, consectetur, from a Lorem Ipsum passage, and going through the cites of the word in classical literature, discovered the undoubtable source. Lorem Ipsum comes from sections 1.10.32 and 1.10.33 of \"de Finibus Bonorum et Malorum\" (The Extremes of Good and Evil) by Cicero, written in 45 BC. This book is a treatise on the theory of ethics, very popular during the Renaissance. The first line of Lorem Ipsum, \"Lorem ipsum dolor sit amet..\", comes from a line in section 1.10.32.\n"
]
}
],
"metadata": {
"kernelspec": {
"display_name": "Python 3 (ipykernel)",
"language": "python",
"name": "python3"
},
"language_info": {
"codemirror_mode": {
"name": "ipython",
"version": 3
},
"file_extension": ".py",
"mimetype": "text/x-python",
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.11.5"
}
},
"nbformat": 4,
"nbformat_minor": 5
}
10 changes: 5 additions & 5 deletions packages/cells/src/searchprovider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ class MarkdownCellSearchProvider extends CellSearchProvider {
const cell = this.cell as MarkdownCell;
if (cell.rendered && this.matchesCount > 0) {
// Unrender the cell
this._unrenderedByHighligh = true;
this._unrenderedByHighlight = true;
const waitForRendered = signalToPromise(cell.renderedChanged);
cell.rendered = false;
await waitForRendered;
Expand All @@ -347,7 +347,7 @@ class MarkdownCellSearchProvider extends CellSearchProvider {
const cell = this.cell as MarkdownCell;
if (cell.rendered && this.matchesCount > 0) {
// Unrender the cell if there are matches within the cell
this._unrenderedByHighligh = true;
this._unrenderedByHighlight = true;
const waitForRendered = signalToPromise(cell.renderedChanged);
cell.rendered = false;
await waitForRendered;
Expand Down Expand Up @@ -397,10 +397,10 @@ class MarkdownCellSearchProvider extends CellSearchProvider {
* @param rendered New rendered value
*/
protected onRenderedChanged(cell: MarkdownCell, rendered: boolean): void {
if (!this._unrenderedByHighligh) {
if (!this._unrenderedByHighlight) {
this.currentIndex = null;
}
this._unrenderedByHighligh = false;
this._unrenderedByHighlight = false;
if (this.isActive) {
if (rendered) {
void this.renderedProvider.startQuery(this.query);
Expand All @@ -413,7 +413,7 @@ class MarkdownCellSearchProvider extends CellSearchProvider {
}

protected renderedProvider: GenericSearchProvider;
private _unrenderedByHighligh = false;
private _unrenderedByHighlight = false;
}

/**
Expand Down
1 change: 0 additions & 1 deletion packages/documentsearch-extension/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,6 @@ const extension: JupyterFrontEndPlugin<ISearchProviderRegistry> = {
if (!currentWidget) {
return;
}

searchViews.get(currentWidget.id)?.close();
}
});
Expand Down
16 changes: 14 additions & 2 deletions packages/documentsearch/src/searchmodel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export class SearchDocumentModel
}
}

searchProvider.stateChanged.connect(this.refresh, this);
searchProvider.stateChanged.connect(this._onProviderStateChanged, this);

this._searchDebouncer = new Debouncer(() => {
this._updateSearch().catch(reason => {
Expand Down Expand Up @@ -248,7 +248,10 @@ export class SearchDocumentModel
});
}

this.searchProvider.stateChanged.disconnect(this.refresh, this);
this.searchProvider.stateChanged.disconnect(
this._onProviderStateChanged,
this
);

this._searchDebouncer.dispose();
super.dispose();
Expand All @@ -258,6 +261,7 @@ export class SearchDocumentModel
* End the query.
*/
async endQuery(): Promise<void> {
this._searchActive = false;
await this.searchProvider.endQuery();
this.stateChanged.emit();
}
Expand Down Expand Up @@ -351,6 +355,7 @@ export class SearchDocumentModel
)
: null;
if (query) {
this._searchActive = true;
await this.searchProvider.startQuery(query, this._filters);
// Emit state change as the index needs to be updated
this.stateChanged.emit();
Expand All @@ -365,13 +370,20 @@ export class SearchDocumentModel
}
}

private _onProviderStateChanged() {
if (this._searchActive) {
this.refresh();
}
}

private _caseSensitive = false;
private _disposed = new Signal<this, void>(this);
private _parsingError = '';
private _preserveCase = false;
private _initialQuery = '';
private _filters: IFilters = {};
private _replaceText: string = '';
private _searchActive = false;
private _searchDebouncer: Debouncer;
private _searchExpression = '';
private _useRegex = false;
Expand Down
1 change: 1 addition & 0 deletions packages/fileeditor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"watch": "tsc -b --watch"
},
"dependencies": {
"@jupyter/ydoc": "^1.1.1",
"@jupyterlab/apputils": "^4.2.0-alpha.4",
"@jupyterlab/codeeditor": "^4.1.0-alpha.4",
"@jupyterlab/codemirror": "^4.1.0-alpha.4",
Expand Down
27 changes: 27 additions & 0 deletions packages/fileeditor/src/searchprovider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
import { ITranslator } from '@jupyterlab/translation';
import { Widget } from '@lumino/widgets';
import { FileEditor } from './widget';
import { ISharedText, SourceChange } from '@jupyter/ydoc';

/**
* Helper type
Expand Down Expand Up @@ -64,6 +65,7 @@ export class FileEditorSearchProvider
query: RegExp,
filters: IFilters | undefined
): Promise<void> {
this._searchActive = true;
await super.startQuery(query, filters);
await this.highlightNext(true, {
from: 'selection-start',
Expand All @@ -72,6 +74,29 @@ export class FileEditorSearchProvider
});
}

/**
* Stop the search and clean any UI elements.
*/
async endQuery(): Promise<void> {
this._searchActive = false;
await super.endQuery();
}

/**
* Callback on source change
*
* @param emitter Source of the change
* @param changes Source change
*/
protected async onSharedModelChanged(
emitter: ISharedText,
changes: SourceChange
): Promise<void> {
if (this._searchActive) {
return super.onSharedModelChanged(emitter, changes);
}
}

/**
* Instantiate a search provider for the widget.
*
Expand Down Expand Up @@ -116,4 +141,6 @@ export class FileEditorSearchProvider
);
return selection;
}

private _searchActive = false;
}
7 changes: 6 additions & 1 deletion packages/notebook/src/searchprovider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ export class NotebookSearchProvider extends SearchProvider<NotebookPanel> {
return;
}
await this.endQuery();
this._searchActive = true;
let cells = this.widget.content.widgets;

this._query = query;
Expand Down Expand Up @@ -426,6 +427,7 @@ export class NotebookSearchProvider extends SearchProvider<NotebookPanel> {
})
);

this._searchActive = false;
this._searchProviders.length = 0;
this._currentProviderIndex = null;
}
Expand Down Expand Up @@ -552,7 +554,9 @@ export class NotebookSearchProvider extends SearchProvider<NotebookPanel> {
this.widget.content.isSelectedOrActive(cell)
)
.then(() => {
void cellSearchProvider.startQuery(this._query, this._filters);
if (this._searchActive) {
void cellSearchProvider.startQuery(this._query, this._filters);
}
});
}

Expand Down Expand Up @@ -914,4 +918,5 @@ export class NotebookSearchProvider extends SearchProvider<NotebookPanel> {
> | null = null;
private _selectionSearchMode: 'cells' | 'text' = 'cells';
private _selectionLock: boolean = false;
private _searchActive: boolean = false;
}
1 change: 1 addition & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3459,6 +3459,7 @@ __metadata:
version: 0.0.0-use.local
resolution: "@jupyterlab/fileeditor@workspace:packages/fileeditor"
dependencies:
"@jupyter/ydoc": ^1.1.1
"@jupyterlab/apputils": ^4.2.0-alpha.4
"@jupyterlab/codeeditor": ^4.1.0-alpha.4
"@jupyterlab/codemirror": ^4.1.0-alpha.4
Expand Down