Skip to content

Commit 9d5c6c0

Browse files
committed
refactor(deepnote): Simplify notebook edit application logic
- Introduced a new `applyNotebookEdits` method in `DeepnoteFileChangeWatcher` to centralize the application of notebook edits, improving code readability and maintainability. - Updated existing calls to `workspace.applyEdit` to utilize the new method, reducing redundancy in the codebase. - Adjusted unit tests to reflect changes in the edit application process, ensuring consistent behavior across the application.
1 parent 0b2504a commit 9d5c6c0

3 files changed

Lines changed: 39 additions & 92 deletions

File tree

src/notebooks/deepnote/deepnoteFileChangeWatcher.ts

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,13 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic
122122
}
123123
}
124124

125+
protected async applyNotebookEdits(uri: Uri, edits: NotebookEdit[]): Promise<boolean> {
126+
const wsEdit = new WorkspaceEdit();
127+
wsEdit.set(uri, edits);
128+
129+
return workspace.applyEdit(wsEdit);
130+
}
131+
125132
private clearAllTimers(): void {
126133
for (const timer of this.debounceTimers.values()) {
127134
clearTimeout(timer);
@@ -350,9 +357,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic
350357
}
351358

352359
// Apply the edit to update in-memory cells immediately (responsive UX).
353-
const wsEdit = new WorkspaceEdit();
354-
wsEdit.set(notebook.uri, edits);
355-
const applied = await workspace.applyEdit(wsEdit);
360+
const applied = await this.applyNotebookEdits(notebook.uri, edits);
356361

357362
if (!applied) {
358363
logger.warn(`[FileChangeWatcher] Failed to apply edit: ${notebook.uri.path}`);
@@ -501,9 +506,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic
501506
}
502507
}
503508
if (metadataEdits.length > 0) {
504-
const wsEdit = new WorkspaceEdit();
505-
wsEdit.set(notebook.uri, metadataEdits);
506-
await workspace.applyEdit(wsEdit);
509+
await this.applyNotebookEdits(notebook.uri, metadataEdits);
507510
}
508511

509512
logger.info(`[FileChangeWatcher] Updated notebook outputs via execution API: ${notebook.uri.path}`);
@@ -531,9 +534,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic
531534
);
532535
}
533536

534-
const wsEdit = new WorkspaceEdit();
535-
wsEdit.set(notebook.uri, replaceEdits);
536-
const applied = await workspace.applyEdit(wsEdit);
537+
const applied = await this.applyNotebookEdits(notebook.uri, replaceEdits);
537538

538539
if (!applied) {
539540
logger.warn(`[FileChangeWatcher] Failed to apply snapshot outputs: ${notebook.uri.path}`);
@@ -552,9 +553,7 @@ export class DeepnoteFileChangeWatcher implements IExtensionSyncActivationServic
552553
})
553554
);
554555
}
555-
const metaEdit = new WorkspaceEdit();
556-
metaEdit.set(notebook.uri, metadataEdits);
557-
await workspace.applyEdit(metaEdit);
556+
await this.applyNotebookEdits(notebook.uri, metadataEdits);
558557

559558
// Save to sync mtime — mark as self-write first
560559
this.markSelfWrite(notebook.uri);

src/notebooks/deepnote/deepnoteFileChangeWatcher.unit.test.ts

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@ import {
99
NotebookCellKind,
1010
NotebookDocument,
1111
NotebookEdit,
12-
Uri,
13-
WorkspaceEdit
12+
Uri
1413
} from 'vscode';
1514

1615
import type { IControllerRegistration } from '../controllers/types';
@@ -94,6 +93,11 @@ const rapidChangeIntervalMs = 100;
9493
const autoSaveGraceMs = 200;
9594
const postSnapshotReadGraceMs = 100;
9695

96+
interface NotebookEditCapture {
97+
uriKey: string;
98+
cellSourceJoined: string;
99+
}
100+
97101
/**
98102
* Polls until a condition is met or a timeout is reached.
99103
*/
@@ -1263,7 +1267,7 @@ project:
12631267
});
12641268

12651269
suite('multi-notebook file sync', () => {
1266-
let workspaceSetCaptures: { uriKey: string; cellSourceJoined: string }[] = [];
1270+
let workspaceSetCaptures: NotebookEditCapture[] = [];
12671271

12681272
setup(() => {
12691273
reset(mockedNotebookManager);
@@ -1272,26 +1276,21 @@ project:
12721276
when(mockedNotebookManager.clearNotebookSelection(anything())).thenReturn();
12731277
resetCalls(mockedNotebookManager);
12741278
workspaceSetCaptures = [];
1275-
when(mockedVSCodeNamespaces.workspace.applyEdit(anything())).thenCall((wsEdit: WorkspaceEdit) => {
1276-
applyEditCount++;
1277-
1278-
const ext = wsEdit as WorkspaceEdit & { notebookEntries(): [Uri, NotebookEdit[]][] };
1279+
sinon.stub(watcher, 'applyNotebookEdits' as any).callsFake(async (...args: unknown[]) => {
1280+
const uri = args[0] as Uri;
1281+
const edits = args[1] as NotebookEdit[];
12791282

1280-
for (const [uri, edits] of ext.notebookEntries()) {
1281-
if (!edits.length) {
1282-
continue;
1283-
}
1283+
applyEditCount++;
12841284

1285-
const firstEdit = edits[0];
1286-
if (firstEdit?.newCells && firstEdit.newCells.length > 0) {
1287-
workspaceSetCaptures.push({
1288-
uriKey: uri.toString(),
1289-
cellSourceJoined: firstEdit.newCells.map((c) => c.value).join('\n')
1290-
});
1291-
}
1285+
const replaceCellsEdit = edits.find((e) => e.newCells?.length > 0);
1286+
if (replaceCellsEdit) {
1287+
workspaceSetCaptures.push({
1288+
uriKey: uri.toString(),
1289+
cellSourceJoined: replaceCellsEdit.newCells.map((c: any) => c.value).join('\n')
1290+
});
12921291
}
12931292

1294-
return Promise.resolve(true);
1293+
return true;
12951294
});
12961295
});
12971296

src/test/mocks/vsc/extHostedTypes.ts

Lines changed: 10 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -707,7 +707,6 @@ export namespace vscMockExtHostedTypes {
707707
private _seqPool: number = 0;
708708

709709
private _resourceEdits: { seq: number; from: vscUri.URI; to: vscUri.URI }[] = [];
710-
private _notebookEdits = new Map<string, { uri: vscUri.URI; edits: NotebookEdit[] }>();
711710
private _textEdits = new Map<string, { seq: number; uri: vscUri.URI; edits: TextEdit[] }>();
712711

713712
// createResource(uri: vscode.Uri): void {
@@ -760,20 +759,7 @@ export namespace vscMockExtHostedTypes {
760759
}
761760

762761
has(uri: vscUri.URI): boolean {
763-
return this._textEdits.has(uri.toString()) || this._notebookEdits.has(uri.toString());
764-
}
765-
766-
/**
767-
* Notebook edits passed to {@link set} (for tests that inspect edits via workspace.applyEdit).
768-
*/
769-
notebookEntries(): [vscUri.URI, NotebookEdit[]][] {
770-
const res: [vscUri.URI, NotebookEdit[]][] = [];
771-
772-
this._notebookEdits.forEach((value) => {
773-
res.push([value.uri, value.edits.slice()]);
774-
});
775-
776-
return res.slice();
762+
return this._textEdits.has(uri.toString());
777763
}
778764

779765
set(uri: vscode.Uri, edits: NotebookEdit[]): void;
@@ -793,54 +779,17 @@ export namespace vscMockExtHostedTypes {
793779
| [TextEdit | SnippetTextEdit, vscode.WorkspaceEditEntryMetadata]
794780
)[]
795781
): void {
796-
const uriKey = uri.toString();
797-
798-
if (!edits) {
799-
const data = this._textEdits.get(uriKey);
800-
if (data) {
801-
// @ts-ignore
802-
data.edits = undefined;
803-
}
804-
this._notebookEdits.delete(uriKey);
805-
806-
return;
807-
}
808-
809-
if (edits.length === 0) {
810-
let data = this._textEdits.get(uriKey);
811-
if (!data) {
812-
data = { seq: this._seqPool++, uri, edits: [] };
813-
this._textEdits.set(uriKey, data);
814-
}
815-
data.edits = [];
816-
817-
return;
818-
}
819-
820-
const first = edits[0];
821-
const firstEdit = Array.isArray(first) ? first[0] : first;
822-
823-
if (firstEdit instanceof NotebookEdit) {
824-
const normalized: NotebookEdit[] = edits.map((item) => {
825-
if (Array.isArray(item)) {
826-
return item[0] as NotebookEdit;
827-
}
828-
829-
return item as NotebookEdit;
830-
});
831-
832-
this._notebookEdits.set(uriKey, { uri, edits: normalized });
833-
834-
return;
835-
}
836-
837-
let data = this._textEdits.get(uriKey);
782+
let data = this._textEdits.get(uri.toString());
838783
if (!data) {
839784
data = { seq: this._seqPool++, uri, edits: [] };
840-
this._textEdits.set(uriKey, data);
785+
this._textEdits.set(uri.toString(), data);
786+
}
787+
if (!edits) {
788+
// @ts-ignore
789+
data.edits = undefined;
790+
} else {
791+
//data.edits = edits.slice(0);
841792
}
842-
843-
data.edits = edits.slice(0) as TextEdit[];
844793
}
845794

846795
get(uri: vscUri.URI): TextEdit[] {
@@ -876,7 +825,7 @@ export namespace vscMockExtHostedTypes {
876825
}
877826

878827
get size(): number {
879-
return this._textEdits.size + this._notebookEdits.size + this._resourceEdits.length;
828+
return this._textEdits.size + this._resourceEdits.length;
880829
}
881830

882831
toJSON(): any {

0 commit comments

Comments
 (0)