From 3edc9925e056d7f35733060370fa87833ef5515c Mon Sep 17 00:00:00 2001 From: Johannes Rieken Date: Mon, 8 Feb 2021 18:38:17 +0100 Subject: [PATCH] NotebookDocument#metadata is readonly --- .../src/notebook.test.ts | 16 ++-- .../workbench/api/common/extHostNotebook.ts | 6 +- .../api/common/extHostNotebookDocument.ts | 85 ++----------------- 3 files changed, 21 insertions(+), 86 deletions(-) diff --git a/extensions/vscode-notebook-tests/src/notebook.test.ts b/extensions/vscode-notebook-tests/src/notebook.test.ts index 0a2a97ac8ad..861f821716d 100644 --- a/extensions/vscode-notebook-tests/src/notebook.test.ts +++ b/extensions/vscode-notebook-tests/src/notebook.test.ts @@ -93,6 +93,12 @@ async function updateCellMetadata(uri: vscode.Uri, cell: vscode.NotebookCell, ne await vscode.workspace.applyEdit(edit); } +async function updateNotebookMetadata(uri: vscode.Uri, newMetadata: vscode.NotebookDocumentMetadata) { + const edit = new vscode.WorkspaceEdit(); + edit.replaceNotebookMetadata(uri, newMetadata); + await vscode.workspace.applyEdit(edit); +} + async function withEvent(event: vscode.Event, callback: (e: Promise) => Promise) { const e = getEventOncePromise(event); await callback(e); @@ -931,7 +937,7 @@ suite('notebook workflow', () => { assert.strictEqual(cell.outputs.length, 0); await withEvent(vscode.notebook.onDidChangeNotebookDocumentMetadata, async event => { - editor.document.metadata.runnable = false; + updateNotebookMetadata(editor.document.uri, { ...editor.document.metadata, runnable: false }); await event; }); @@ -939,7 +945,7 @@ suite('notebook workflow', () => { assert.strictEqual(cell.outputs.length, 0, 'should not execute'); // not runnable, didn't work await withEvent(vscode.notebook.onDidChangeNotebookDocumentMetadata, async event => { - editor.document.metadata.runnable = true; + updateNotebookMetadata(editor.document.uri, { ...editor.document.metadata, runnable: true }); await event; }); @@ -979,7 +985,7 @@ suite('notebook workflow', () => { const cell = editor.document.cells[0]; await withEvent(vscode.notebook.onDidChangeNotebookDocumentMetadata, async event => { - editor.document.metadata.runnable = true; + updateNotebookMetadata(editor.document.uri, { ...editor.document.metadata, runnable: true }); await event; }); @@ -1020,7 +1026,7 @@ suite('notebook workflow', () => { const cell = editor.document.cells[0]; const metadataChangeEvent = getEventOncePromise(vscode.notebook.onDidChangeNotebookDocumentMetadata); - editor.document.metadata.runnable = true; + updateNotebookMetadata(editor.document.uri, { ...editor.document.metadata, runnable: true }); await metadataChangeEvent; assert.strictEqual(editor.document.metadata.runnable, true); @@ -1060,7 +1066,7 @@ suite('notebook workflow', () => { const cell = editor.document.cells[0]; const metadataChangeEvent = getEventOncePromise(vscode.notebook.onDidChangeNotebookDocumentMetadata); - editor.document.metadata.runnable = true; + updateNotebookMetadata(editor.document.uri, { ...editor.document.metadata, runnable: true }); await metadataChangeEvent; await vscode.commands.executeCommand('notebook.cell.execute'); diff --git a/src/vs/workbench/api/common/extHostNotebook.ts b/src/vs/workbench/api/common/extHostNotebook.ts index cb48c971899..cc172f8d0ad 100644 --- a/src/vs/workbench/api/common/extHostNotebook.ts +++ b/src/vs/workbench/api/common/extHostNotebook.ts @@ -9,7 +9,7 @@ import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; import { URI, UriComponents } from 'vs/base/common/uri'; import * as UUID from 'vs/base/common/uuid'; import { IExtensionDescription } from 'vs/platform/extensions/common/extensions'; -import { ExtHostNotebookShape, ICommandDto, IMainContext, IModelAddedData, INotebookDocumentPropertiesChangeData, INotebookDocumentsAndEditorsDelta, INotebookDocumentShowOptions, INotebookEditorPropertiesChangeData, MainContext, MainThreadBulkEditsShape, MainThreadNotebookShape } from 'vs/workbench/api/common/extHost.protocol'; +import { ExtHostNotebookShape, ICommandDto, IMainContext, IModelAddedData, INotebookDocumentPropertiesChangeData, INotebookDocumentsAndEditorsDelta, INotebookDocumentShowOptions, INotebookEditorPropertiesChangeData, MainContext, MainThreadNotebookShape } from 'vs/workbench/api/common/extHost.protocol'; import { ILogService } from 'vs/platform/log/common/log'; import { CommandsConverter, ExtHostCommands } from 'vs/workbench/api/common/extHostCommands'; import { ExtHostDocumentsAndEditors } from 'vs/workbench/api/common/extHostDocumentsAndEditors'; @@ -214,7 +214,6 @@ export class ExtHostNotebookController implements ExtHostNotebookShape, ExtHostN private static _notebookKernelProviderHandlePool: number = 0; private readonly _proxy: MainThreadNotebookShape; - private readonly _mainThreadBulkEdits: MainThreadBulkEditsShape; private readonly _notebookContentProviders = new Map(); private readonly _notebookKernelProviders = new Map(); private readonly _documents = new ResourceMap(); @@ -271,7 +270,6 @@ export class ExtHostNotebookController implements ExtHostNotebookShape, ExtHostN private readonly _extensionStoragePaths: IExtensionStoragePaths, ) { this._proxy = mainContext.getProxy(MainContext.MainThreadNotebook); - this._mainThreadBulkEdits = mainContext.getProxy(MainContext.MainThreadBulkEdits); this._commandsConverter = commands.converter; commands.registerArgumentProcessor({ @@ -708,7 +706,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape, ExtHostN } const that = this; - const document = new ExtHostNotebookDocument(this._proxy, this._documentsAndEditors, this._mainThreadBulkEdits, { + const document = new ExtHostNotebookDocument(this._proxy, this._documentsAndEditors, { emitModelChange(event: vscode.NotebookCellsChangeEvent): void { that._onDidChangeNotebookCells.fire(event); }, diff --git a/src/vs/workbench/api/common/extHostNotebookDocument.ts b/src/vs/workbench/api/common/extHostNotebookDocument.ts index 8bec8c0923a..e06d7074543 100644 --- a/src/vs/workbench/api/common/extHostNotebookDocument.ts +++ b/src/vs/workbench/api/common/extHostNotebookDocument.ts @@ -5,40 +5,18 @@ import { Emitter, Event } from 'vs/base/common/event'; import { hash } from 'vs/base/common/hash'; -import { Disposable, DisposableStore, dispose, IDisposable } from 'vs/base/common/lifecycle'; +import { Disposable, DisposableStore, dispose } from 'vs/base/common/lifecycle'; 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 { CellKind, INotebookDocumentPropertiesChangeData, IWorkspaceCellEditDto, MainThreadBulkEditsShape, MainThreadNotebookShape, WorkspaceEditType } from 'vs/workbench/api/common/extHost.protocol'; +import { CellKind, INotebookDocumentPropertiesChangeData, MainThreadNotebookShape } from 'vs/workbench/api/common/extHost.protocol'; import { ExtHostDocumentsAndEditors, IExtHostModelAddedData } from 'vs/workbench/api/common/extHostDocumentsAndEditors'; import { NotebookCellOutput } from 'vs/workbench/api/common/extHostTypes'; import * as extHostTypeConverters from 'vs/workbench/api/common/extHostTypeConverters'; -import { CellEditType, IMainCellDto, IOutputDto, NotebookCellMetadata, NotebookCellsChangedEventDto, NotebookCellsChangeType, NotebookCellsSplice2, notebookDocumentMetadataDefaults } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { IMainCellDto, IOutputDto, NotebookCellMetadata, NotebookCellsChangedEventDto, NotebookCellsChangeType, NotebookCellsSplice2, notebookDocumentMetadataDefaults } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import * as vscode from 'vscode'; - -interface IObservable { - proxy: T; - onDidChange: Event; -} - -function getObservable(obj: T): IObservable { - const onDidChange = new Emitter(); - const proxy = new Proxy(obj, { - set(target: T, p: PropertyKey, value: any, _receiver: any): boolean { - target[p as keyof T] = value; - onDidChange.fire(); - return true; - } - }); - - return { - proxy, - onDidChange: onDidChange.event - }; -} - class RawContentChangeEvent { constructor(readonly start: number, readonly deletedCount: number, readonly deletedItems: ExtHostCell[], readonly items: ExtHostCell[]) { } @@ -164,8 +142,8 @@ export class ExtHostNotebookDocument extends Disposable { private _cellDisposableMapping = new Map(); private _notebook: vscode.NotebookDocument | undefined; - private _metadata: Required; - private _metadataChangeListener: IDisposable; + // private _metadata: Required; + // private _metadataChangeListener: IDisposable; private _versionId = 0; private _isDirty: boolean = false; private _backupCounter = 1; @@ -176,21 +154,14 @@ export class ExtHostNotebookDocument extends Disposable { constructor( private readonly _proxy: MainThreadNotebookShape, private readonly _documentsAndEditors: ExtHostDocumentsAndEditors, - private readonly _mainThreadBulkEdits: MainThreadBulkEditsShape, private readonly _emitter: INotebookEventEmitter, private readonly _viewType: string, private readonly _contentOptions: vscode.NotebookDocumentContentOptions, - metadata: Required, + private _metadata: Required, public readonly uri: URI, private readonly _storagePath: URI | undefined ) { super(); - - const observableMetadata = getObservable(metadata); - this._metadata = observableMetadata.proxy; - this._metadataChangeListener = this._register(observableMetadata.onDidChange(() => { - this._tryUpdateMetadata(); - })); } dispose() { @@ -199,36 +170,6 @@ export class ExtHostNotebookDocument extends Disposable { dispose(this._cellDisposableMapping.values()); } - private _updateMetadata(newMetadata: Required) { - this._metadataChangeListener.dispose(); - newMetadata = { - ...notebookDocumentMetadataDefaults, - ...newMetadata - }; - if (this._metadataChangeListener) { - this._metadataChangeListener.dispose(); - } - - const observableMetadata = getObservable(newMetadata); - this._metadata = observableMetadata.proxy; - this._metadataChangeListener = this._register(observableMetadata.onDidChange(() => { - this._tryUpdateMetadata(); - })); - - this._tryUpdateMetadata(); - } - - private _tryUpdateMetadata() { - const edit: IWorkspaceCellEditDto = { - _type: WorkspaceEditType.Cell, - metadata: undefined, - edit: { editType: CellEditType.DocumentMetadata, metadata: this._metadata }, - resource: this.uri, - notebookVersionId: this.notebookDocument.version, - }; - - return this._mainThreadBulkEdits.$tryApplyWorkspaceEdit({ edits: [edit] }); - } get notebookDocument(): vscode.NotebookDocument { if (!this._notebook) { @@ -244,7 +185,7 @@ export class ExtHostNotebookDocument extends Disposable { get languages() { return that._languages; }, set languages(value: string[]) { that._trySetLanguages(value); }, get metadata() { return that._metadata; }, - set metadata(value: Required) { that._updateMetadata(value); }, + set metadata(_value: Required) { throw new Error('Use WorkspaceEdit to update metadata.'); }, get contentOptions() { return that._contentOptions; } }); } @@ -279,17 +220,7 @@ export class ExtHostNotebookDocument extends Disposable { ...notebookDocumentMetadataDefaults, ...data.metadata }; - - if (this._metadataChangeListener) { - this._metadataChangeListener.dispose(); - } - - const observableMetadata = getObservable(newMetadata); - this._metadata = observableMetadata.proxy; - this._metadataChangeListener = this._register(observableMetadata.onDidChange(() => { - this._tryUpdateMetadata(); - })); - + this._metadata = newMetadata; this._emitter.emitDocumentMetadataChange({ document: this.notebookDocument }); }