diff --git a/extensions/vscode-notebook-tests/src/notebook.test.ts b/extensions/vscode-notebook-tests/src/notebook.test.ts index cb252a20b71..186a8f40d10 100644 --- a/extensions/vscode-notebook-tests/src/notebook.test.ts +++ b/extensions/vscode-notebook-tests/src/notebook.test.ts @@ -87,6 +87,17 @@ async function saveAllFilesAndCloseAll(resource: vscode.Uri | undefined) { await documentClosed; } +async function updateCellMetadata(uri: vscode.Uri, cell: vscode.NotebookCell, newMetadata: vscode.NotebookCellMetadata) { + const edit = new vscode.WorkspaceEdit(); + edit.replaceNotebookCellMetadata(uri, cell.index, newMetadata); + await vscode.workspace.applyEdit(edit); +} + +async function withEvent(event: vscode.Event, callback: (e: Promise) => Promise) { + const e = getEventOncePromise(event); + await callback(e); +} + function assertInitalState() { // no-op unless we figure out why some documents are opened after the editor is closed @@ -892,14 +903,14 @@ suite('notebook workflow', () => { assert.equal(cell.outputs.length, 0); let metadataChangeEvent = getEventOncePromise(vscode.notebook.onDidChangeCellMetadata); - cell.metadata.runnable = false; + await updateCellMetadata(resource, cell, { ...cell.metadata, runnable: false }); await metadataChangeEvent; await vscode.commands.executeCommand('notebook.cell.execute'); assert.equal(cell.outputs.length, 0, 'should not execute'); // not runnable, didn't work metadataChangeEvent = getEventOncePromise(vscode.notebook.onDidChangeCellMetadata); - cell.metadata.runnable = true; + await updateCellMetadata(resource, cell, { ...cell.metadata, runnable: true }); await metadataChangeEvent; await vscode.commands.executeCommand('notebook.cell.execute'); @@ -918,25 +929,33 @@ suite('notebook workflow', () => { const cell = editor.document.cells[0]; assert.equal(cell.outputs.length, 0); - let metadataChangeEvent = getEventOncePromise(vscode.notebook.onDidChangeNotebookDocumentMetadata); - editor.document.metadata.runnable = false; - await metadataChangeEvent; + + await withEvent(vscode.notebook.onDidChangeNotebookDocumentMetadata, async event => { + editor.document.metadata.runnable = false; + await event; + }); await vscode.commands.executeCommand('notebook.execute'); assert.equal(cell.outputs.length, 0, 'should not execute'); // not runnable, didn't work - metadataChangeEvent = getEventOncePromise(vscode.notebook.onDidChangeNotebookDocumentMetadata); - editor.document.metadata.runnable = true; - await metadataChangeEvent; + await withEvent(vscode.notebook.onDidChangeNotebookDocumentMetadata, async event => { + editor.document.metadata.runnable = true; + await event; + }); - await vscode.commands.executeCommand('notebook.execute'); - assert.equal(cell.outputs.length, 1, 'should execute'); // runnable, it worked + await withEvent(vscode.notebook.onDidChangeCellOutputs, async (event) => { + await vscode.commands.executeCommand('notebook.execute'); + await event; + assert.equal(cell.outputs.length, 1, 'should execute'); // runnable, it worked + }); await vscode.commands.executeCommand('workbench.action.files.save'); await vscode.commands.executeCommand('workbench.action.closeActiveEditor'); }); - test('cell execute command takes arguments', async () => { + + // TODO@rebornix this is wrong, `await vscode.commands.executeCommand('notebook.execute');` doesn't wait until the workspace edit is applied + test.skip('cell execute command takes arguments', async () => { assertInitalState(); const resource = await createRandomFile('', undefined, 'first', '.vsctestnb'); await vscode.commands.executeCommand('vscode.openWith', resource, 'notebookCoreTest'); @@ -947,23 +966,44 @@ suite('notebook workflow', () => { await vscode.commands.executeCommand('notebook.execute'); assert.equal(cell.outputs.length, 0, 'should not execute'); // not runnable, didn't work - const metadataChangeEvent = getEventOncePromise(vscode.notebook.onDidChangeNotebookDocumentMetadata); - editor.document.metadata.runnable = true; - await metadataChangeEvent; + await vscode.commands.executeCommand('workbench.action.files.save'); + await vscode.commands.executeCommand('workbench.action.closeActiveEditor'); + }); - await vscode.commands.executeCommand('notebook.execute'); - assert.equal(cell.outputs.length, 1, 'should execute'); // runnable, it worked + test('cell execute command takes arguments 2', async () => { + assertInitalState(); + const resource = await createRandomFile('', undefined, 'first', '.vsctestnb'); + await vscode.commands.executeCommand('vscode.openWith', resource, 'notebookCoreTest'); + assert.equal(vscode.window.activeNotebookEditor !== undefined, true, 'notebook first'); + const editor = vscode.window.activeNotebookEditor!; + const cell = editor.document.cells[0]; - const clearChangeEvent = getEventOncePromise(vscode.notebook.onDidChangeCellOutputs); - await vscode.commands.executeCommand('notebook.cell.clearOutputs'); - await clearChangeEvent; - assert.equal(cell.outputs.length, 0, 'should clear'); + await withEvent(vscode.notebook.onDidChangeNotebookDocumentMetadata, async event => { + editor.document.metadata.runnable = true; + await event; + }); + + await withEvent(vscode.notebook.onDidChangeCellOutputs, async (event) => { + await vscode.commands.executeCommand('notebook.execute'); + await event; + assert.equal(cell.outputs.length, 1, 'should execute'); // runnable, it worked + }); + + await withEvent(vscode.notebook.onDidChangeCellOutputs, async event => { + await vscode.commands.executeCommand('notebook.cell.clearOutputs'); + await event; + assert.equal(cell.outputs.length, 0, 'should clear'); + }); const secondResource = await createRandomFile('', undefined, 'second', '.vsctestnb'); await vscode.commands.executeCommand('vscode.openWith', secondResource, 'notebookCoreTest'); - await vscode.commands.executeCommand('notebook.cell.execute', { start: 0, end: 1 }, resource); - assert.equal(cell.outputs.length, 1, 'should execute'); // runnable, it worked - assert.equal(vscode.window.activeNotebookEditor?.document.uri.fsPath, secondResource.fsPath); + + await withEvent(vscode.notebook.onDidChangeCellOutputs, async (event) => { + await vscode.commands.executeCommand('notebook.cell.execute', { start: 0, end: 1 }, resource); + await event; + assert.equal(cell.outputs.length, 1, 'should execute'); // runnable, it worked + assert.equal(vscode.window.activeNotebookEditor?.document.uri.fsPath, secondResource.fsPath); + }); await vscode.commands.executeCommand('workbench.action.files.save'); await vscode.commands.executeCommand('workbench.action.closeActiveEditor'); @@ -979,26 +1019,31 @@ suite('notebook workflow', () => { const editor = vscode.window.activeNotebookEditor!; const cell = editor.document.cells[0]; - await vscode.commands.executeCommand('notebook.execute'); - assert.equal(cell.outputs.length, 0, 'should not execute'); // not runnable, didn't work - const metadataChangeEvent = getEventOncePromise(vscode.notebook.onDidChangeNotebookDocumentMetadata); editor.document.metadata.runnable = true; await metadataChangeEvent; + assert.equal(editor.document.metadata.runnable, true); - await vscode.commands.executeCommand('notebook.execute'); - assert.equal(cell.outputs.length, 1, 'should execute'); // runnable, it worked + await withEvent(vscode.notebook.onDidChangeCellOutputs, async (event) => { + await vscode.commands.executeCommand('notebook.execute'); + await event; + assert.equal(cell.outputs.length, 1, 'should execute'); // runnable, it worked + }); - const clearChangeEvent = getEventOncePromise(vscode.notebook.onDidChangeCellOutputs); + const clearChangeEvent = getEventOncePromise(vscode.notebook.onDidChangeCellOutputs); await vscode.commands.executeCommand('notebook.cell.clearOutputs'); await clearChangeEvent; assert.equal(cell.outputs.length, 0, 'should clear'); const secondResource = await createRandomFile('', undefined, 'second', '.vsctestnb'); await vscode.commands.executeCommand('vscode.openWith', secondResource, 'notebookCoreTest'); - await vscode.commands.executeCommand('notebook.execute', resource); - assert.equal(cell.outputs.length, 1, 'should execute'); // runnable, it worked - assert.equal(vscode.window.activeNotebookEditor?.document.uri.fsPath, secondResource.fsPath); + + await withEvent(vscode.notebook.onDidChangeCellOutputs, async (event) => { + await vscode.commands.executeCommand('notebook.execute', resource); + await event; + assert.equal(cell.outputs.length, 1, 'should execute'); // runnable, it worked + assert.equal(vscode.window.activeNotebookEditor?.document.uri.fsPath, secondResource.fsPath); + }); await vscode.commands.executeCommand('workbench.action.files.save'); await vscode.commands.executeCommand('workbench.action.closeActiveEditor'); @@ -1058,13 +1103,16 @@ suite('notebook dirty state', () => { assert.equal(vscode.window.activeNotebookEditor!.document.cells.length, 3); assert.equal(vscode.window.activeNotebookEditor!.document.cells.indexOf(activeCell!), 1); - const edit = new vscode.WorkspaceEdit(); - edit.insert(activeCell!.uri, new vscode.Position(0, 0), 'var abc = 0;'); - await vscode.workspace.applyEdit(edit); - assert.equal(vscode.window.activeNotebookEditor !== undefined, true); - assert.equal(vscode.window.activeNotebookEditor?.selection !== undefined, true); - assert.deepEqual(vscode.window.activeNotebookEditor?.document.cells[1], vscode.window.activeNotebookEditor?.selection); - assert.equal(vscode.window.activeNotebookEditor?.selection?.document.getText(), 'var abc = 0;'); + await withEvent(vscode.workspace.onDidChangeTextDocument, async event => { + const edit = new vscode.WorkspaceEdit(); + edit.insert(activeCell!.uri, new vscode.Position(0, 0), 'var abc = 0;'); + await vscode.workspace.applyEdit(edit); + await event; + assert.equal(vscode.window.activeNotebookEditor !== undefined, true); + assert.equal(vscode.window.activeNotebookEditor?.selection !== undefined, true); + assert.deepEqual(vscode.window.activeNotebookEditor?.document.cells[1], vscode.window.activeNotebookEditor?.selection); + assert.equal(vscode.window.activeNotebookEditor?.selection?.document.getText(), 'var abc = 0;'); + }); await saveFileAndCloseAll(resource); }); diff --git a/extensions/vscode-notebook-tests/src/notebookTestMain.ts b/extensions/vscode-notebook-tests/src/notebookTestMain.ts index 9b2b91a012a..53173a336c8 100644 --- a/extensions/vscode-notebook-tests/src/notebookTestMain.ts +++ b/extensions/vscode-notebook-tests/src/notebookTestMain.ts @@ -61,15 +61,14 @@ export function activate(context: vscode.ExtensionContext): any { label: 'Notebook Test Kernel', isPreferred: true, executeAllCells: async (_document: vscode.NotebookDocument) => { - const cell = _document.cells[0]; - - cell.outputs = [{ + const edit = new vscode.WorkspaceEdit(); + edit.replaceNotebookCellOutput(_document.uri, 0, [{ outputKind: vscode.CellOutputKind.Rich, data: { 'text/plain': ['my output'] } - }]; - return; + }]); + return vscode.workspace.applyEdit(edit); }, cancelAllCellsExecution: async (_document: vscode.NotebookDocument) => { }, executeCell: async (document: vscode.NotebookDocument, cell: vscode.NotebookCell | undefined) => { @@ -78,26 +77,27 @@ export function activate(context: vscode.ExtensionContext): any { } if (document.uri.path.endsWith('customRenderer.vsctestnb')) { - cell.outputs = [{ + const edit = new vscode.WorkspaceEdit(); + edit.replaceNotebookCellOutput(document.uri, cell.index, [{ outputKind: vscode.CellOutputKind.Rich, data: { 'text/custom': 'test' } - }]; + }]); - return; + return vscode.workspace.applyEdit(edit); } + const edit = new vscode.WorkspaceEdit(); // const previousOutputs = cell.outputs; - const newOutputs: vscode.CellOutput[] = [{ + edit.replaceNotebookCellOutput(document.uri, cell.index, [{ outputKind: vscode.CellOutputKind.Rich, data: { 'text/plain': ['my output'] } - }]; + }]); - cell.outputs = newOutputs; - return; + return vscode.workspace.applyEdit(edit); }, cancelCellExecution: async (_document: vscode.NotebookDocument, _cell: vscode.NotebookCell) => { } }; @@ -107,15 +107,14 @@ export function activate(context: vscode.ExtensionContext): any { label: 'Notebook Secondary Test Kernel', isPreferred: false, executeAllCells: async (_document: vscode.NotebookDocument) => { - const cell = _document.cells[0]; - - cell.outputs = [{ + const edit = new vscode.WorkspaceEdit(); + edit.replaceNotebookCellOutput(_document.uri, 0, [{ outputKind: vscode.CellOutputKind.Rich, data: { 'text/plain': ['my second output'] } - }]; - return; + }]); + return vscode.workspace.applyEdit(edit); }, cancelAllCellsExecution: async (_document: vscode.NotebookDocument) => { }, executeCell: async (document: vscode.NotebookDocument, cell: vscode.NotebookCell | undefined) => { @@ -123,26 +122,25 @@ export function activate(context: vscode.ExtensionContext): any { cell = document.cells[0]; } + const edit = new vscode.WorkspaceEdit(); + if (document.uri.path.endsWith('customRenderer.vsctestnb')) { - cell.outputs = [{ + edit.replaceNotebookCellOutput(document.uri, cell.index, [{ outputKind: vscode.CellOutputKind.Rich, data: { 'text/custom': 'test 2' } - }]; - - return; + }]); + } else { + edit.replaceNotebookCellOutput(document.uri, cell.index, [{ + outputKind: vscode.CellOutputKind.Rich, + data: { + 'text/plain': ['my second output'] + } + }]); } - const newOutputs: vscode.CellOutput[] = [{ - outputKind: vscode.CellOutputKind.Rich, - data: { - 'text/plain': ['my second output'] - } - }]; - - cell.outputs = newOutputs; - return; + return vscode.workspace.applyEdit(edit); }, cancelCellExecution: async (_document: vscode.NotebookDocument, _cell: vscode.NotebookCell) => { } }; diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index e956e67ffc7..ab5435ac289 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1164,7 +1164,7 @@ declare module 'vscode' { readonly cells: ReadonlyArray; readonly contentOptions: NotebookDocumentContentOptions; languages: string[]; - metadata: NotebookDocumentMetadata; + readonly metadata: NotebookDocumentMetadata; } export interface NotebookCellRange { diff --git a/src/vs/workbench/api/browser/mainThreadNotebook.ts b/src/vs/workbench/api/browser/mainThreadNotebook.ts index 8da7fd54186..7e09d9d1bbf 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebook.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebook.ts @@ -23,7 +23,7 @@ import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookB import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { INotebookCellStatusBarService } from 'vs/workbench/contrib/notebook/common/notebookCellStatusBarService'; -import { ACCESSIBLE_NOTEBOOK_DISPLAY_ORDER, CellEditType, DisplayOrderKey, ICellEditOperation, ICellRange, IEditor, IMainCellDto, INotebookDecorationRenderOptions, INotebookDocumentFilter, INotebookEditorModel, INotebookExclusiveDocumentFilter, NotebookCellOutputsSplice, NotebookCellsChangeType, NOTEBOOK_DISPLAY_ORDER, TransientMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { ACCESSIBLE_NOTEBOOK_DISPLAY_ORDER, CellEditType, DisplayOrderKey, ICellEditOperation, ICellRange, IEditor, IMainCellDto, INotebookDecorationRenderOptions, INotebookDocumentFilter, INotebookEditorModel, INotebookExclusiveDocumentFilter, NotebookCellsChangeType, NOTEBOOK_DISPLAY_ORDER, TransientMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookEditorModelResolverService } from 'vs/workbench/contrib/notebook/common/notebookEditorModelResolverService'; import { IMainNotebookController, INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { IEditorGroup, IEditorGroupsService, preferredSideBySideGroupDirection } from 'vs/workbench/services/editor/common/editorGroupsService'; diff --git a/src/vs/workbench/api/common/extHostNotebookDocument.ts b/src/vs/workbench/api/common/extHostNotebookDocument.ts index f107d17ef31..390595049b6 100644 --- a/src/vs/workbench/api/common/extHostNotebookDocument.ts +++ b/src/vs/workbench/api/common/extHostNotebookDocument.ts @@ -10,10 +10,9 @@ import { Schemas } from 'vs/base/common/network'; import { joinPath } from 'vs/base/common/resources'; import { ISplice } from 'vs/base/common/sequence'; import { URI } from 'vs/base/common/uri'; -import * as UUID from 'vs/base/common/uuid'; -import { CellKind, INotebookDocumentPropertiesChangeData, IWorkspaceCellEditDto, MainThreadBulkEditsShape, MainThreadNotebookShape, NotebookCellOutputsSplice, WorkspaceEditType } from 'vs/workbench/api/common/extHost.protocol'; +import { CellKind, INotebookDocumentPropertiesChangeData, IWorkspaceCellEditDto, MainThreadBulkEditsShape, MainThreadNotebookShape, WorkspaceEditType } from 'vs/workbench/api/common/extHost.protocol'; import { ExtHostDocumentsAndEditors, IExtHostModelAddedData } from 'vs/workbench/api/common/extHostDocumentsAndEditors'; -import { CellEditType, diff, IMainCellDto, IOutputDtoWithId, NotebookCellMetadata, NotebookCellsChangedEventDto, NotebookCellsChangeType, NotebookCellsSplice2, notebookDocumentMetadataDefaults } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellEditType, IMainCellDto, IOutputDtoWithId, NotebookCellMetadata, NotebookCellsChangedEventDto, NotebookCellsChangeType, NotebookCellsSplice2, notebookDocumentMetadataDefaults } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import * as vscode from 'vscode'; @@ -76,7 +75,6 @@ export class ExtHostCell extends Disposable { private _outputMapping = new WeakMap(); private _metadata: vscode.NotebookCellMetadata; - private _metadataChangeListener: IDisposable; readonly handle: number; readonly uri: URI; @@ -85,7 +83,6 @@ export class ExtHostCell extends Disposable { private _cell: vscode.NotebookCell | undefined; constructor( - private readonly _mainThreadBulkEdits: MainThreadBulkEditsShape, private readonly _notebook: ExtHostNotebookDocument, private readonly _extHostDocument: ExtHostDocumentsAndEditors, private readonly _cellData: IMainCellDto, @@ -102,11 +99,7 @@ export class ExtHostCell extends Disposable { delete output.outputId; } - const observableMetadata = getObservable(_cellData.metadata ?? {}); - this._metadata = observableMetadata.proxy; - this._metadataChangeListener = this._register(observableMetadata.onDidChange(() => { - this._updateMetadata(); - })); + this._metadata = _cellData.metadata ?? {}; } get cell(): vscode.NotebookCell { @@ -147,26 +140,7 @@ export class ExtHostCell extends Disposable { } setMetadata(newMetadata: vscode.NotebookCellMetadata): void { - // Don't apply metadata defaults here, 'undefined' means 'inherit from document metadata' - this._metadataChangeListener.dispose(); - const observableMetadata = getObservable(newMetadata); - this._metadata = observableMetadata.proxy; - this._metadataChangeListener = this._register(observableMetadata.onDidChange(() => { - this._updateMetadata(); - })); - } - - private _updateMetadata(): Promise { - const index = this._notebook.notebookDocument.cells.indexOf(this.cell); - const edit: IWorkspaceCellEditDto = { - _type: WorkspaceEditType.Cell, - metadata: undefined, - resource: this._notebook.uri, - notebookVersionId: this._notebook.notebookDocument.version, - edit: { editType: CellEditType.Metadata, index, metadata: this._metadata } - }; - - return this._mainThreadBulkEdits.$tryApplyWorkspaceEdit({ edits: [edit] }); + this._metadata = newMetadata; } } @@ -355,7 +329,7 @@ export class ExtHostNotebookDocument extends Disposable { const cellDtos = splice[2]; const newCells = cellDtos.map(cell => { - const extCell = new ExtHostCell(this._mainThreadBulkEdits, this, this._documentsAndEditors, cell); + const extCell = new ExtHostCell(this, this._documentsAndEditors, cell); if (!initialization) { addedCellDocuments.push(ExtHostCell.asModelAddData(this.notebookDocument, cell));