Skip to content

Commit 43d0483

Browse files
authored
Merge pull request microsoft#146185 from microsoft/joh/notebookCleanup
2 parents b826b37 + b1faab4 commit 43d0483

17 files changed

+126
-275
lines changed

extensions/ipynb/package.json

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@
1010
},
1111
"enabledApiProposals": [
1212
"notebookEditor",
13-
"notebookEditorEdit"
13+
"notebookEditorEdit",
14+
"notebookDocumentEvents"
1415
],
1516
"activationEvents": [
1617
"*"
@@ -31,9 +32,9 @@
3132
"commands": [
3233
{
3334
"command": "ipynb.newUntitledIpynb",
34-
"title": "New Jupyter Notebook",
35+
"title": "New Jupyter Notebook",
3536
"shortTitle": "Jupyter Notebook",
36-
"category": "Create"
37+
"category": "Create"
3738
},
3839
{
3940
"command": "ipynb.openIpynbInNotebookEditor",
@@ -60,9 +61,9 @@
6061
}
6162
],
6263
"commandPalette": [
63-
{
64-
"command": "ipynb.newUntitledIpynb"
65-
},
64+
{
65+
"command": "ipynb.newUntitledIpynb"
66+
},
6667
{
6768
"command": "ipynb.openIpynbInNotebookEditor",
6869
"when": "false"

extensions/ipynb/src/cellIdService.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55

6-
import { ExtensionContext, NotebookCellsChangeEvent, NotebookDocument, notebooks, workspace, WorkspaceEdit } from 'vscode';
6+
import { ExtensionContext, NotebookDocument, NotebookDocumentChangeEvent, workspace, WorkspaceEdit } from 'vscode';
77
import { v4 as uuid } from 'uuid';
88
import { getCellMetadata } from './serializers';
99
import { CellMetadata } from './common';
@@ -15,21 +15,21 @@ import * as nbformat from '@jupyterlab/nbformat';
1515
* Details of the spec can be found here https://jupyter.org/enhancement-proposals/62-cell-id/cell-id.html#
1616
*/
1717
export function ensureAllNewCellsHaveCellIds(context: ExtensionContext) {
18-
notebooks.onDidChangeNotebookCells(onDidChangeNotebookCells, undefined, context.subscriptions);
18+
workspace.onDidChangeNotebookDocument(onDidChangeNotebookCells, undefined, context.subscriptions);
1919
}
2020

21-
function onDidChangeNotebookCells(e: NotebookCellsChangeEvent) {
22-
const nbMetadata = getNotebookMetadata(e.document);
21+
function onDidChangeNotebookCells(e: NotebookDocumentChangeEvent) {
22+
const nbMetadata = getNotebookMetadata(e.notebook);
2323
if (!isCellIdRequired(nbMetadata)) {
2424
return;
2525
}
26-
e.changes.forEach(change => {
27-
change.items.forEach(cell => {
26+
e.contentChanges.forEach(change => {
27+
change.addedCells.forEach(cell => {
2828
const cellMetadata = getCellMetadata(cell);
2929
if (cellMetadata?.id) {
3030
return;
3131
}
32-
const id = generateCellId(e.document);
32+
const id = generateCellId(e.notebook);
3333
const edit = new WorkspaceEdit();
3434
// Don't edit the metadata directly, always get a clone (prevents accidental singletons and directly editing the objects).
3535
const updatedMetadata: CellMetadata = { ...JSON.parse(JSON.stringify(cellMetadata || {})) };

extensions/ipynb/tsconfig.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,6 @@
1111
"../../src/vscode-dts/vscode.d.ts",
1212
"../../src/vscode-dts/vscode.proposed.notebookEditor.d.ts",
1313
"../../src/vscode-dts/vscode.proposed.notebookEditorEdit.d.ts",
14+
"../../src/vscode-dts/vscode.proposed.notebookDocumentEvents.d.ts",
1415
]
1516
}

extensions/vscode-api-tests/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
"notebookControllerKind",
2424
"notebookDebugOptions",
2525
"notebookDeprecated",
26+
"notebookDocumentEvents",
2627
"notebookEditor",
2728
"notebookEditorDecorationType",
2829
"notebookEditorEdit",

extensions/vscode-api-tests/src/singlefolder-tests/notebook.document.test.ts

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ suite.skip('Notebook Document', function () {
261261
value: 'new_code'
262262
}]);
263263

264-
const event = utils.asPromise<vscode.NotebookCellsChangeEvent>(vscode.notebooks.onDidChangeNotebookCells);
264+
const event = utils.asPromise<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument);
265265

266266
const success = await vscode.workspace.applyEdit(edit);
267267
assert.strictEqual(success, true);
@@ -274,13 +274,13 @@ suite.skip('Notebook Document', function () {
274274
assert.strictEqual(document.cellAt(1).document.getText(), 'new_code');
275275

276276
// check event data
277-
assert.strictEqual(data.document === document, true);
278-
assert.strictEqual(data.changes.length, 1);
279-
assert.strictEqual(data.changes[0].deletedCount, 0);
280-
assert.strictEqual(data.changes[0].deletedItems.length, 0);
281-
assert.strictEqual(data.changes[0].items.length, 2);
282-
assert.strictEqual(data.changes[0].items[0], document.cellAt(0));
283-
assert.strictEqual(data.changes[0].items[1], document.cellAt(1));
277+
assert.strictEqual(data.notebook === document, true);
278+
assert.strictEqual(data.contentChanges.length, 1);
279+
assert.strictEqual(data.contentChanges[0].range.isEmpty, true);
280+
assert.strictEqual(data.contentChanges[0].removedCells.length, 0);
281+
assert.strictEqual(data.contentChanges[0].addedCells.length, 2);
282+
assert.strictEqual(data.contentChanges[0].addedCells[0], document.cellAt(0));
283+
assert.strictEqual(data.contentChanges[0].addedCells[1], document.cellAt(1));
284284
});
285285

286286
test('workspace edit API (replaceMetadata)', async function () {
@@ -299,7 +299,7 @@ suite.skip('Notebook Document', function () {
299299
const document = await vscode.workspace.openNotebookDocument(uri);
300300

301301
const edit = new vscode.WorkspaceEdit();
302-
const event = utils.asPromise<vscode.NotebookCellMetadataChangeEvent>(vscode.notebooks.onDidChangeCellMetadata);
302+
const event = utils.asPromise<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument);
303303

304304
edit.replaceNotebookCellMetadata(document.uri, 0, { inputCollapsed: true });
305305
const success = await vscode.workspace.applyEdit(edit);
@@ -310,8 +310,10 @@ suite.skip('Notebook Document', function () {
310310
assert.strictEqual(document.cellAt(0).metadata.inputCollapsed, true);
311311

312312
// check event data
313-
assert.strictEqual(data.document === document, true);
314-
assert.strictEqual(data.cell.index, 0);
313+
assert.strictEqual(data.notebook === document, true);
314+
assert.strictEqual(data.contentChanges.length, 0);
315+
assert.strictEqual(data.cellChanges.length, 1);
316+
assert.strictEqual(data.cellChanges[0].cell.index, 0);
315317
});
316318

317319
test('document save API', async function () {

extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts

Lines changed: 56 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -194,41 +194,52 @@ const apiTestContentProvider: vscode.NotebookContentProvider = {
194194
const notebook = await openRandomNotebookDocument();
195195
const editor = await vscode.window.showNotebookDocument(notebook);
196196

197-
const cellsChangeEvent = asPromise<vscode.NotebookCellsChangeEvent>(vscode.notebooks.onDidChangeNotebookCells);
197+
const cellsChangeEvent = asPromise<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument);
198198
await vscode.commands.executeCommand('notebook.cell.insertCodeCellBelow');
199199
const cellChangeEventRet = await cellsChangeEvent;
200-
assert.strictEqual(cellChangeEventRet.document, editor.document);
201-
assert.strictEqual(cellChangeEventRet.changes.length, 1);
202-
assert.deepStrictEqual(cellChangeEventRet.changes[0], {
203-
start: 1,
204-
deletedCount: 0,
205-
deletedItems: [],
206-
items: [
207-
editor.document.cellAt(1)
208-
]
200+
assert.strictEqual(cellChangeEventRet.notebook, editor.document);
201+
assert.strictEqual(cellChangeEventRet.contentChanges.length, 1);
202+
assert.deepStrictEqual(cellChangeEventRet.contentChanges[0], <vscode.NotebookDocumentContentChange>{
203+
range: new vscode.NotebookRange(1, 1),
204+
removedCells: [],
205+
addedCells: [editor.document.cellAt(1)]
209206
});
210207

211-
const moveCellEvent = asPromise<vscode.NotebookCellsChangeEvent>(vscode.notebooks.onDidChangeNotebookCells);
208+
const moveCellEvent = asPromise<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument);
212209
await vscode.commands.executeCommand('notebook.cell.moveUp');
213210
await moveCellEvent;
214211

215-
const cellOutputChange = asPromise<vscode.NotebookCellOutputsChangeEvent>(vscode.notebooks.onDidChangeCellOutputs);
212+
const cellOutputChange = asPromise<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument);
216213
await vscode.commands.executeCommand('notebook.cell.execute');
217214
const cellOutputsAddedRet = await cellOutputChange;
218-
assert.deepStrictEqual(cellOutputsAddedRet, {
219-
document: editor.document,
220-
cells: [editor.document.cellAt(0)]
221-
});
222-
assert.strictEqual(cellOutputsAddedRet.cells[0].outputs.length, 1);
223215

224-
const cellOutputClear = asPromise<vscode.NotebookCellOutputsChangeEvent>(vscode.notebooks.onDidChangeCellOutputs);
216+
assert.strictEqual(cellOutputsAddedRet.notebook.uri.toString(), editor.document.uri.toString());
217+
assert.strictEqual(cellOutputsAddedRet.metadata, undefined);
218+
assert.strictEqual(cellOutputsAddedRet.contentChanges.length, 0);
219+
assert.strictEqual(cellOutputsAddedRet.cellChanges.length, 1);
220+
assert.deepStrictEqual(cellOutputsAddedRet.cellChanges[0].cell, editor.document.cellAt(0));
221+
assert.deepStrictEqual(cellOutputsAddedRet.cellChanges[0].executionSummary, { executionOrder: undefined, success: undefined, timing: undefined }); // TODO@jrieken should this be undefined instead all empty?
222+
assert.strictEqual(cellOutputsAddedRet.cellChanges[0].document, undefined);
223+
assert.strictEqual(cellOutputsAddedRet.cellChanges[0].metadata, undefined);
224+
assert.strictEqual(cellOutputsAddedRet.cellChanges[0].outputs, undefined);
225+
assert.strictEqual(cellOutputsAddedRet.cellChanges[0].cell.outputs.length, 1);
226+
227+
const cellOutputClear = asPromise<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument);
225228
await vscode.commands.executeCommand('notebook.cell.clearOutputs');
226229
const cellOutputsCleardRet = await cellOutputClear;
227-
assert.deepStrictEqual(cellOutputsCleardRet, {
228-
document: editor.document,
229-
cells: [editor.document.cellAt(0)]
230+
assert.deepStrictEqual(cellOutputsCleardRet, <vscode.NotebookDocumentChangeEvent>{
231+
notebook: editor.document,
232+
metadata: undefined,
233+
contentChanges: [],
234+
cellChanges: [{
235+
cell: editor.document.cellAt(0),
236+
document: undefined,
237+
executionSummary: undefined,
238+
metadata: undefined,
239+
outputs: editor.document.cellAt(0).outputs
240+
}],
230241
});
231-
assert.strictEqual(cellOutputsAddedRet.cells[0].outputs.length, 0);
242+
assert.strictEqual(cellOutputsCleardRet.cellChanges[0].cell.outputs.length, 0);
232243

233244
// const cellChangeLanguage = getEventOncePromise<vscode.NotebookCellLanguageChangeEvent>(vscode.notebooks.onDidChangeCellLanguage);
234245
// await vscode.commands.executeCommand('notebook.cell.changeToMarkdown');
@@ -244,33 +255,29 @@ const apiTestContentProvider: vscode.NotebookContentProvider = {
244255
const notebook = await openRandomNotebookDocument();
245256
const editor = await vscode.window.showNotebookDocument(notebook);
246257

247-
const cellsChangeEvent = asPromise<vscode.NotebookCellsChangeEvent>(vscode.notebooks.onDidChangeNotebookCells);
248-
const cellMetadataChangeEvent = asPromise<vscode.NotebookCellMetadataChangeEvent>(vscode.notebooks.onDidChangeCellMetadata);
258+
const notebookChangeEvent = asPromise<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument);
249259
const version = editor.document.version;
250260
await editor.edit(editBuilder => {
251261
editBuilder.replaceCells(1, 0, [{ kind: vscode.NotebookCellKind.Code, languageId: 'javascript', value: 'test 2', outputs: [], metadata: undefined }]);
252262
editBuilder.replaceCellMetadata(0, { inputCollapsed: false });
253263
});
254264

255-
await cellsChangeEvent;
256-
await cellMetadataChangeEvent;
265+
await notebookChangeEvent;
257266
assert.strictEqual(version + 1, editor.document.version);
258267
});
259268

260269
test('edit API batch edits undo/redo', async function () {
261270
const notebook = await openRandomNotebookDocument();
262271
const editor = await vscode.window.showNotebookDocument(notebook);
263272

264-
const cellsChangeEvent = asPromise<vscode.NotebookCellsChangeEvent>(vscode.notebooks.onDidChangeNotebookCells);
265-
const cellMetadataChangeEvent = asPromise<vscode.NotebookCellMetadataChangeEvent>(vscode.notebooks.onDidChangeCellMetadata);
273+
const notebookChangeEvent = asPromise<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument);
266274
const version = editor.document.version;
267275
await editor.edit(editBuilder => {
268276
editBuilder.replaceCells(1, 0, [{ kind: vscode.NotebookCellKind.Code, languageId: 'javascript', value: 'test 2', outputs: [], metadata: undefined }]);
269277
editBuilder.replaceCellMetadata(0, { inputCollapsed: false });
270278
});
271279

272-
await cellsChangeEvent;
273-
await cellMetadataChangeEvent;
280+
await notebookChangeEvent;
274281
assert.strictEqual(editor.document.cellCount, 3);
275282
assert.strictEqual(editor.document.cellAt(0)?.metadata.inputCollapsed, false);
276283
assert.strictEqual(version + 1, editor.document.version);
@@ -285,7 +292,7 @@ const apiTestContentProvider: vscode.NotebookContentProvider = {
285292
test.skip('#98841, initialzation should not emit cell change events.', async function () {
286293
let count = 0;
287294

288-
testDisposables.push(vscode.notebooks.onDidChangeNotebookCells(() => {
295+
testDisposables.push(vscode.workspace.onDidChangeNotebookDocument(() => {
289296
count++;
290297
}));
291298

@@ -387,9 +394,9 @@ const apiTestContentProvider: vscode.NotebookContentProvider = {
387394

388395
const activeCell = getFocusedCell(editor);
389396
assert.strictEqual(activeCell?.index, 0);
390-
const moveChange = asPromise(vscode.notebooks.onDidChangeNotebookCells);
397+
const notebookChangeEvent = asPromise(vscode.workspace.onDidChangeNotebookDocument);
391398
await vscode.commands.executeCommand('notebook.cell.moveDown');
392-
assert.ok(await moveChange);
399+
assert.ok(await notebookChangeEvent);
393400

394401
const newActiveCell = getFocusedCell(editor);
395402
assert.strictEqual(newActiveCell?.index, 1);
@@ -440,13 +447,13 @@ const apiTestContentProvider: vscode.NotebookContentProvider = {
440447
const editor = vscode.window.activeNotebookEditor!;
441448
const cell = editor.document.cellAt(0);
442449

443-
await withEvent(vscode.notebooks.onDidChangeCellOutputs, async (event) => {
450+
await withEvent(vscode.workspace.onDidChangeNotebookDocument, async (event) => {
444451
await vscode.commands.executeCommand('notebook.execute');
445452
await event;
446453
assert.strictEqual(cell.outputs.length, 1, 'should execute'); // runnable, it worked
447454
});
448455

449-
await withEvent(vscode.notebooks.onDidChangeCellOutputs, async event => {
456+
await withEvent(vscode.workspace.onDidChangeNotebookDocument, async event => {
450457
await vscode.commands.executeCommand('notebook.cell.clearOutputs');
451458
await event;
452459
assert.strictEqual(cell.outputs.length, 0, 'should clear');
@@ -455,7 +462,7 @@ const apiTestContentProvider: vscode.NotebookContentProvider = {
455462
const secondResource = await createRandomNotebookFile();
456463
await vscode.commands.executeCommand('vscode.openWith', secondResource, 'notebookCoreTest');
457464

458-
await withEvent<vscode.NotebookCellOutputsChangeEvent>(vscode.notebooks.onDidChangeCellOutputs, async (event) => {
465+
await withEvent<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument, async (event) => {
459466
await vscode.commands.executeCommand('notebook.cell.execute', { start: 0, end: 1 }, resource);
460467
await event;
461468
assert.strictEqual(cell.outputs.length, 1, 'should execute'); // runnable, it worked
@@ -473,13 +480,13 @@ const apiTestContentProvider: vscode.NotebookContentProvider = {
473480
let secondCellExecuted = false;
474481
let resolve: () => void;
475482
const p = new Promise<void>(r => resolve = r);
476-
const listener = vscode.notebooks.onDidChangeCellOutputs(e => {
477-
e.cells.forEach(cell => {
478-
if (cell.index === 0) {
483+
const listener = vscode.workspace.onDidChangeNotebookDocument(e => {
484+
e.cellChanges.forEach(change => {
485+
if (change.cell.index === 0) {
479486
firstCellExecuted = true;
480487
}
481488

482-
if (cell.index === 1) {
489+
if (change.cell.index === 1) {
483490
secondCellExecuted = true;
484491
}
485492
});
@@ -501,21 +508,21 @@ const apiTestContentProvider: vscode.NotebookContentProvider = {
501508
const editor = vscode.window.activeNotebookEditor!;
502509
const cell = editor.document.cellAt(0);
503510

504-
await withEvent<vscode.NotebookCellOutputsChangeEvent>(vscode.notebooks.onDidChangeCellOutputs, async (event) => {
511+
await withEvent<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument, async (event) => {
505512
await vscode.commands.executeCommand('notebook.execute');
506513
await event;
507514
assert.strictEqual(cell.outputs.length, 1, 'should execute'); // runnable, it worked
508515
});
509516

510-
const clearChangeEvent = asPromise<vscode.NotebookCellOutputsChangeEvent>(vscode.notebooks.onDidChangeCellOutputs);
517+
const clearChangeEvent = asPromise<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument);
511518
await vscode.commands.executeCommand('notebook.cell.clearOutputs');
512519
await clearChangeEvent;
513520
assert.strictEqual(cell.outputs.length, 0, 'should clear');
514521

515522
const secondResource = await createRandomNotebookFile();
516523
await vscode.commands.executeCommand('vscode.openWith', secondResource, 'notebookCoreTest');
517524

518-
await withEvent<vscode.NotebookCellOutputsChangeEvent>(vscode.notebooks.onDidChangeCellOutputs, async (event) => {
525+
await withEvent<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument, async (event) => {
519526
await vscode.commands.executeCommand('notebook.execute', resource);
520527
await event;
521528
assert.strictEqual(cell.outputs.length, 1, 'should execute'); // runnable, it worked
@@ -547,7 +554,7 @@ const apiTestContentProvider: vscode.NotebookContentProvider = {
547554
};
548555
testDisposables.push(alternativeKernel.controller);
549556

550-
await withEvent<vscode.NotebookCellOutputsChangeEvent>(vscode.notebooks.onDidChangeCellOutputs, async (event) => {
557+
await withEvent<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument, async (event) => {
551558
await assertKernel(defaultKernel, notebook);
552559
await vscode.commands.executeCommand('notebook.cell.execute');
553560
await event;
@@ -557,7 +564,7 @@ const apiTestContentProvider: vscode.NotebookContentProvider = {
557564
assert.deepStrictEqual(new TextDecoder().decode(cell.outputs[0].items[0].data), cell.document.getText());
558565
});
559566

560-
await withEvent<vscode.NotebookCellOutputsChangeEvent>(vscode.notebooks.onDidChangeCellOutputs, async (event) => {
567+
await withEvent<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument, async (event) => {
561568
await assertKernel(alternativeKernel, notebook);
562569
await vscode.commands.executeCommand('notebook.cell.execute');
563570
await event;
@@ -798,16 +805,16 @@ const apiTestContentProvider: vscode.NotebookContentProvider = {
798805
const notebook = await vscode.workspace.openNotebookDocument(resource);
799806
const editor = await vscode.window.showNotebookDocument(notebook);
800807

801-
const cellsChangeEvent = asPromise<vscode.NotebookCellsChangeEvent>(vscode.notebooks.onDidChangeNotebookCells);
808+
const cellsChangeEvent = asPromise<vscode.NotebookDocumentChangeEvent>(vscode.workspace.onDidChangeNotebookDocument);
802809
await editor.edit(editBuilder => {
803810
editBuilder.replaceCells(1, 0, [{ kind: vscode.NotebookCellKind.Code, languageId: 'javascript', value: 'test 2', outputs: [], metadata: undefined }]);
804811
});
805812

806813
const cellChangeEventRet = await cellsChangeEvent;
807-
assert.strictEqual(cellChangeEventRet.document === notebook, true);
808-
assert.strictEqual(cellChangeEventRet.document.isDirty, true);
814+
assert.strictEqual(cellChangeEventRet.notebook === notebook, true);
815+
assert.strictEqual(cellChangeEventRet.notebook.isDirty, true);
809816

810-
const saveEvent = asPromise(vscode.notebooks.onDidSaveNotebookDocument);
817+
const saveEvent = asPromise(vscode.workspace.onDidSaveNotebookDocument);
811818

812819
await notebook.save();
813820

src/vs/workbench/api/browser/mainThreadNotebookDocuments.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,6 @@ export class MainThreadNotebookDocuments implements MainThreadNotebookDocumentsS
108108
this._notebookEditorModelResolverService.isDirty(textModel.uri),
109109
hasDocumentMetadataChangeEvent ? textModel.metadata : undefined
110110
);
111-
112-
if (hasDocumentMetadataChangeEvent) {
113-
this._proxy.$acceptDocumentPropertiesChanged(textModel.uri, { metadata: textModel.metadata });
114-
}
115111
}));
116112

117113
this._documentEventListenersMapping.set(textModel.uri, disposableStore);

0 commit comments

Comments
 (0)