From 5870204e95c504eb3c15f39295537c496bbb8765 Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Wed, 17 Feb 2021 14:19:40 +0100 Subject: [PATCH] make notebook and cell metadata classes, https://github.com/microsoft/vscode/issues/116333 --- .../notebook.document.test.ts | 4 +- .../src/singlefolder-tests/notebook.test.ts | 32 ++-- .../vscode-notebook-tests/src/extension.ts | 10 +- src/vs/vscode.proposed.d.ts | 144 ++++++------------ .../workbench/api/common/extHost.api.impl.ts | 6 + .../workbench/api/common/extHostNotebook.ts | 40 +++-- .../api/common/extHostTypeConverters.ts | 4 +- 7 files changed, 97 insertions(+), 143 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.document.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.document.test.ts index 34a8218e3a1..0d5ecc671fa 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.document.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.document.test.ts @@ -12,8 +12,8 @@ suite('Notebook Document', function () { const contentProvider = new class implements vscode.NotebookContentProvider { async openNotebook(uri: vscode.Uri, _openContext: vscode.NotebookDocumentOpenContext): Promise { return { - cells: [{ cellKind: vscode.NotebookCellKind.Code, source: uri.toString(), language: 'javascript', metadata: {}, outputs: [] }], - metadata: {} + cells: [{ cellKind: vscode.NotebookCellKind.Code, source: uri.toString(), language: 'javascript', metadata: new vscode.NotebookCellMetadata(), outputs: [] }], + metadata: new vscode.NotebookDocumentMetadata() }; } async resolveNotebook(_document: vscode.NotebookDocument, _webview: vscode.NotebookCommunication) { diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts index 81a03ed3250..c21be1935ab 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts @@ -90,24 +90,20 @@ suite('Notebook API tests', function () { openNotebook: async (_resource: vscode.Uri): Promise => { if (/.*empty\-.*\.vsctestnb$/.test(_resource.path)) { return { - metadata: {}, + metadata: new vscode.NotebookDocumentMetadata(), cells: [] }; } const dto: vscode.NotebookData = { - metadata: { - custom: { testMetadata: false } - }, + metadata: new vscode.NotebookDocumentMetadata().with({ custom: { testMetadata: false } }), cells: [ { source: 'test', language: 'typescript', cellKind: vscode.NotebookCellKind.Code, outputs: [], - metadata: { - custom: { testCellMetadata: 123 } - } + metadata: new vscode.NotebookCellMetadata().with({ custom: { testCellMetadata: 123 } }) } ] }; @@ -599,7 +595,7 @@ suite('Notebook API tests', function () { await vscode.commands.executeCommand('vscode.openWith', resource, 'notebookCoreTest'); await vscode.window.activeNotebookEditor!.edit(editBuilder => { - editBuilder.replaceCellMetadata(0, { inputCollapsed: true, executionOrder: 17 }); + editBuilder.replaceCellMetadata(0, new vscode.NotebookCellMetadata().with({ inputCollapsed: true, executionOrder: 17 })); }); const document = vscode.window.activeNotebookEditor?.document!; @@ -620,7 +616,7 @@ suite('Notebook API tests', function () { const event = asPromise(vscode.notebook.onDidChangeCellMetadata); await vscode.window.activeNotebookEditor!.edit(editBuilder => { - editBuilder.replaceCellMetadata(0, { inputCollapsed: true, executionOrder: 17 }); + editBuilder.replaceCellMetadata(0, new vscode.NotebookCellMetadata().with({ inputCollapsed: true, executionOrder: 17 })); }); const data = await event; @@ -642,7 +638,7 @@ suite('Notebook API tests', function () { const version = vscode.window.activeNotebookEditor!.document.version; await vscode.window.activeNotebookEditor!.edit(editBuilder => { editBuilder.replaceCells(1, 0, [{ cellKind: vscode.NotebookCellKind.Code, language: 'javascript', source: 'test 2', outputs: [], metadata: undefined }]); - editBuilder.replaceCellMetadata(0, { runnable: false }); + editBuilder.replaceCellMetadata(0, new vscode.NotebookCellMetadata().with({ runnable: false })); }); await cellsChangeEvent; @@ -661,7 +657,7 @@ suite('Notebook API tests', function () { const version = vscode.window.activeNotebookEditor!.document.version; await vscode.window.activeNotebookEditor!.edit(editBuilder => { editBuilder.replaceCells(1, 0, [{ cellKind: vscode.NotebookCellKind.Code, language: 'javascript', source: 'test 2', outputs: [], metadata: undefined }]); - editBuilder.replaceCellMetadata(0, { runnable: false }); + editBuilder.replaceCellMetadata(0, new vscode.NotebookCellMetadata().with({ runnable: false })); }); await cellsChangeEvent; @@ -876,14 +872,14 @@ suite('Notebook API tests', function () { assert.strictEqual(cell.outputs.length, 0); let metadataChangeEvent = asPromise(vscode.notebook.onDidChangeCellMetadata); - await updateCellMetadata(resource, cell, { ...cell.metadata, runnable: false }); + await updateCellMetadata(resource, cell, cell.metadata.with({ runnable: false })); await metadataChangeEvent; await vscode.commands.executeCommand('notebook.cell.execute'); assert.strictEqual(cell.outputs.length, 0, 'should not execute'); // not runnable, didn't work metadataChangeEvent = asPromise(vscode.notebook.onDidChangeCellMetadata); - await updateCellMetadata(resource, cell, { ...cell.metadata, runnable: true }); + await updateCellMetadata(resource, cell, cell.metadata.with({ runnable: true })); await metadataChangeEvent; await vscode.commands.executeCommand('notebook.cell.execute'); @@ -904,7 +900,7 @@ suite('Notebook API tests', function () { assert.strictEqual(cell.outputs.length, 0); await withEvent(vscode.notebook.onDidChangeNotebookDocumentMetadata, async event => { - updateNotebookMetadata(editor.document.uri, { ...editor.document.metadata, runnable: false }); + updateNotebookMetadata(editor.document.uri, editor.document.metadata.with({ runnable: false })); await event; }); @@ -912,7 +908,7 @@ suite('Notebook API tests', function () { assert.strictEqual(cell.outputs.length, 0, 'should not execute'); // not runnable, didn't work await withEvent(vscode.notebook.onDidChangeNotebookDocumentMetadata, async event => { - updateNotebookMetadata(editor.document.uri, { ...editor.document.metadata, runnable: true }); + updateNotebookMetadata(editor.document.uri, editor.document.metadata.with({ runnable: true })); await event; }); @@ -952,7 +948,7 @@ suite('Notebook API tests', function () { const cell = editor.document.cells[0]; await withEvent(vscode.notebook.onDidChangeNotebookDocumentMetadata, async event => { - updateNotebookMetadata(editor.document.uri, { ...editor.document.metadata, runnable: true }); + updateNotebookMetadata(editor.document.uri, editor.document.metadata.with({ runnable: true })); await event; }); @@ -993,7 +989,7 @@ suite('Notebook API tests', function () { const cell = editor.document.cells[0]; const metadataChangeEvent = asPromise(vscode.notebook.onDidChangeNotebookDocumentMetadata); - updateNotebookMetadata(editor.document.uri, { ...editor.document.metadata, runnable: true }); + updateNotebookMetadata(editor.document.uri, editor.document.metadata.with({ runnable: true })); await metadataChangeEvent; assert.strictEqual(editor.document.metadata.runnable, true); @@ -1033,7 +1029,7 @@ suite('Notebook API tests', function () { const cell = editor.document.cells[0]; const metadataChangeEvent = asPromise(vscode.notebook.onDidChangeNotebookDocumentMetadata); - updateNotebookMetadata(editor.document.uri, { ...editor.document.metadata, runnable: true }); + updateNotebookMetadata(editor.document.uri, editor.document.metadata.with({ runnable: true })); await metadataChangeEvent; await vscode.commands.executeCommand('notebook.cell.execute'); diff --git a/extensions/vscode-notebook-tests/src/extension.ts b/extensions/vscode-notebook-tests/src/extension.ts index bf66b529bc2..55231411594 100644 --- a/extensions/vscode-notebook-tests/src/extension.ts +++ b/extensions/vscode-notebook-tests/src/extension.ts @@ -23,25 +23,21 @@ export function activate(context: vscode.ExtensionContext): any { context.subscriptions.push(vscode.notebook.registerNotebookContentProvider('notebookSmokeTest', { openNotebook: async (_resource: vscode.Uri) => { const dto: vscode.NotebookData = { - metadata: {}, + metadata: new vscode.NotebookDocumentMetadata(), cells: [ { source: 'code()', language: 'typescript', cellKind: vscode.NotebookCellKind.Code, outputs: [], - metadata: { - custom: { testCellMetadata: 123 } - } + metadata: new vscode.NotebookCellMetadata().with({ custom: { testCellMetadata: 123 } }) }, { source: 'Markdown Cell', language: 'markdown', cellKind: vscode.NotebookCellKind.Markdown, outputs: [], - metadata: { - custom: { testCellMetadata: 123 } - } + metadata: new vscode.NotebookCellMetadata().with({ custom: { testCellMetadata: 123 } }) } ] }; diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index a06d8165fab..ce9a5668450 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1162,34 +1162,44 @@ declare module 'vscode' { Idle = 2 } - // TODO@API - // make this a class, allow modified using with-pattern - export interface NotebookCellMetadata { + export class NotebookCellMetadata { /** * Controls whether a cell's editor is editable/readonly. */ - editable?: boolean; - + readonly editable?: boolean; /** * Controls if the cell has a margin to support the breakpoint UI. * This metadata is ignored for markdown cell. */ - breakpointMargin?: boolean; - + readonly breakpointMargin?: boolean; /** * Whether a code cell's editor is collapsed */ - inputCollapsed?: boolean; - + readonly outputCollapsed?: boolean; /** * Whether a code cell's outputs are collapsed */ - outputCollapsed?: boolean; - + readonly inputCollapsed?: boolean; /** * Additional attributes of a cell metadata. */ - custom?: { [key: string]: any; }; + readonly custom?: Record; + + // todo@API duplicates status bar API + readonly statusMessage?: string; + + // run related API, will be removed + readonly runnable?: boolean; + readonly hasExecutionOrder?: boolean; + readonly executionOrder?: number; + readonly runState?: NotebookCellRunState; + readonly runStartTime?: number; + readonly lastRunDuration?: number; + + constructor(editable?: boolean, breakpointMargin?: boolean, runnable?: boolean, hasExecutionOrder?: boolean, executionOrder?: number, runState?: NotebookCellRunState, runStartTime?: number, statusMessage?: string, lastRunDuration?: number, inputCollapsed?: boolean, outputCollapsed?: boolean, custom?: Record) + + // todo@API write a proper signature + with(change: Partial>): NotebookCellMetadata; } // todo@API support ids https://github.com/jupyter/enhancement-proposals/blob/master/62-cell-id/cell-id.md @@ -1201,39 +1211,48 @@ declare module 'vscode' { readonly document: TextDocument; readonly language: string; readonly outputs: readonly NotebookCellOutput[]; - readonly metadata: NotebookCellMetadata; - /** @deprecated use WorkspaceEdit.replaceCellOutput */ - // outputs: CellOutput[]; - // readonly outputs2: NotebookCellOutput[]; - /** @deprecated use WorkspaceEdit.replaceCellMetadata */ - // metadata: NotebookCellMetadata; + readonly metadata: NotebookCellMetadata } + export class NotebookDocumentMetadata { - export interface NotebookDocumentMetadata { /** * Controls if users can add or delete cells * Defaults to true */ - editable?: boolean; - + readonly editable: boolean; /** * Default value for [cell editable metadata](#NotebookCellMetadata.editable). * Defaults to true. */ - cellEditable?: boolean; - displayOrder?: GlobPattern[]; - + readonly cellEditable: boolean; /** * Additional attributes of the document metadata. */ - custom?: { [key: string]: any; }; - + readonly custom: { [key: string]: any; }; /** * Whether the document is trusted, default to true * When false, insecure outputs like HTML, JavaScript, SVG will not be rendered. */ - trusted?: boolean; + readonly trusted: boolean; + + // todo@API how does glob apply to mime times? + readonly displayOrder: GlobPattern[]; + + // todo@API is this a kernel property? + readonly cellHasExecutionOrder: boolean; + + // run related, remove infer from kernel, exec + // todo@API infer from kernel + // todo@API remove + readonly runnable: boolean; + readonly cellRunnable: boolean; + readonly runState: NotebookRunState; + + constructor(editable?: boolean, runnable?: boolean, cellEditable?: boolean, cellRunnable?: boolean, cellHasExecutionOrder?: boolean, displayOrder?: GlobPattern[], custom?: { [key: string]: any; }, runState?: NotebookRunState, trusted?: boolean); + + // TODO@API make this a proper signature + with(change: Partial>): NotebookDocumentMetadata; } export interface NotebookDocumentContentOptions { @@ -1640,34 +1659,6 @@ declare module 'vscode' { //#region https://github.com/microsoft/vscode/issues/106744, NotebookKernel - export interface NotebookDocumentMetadata { - - /** - * Controls whether the full notebook can be run at once. - * Defaults to true - */ - // todo@API infer from kernel - // todo@API remove - runnable?: boolean; - - /** - * Default value for [cell runnable metadata](#NotebookCellMetadata.runnable). - * Defaults to true. - */ - cellRunnable?: boolean; - - /** - * Default value for [cell hasExecutionOrder metadata](#NotebookCellMetadata.hasExecutionOrder). - * Defaults to true. - */ - cellHasExecutionOrder?: boolean; - - /** - * The document's current run state - */ - runState?: NotebookRunState; - } - // todo@API use the NotebookCellExecution-object as a container to model and enforce // the flow of a cell execution @@ -1689,49 +1680,6 @@ declare module 'vscode' { // export const onDidStartNotebookCellExecution: Event; // export const onDidStopNotebookCellExecution: Event; - export interface NotebookCellMetadata { - - /** - * Controls if the cell is executable. - * This metadata is ignored for markdown cell. - */ - // todo@API infer from kernel - runnable?: boolean; - - /** - * Whether the [execution order](#NotebookCellMetadata.executionOrder) indicator will be displayed. - * Defaults to true. - */ - hasExecutionOrder?: boolean; - - /** - * The order in which this cell was executed. - */ - executionOrder?: number; - - /** - * A status message to be shown in the cell's status bar - */ - // todo@API duplicates status bar API - statusMessage?: string; - - /** - * The cell's current run state - */ - runState?: NotebookCellRunState; - - /** - * If the cell is running, the time at which the cell started running - */ - runStartTime?: number; - - /** - * The total duration of the cell's last run - */ - // todo@API depends on having output - lastRunDuration?: number; - } - export interface NotebookKernel { readonly id?: string; label: string; diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index 32d13f792e0..155c978701a 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -1273,6 +1273,12 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I // checkProposedApiEnabled(extension); return extHostTypes.NotebookCellRunState; }, + get NotebookDocumentMetadata() { + return extHostTypes.NotebookDocumentMetadata; + }, + get NotebookCellMetadata() { + return extHostTypes.NotebookCellMetadata; + }, get NotebookRunState() { // checkProposedApiEnabled(extension); return extHostTypes.NotebookRunState; diff --git a/src/vs/workbench/api/common/extHostNotebook.ts b/src/vs/workbench/api/common/extHostNotebook.ts index 04a5cd56a2c..e6ba271e43d 100644 --- a/src/vs/workbench/api/common/extHostNotebook.ts +++ b/src/vs/workbench/api/common/extHostNotebook.ts @@ -714,23 +714,31 @@ export class ExtHostNotebookController implements ExtHostNotebookShape, ExtHostN } const that = this; - const document = new ExtHostNotebookDocument(this._documentsAndEditors, { - emitModelChange(event: vscode.NotebookCellsChangeEvent): void { - that._onDidChangeNotebookCells.fire(event); + const document = new ExtHostNotebookDocument( + this._documentsAndEditors, + { + emitModelChange(event: vscode.NotebookCellsChangeEvent): void { + that._onDidChangeNotebookCells.fire(event); + }, + emitCellOutputsChange(event: vscode.NotebookCellOutputsChangeEvent): void { + that._onDidChangeCellOutputs.fire(event); + }, + emitCellLanguageChange(event: vscode.NotebookCellLanguageChangeEvent): void { + that._onDidChangeCellLanguage.fire(event); + }, + emitCellMetadataChange(event: vscode.NotebookCellMetadataChangeEvent): void { + that._onDidChangeCellMetadata.fire(event); + }, + emitDocumentMetadataChange(event: vscode.NotebookDocumentMetadataChangeEvent): void { + that._onDidChangeNotebookDocumentMetadata.fire(event); + } }, - emitCellOutputsChange(event: vscode.NotebookCellOutputsChangeEvent): void { - that._onDidChangeCellOutputs.fire(event); - }, - emitCellLanguageChange(event: vscode.NotebookCellLanguageChangeEvent): void { - that._onDidChangeCellLanguage.fire(event); - }, - emitCellMetadataChange(event: vscode.NotebookCellMetadataChangeEvent): void { - that._onDidChangeCellMetadata.fire(event); - }, - emitDocumentMetadataChange(event: vscode.NotebookDocumentMetadataChangeEvent): void { - that._onDidChangeNotebookDocumentMetadata.fire(event); - } - }, viewType, modelData.contentOptions, typeConverters.NotebookDocumentMetadata.to(modelData.metadata ?? {}), uri, storageRoot); + viewType, + modelData.contentOptions, + modelData.metadata ? typeConverters.NotebookDocumentMetadata.to(modelData.metadata) : new extHostTypes.NotebookDocumentMetadata(), + uri, + storageRoot + ); document.acceptModelChanged({ versionId: modelData.versionId, diff --git a/src/vs/workbench/api/common/extHostTypeConverters.ts b/src/vs/workbench/api/common/extHostTypeConverters.ts index 039418367df..1c0301a8ac2 100644 --- a/src/vs/workbench/api/common/extHostTypeConverters.ts +++ b/src/vs/workbench/api/common/extHostTypeConverters.ts @@ -1418,7 +1418,7 @@ export namespace NotebookCellRange { export namespace NotebookCellMetadata { - export function to(data: vscode.NotebookCellMetadata): types.NotebookCellMetadata { + export function to(data: notebooks.NotebookCellMetadata): types.NotebookCellMetadata { return new types.NotebookCellMetadata(data.editable, data.breakpointMargin, data.runnable, data.hasExecutionOrder, data.executionOrder, data.runStartTime, data.runStartTime, data.statusMessage, data.lastRunDuration, data.inputCollapsed, data.outputCollapsed, data.custom); } } @@ -1429,7 +1429,7 @@ export namespace NotebookDocumentMetadata { return data; } - export function to(data: vscode.NotebookDocumentMetadata): types.NotebookDocumentMetadata { + export function to(data: notebooks.NotebookDocumentMetadata): types.NotebookDocumentMetadata { return new types.NotebookDocumentMetadata(data.editable, data.runnable, data.cellEditable, data.cellRunnable, data.cellHasExecutionOrder, data.displayOrder, data.custom, data.runState, data.trusted); }