From 03960a5aa1c21f8dc6db2211e0a62a736624ff71 Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 9 Sep 2020 16:38:52 -0700 Subject: [PATCH] re #105735. no more udpateMetadata api. --- .../src/notebook.test.ts | 6 ++ src/vs/vscode.proposed.d.ts | 5 ++ .../api/browser/mainThreadNotebook.ts | 19 ++-- .../workbench/api/common/extHost.api.impl.ts | 4 + .../workbench/api/common/extHost.protocol.ts | 1 - .../workbench/api/common/extHostNotebook.ts | 10 ++- .../api/common/extHostNotebookDocument.ts | 34 ++++++- .../api/common/extHostNotebookEditor.ts | 3 +- .../contrib/bulkEdit/browser/bulkCellEdits.ts | 7 +- .../common/model/notebookTextModel.ts | 89 +++++++++++-------- .../contrib/notebook/common/notebookCommon.ts | 10 ++- 11 files changed, 128 insertions(+), 60 deletions(-) diff --git a/extensions/vscode-notebook-tests/src/notebook.test.ts b/extensions/vscode-notebook-tests/src/notebook.test.ts index 746d7a303b23b..0547012433ad7 100644 --- a/extensions/vscode-notebook-tests/src/notebook.test.ts +++ b/extensions/vscode-notebook-tests/src/notebook.test.ts @@ -718,11 +718,17 @@ 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 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 vscode.commands.executeCommand('notebook.execute'); assert.equal(cell.outputs.length, 1, 'should execute'); // runnable, it worked diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 5741ff5aa8557..108a6515e425f 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -1470,6 +1470,10 @@ declare module 'vscode' { outputId: string; } + export interface NotebookDocumentMetadataChangeEvent { + readonly document: NotebookDocument; + } + export interface NotebookCellsChangeData { readonly start: number; readonly deletedCount: number; @@ -1747,6 +1751,7 @@ declare module 'vscode' { export const onDidChangeActiveNotebookEditor: Event; export const onDidChangeNotebookEditorSelection: Event; export const onDidChangeNotebookEditorVisibleRanges: Event; + export const onDidChangeNotebookDocumentMetadata: Event; export const onDidChangeNotebookCells: Event; export const onDidChangeCellOutputs: Event; export const onDidChangeCellLanguage: Event; diff --git a/src/vs/workbench/api/browser/mainThreadNotebook.ts b/src/vs/workbench/api/browser/mainThreadNotebook.ts index b931d5b17aa6a..582fd07d1650f 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebook.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebook.ts @@ -162,9 +162,13 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo return false; } this._notebookService.transformEditsOutputs(textModel, cellEdits); - //TODO@rebornix should this go into applyEdit? if (newMetadata) { - textModel.updateNotebookMetadata(newMetadata); + textModel.applyEdit(textModel.versionId, [ + { + editType: CellEditType.DocumentMetadata, + metadata: newMetadata + } + ], true); } return textModel.applyEdit(modelVersionId, cellEdits, true); } @@ -306,9 +310,10 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo * Since `e.transient` decides if the model should be dirty or not, we will use the same logic here. */ this._proxy.$acceptModelChanged(textModel.uri, data, !e.transient); - this._proxy.$acceptDocumentPropertiesChanged(textModel.uri, { metadata: null }); + if (e.kind === NotebookCellsChangeType.ChangeDocumentMetadata) { + this._proxy.$acceptDocumentPropertiesChanged(textModel.uri, { metadata: textModel.metadata }); + } })); - this._editorEventListenersMapping.set(textModel!.uri.toString(), disposableStore); } }; @@ -560,12 +565,6 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo textModel?.updateLanguages(languages); } - async $updateNotebookMetadata(viewType: string, resource: UriComponents, metadata: NotebookDocumentMetadata): Promise { - this.logService.debug('MainThreadNotebooks#updateNotebookMetadata', resource.path, metadata); - const textModel = this._notebookService.getNotebookTextModel(URI.from(resource)); - textModel?.updateNotebookMetadata(metadata); - } - async $spliceNotebookCellOutputs(viewType: string, resource: UriComponents, cellHandle: number, splices: NotebookCellOutputsSplice[]): Promise { this.logService.debug('MainThreadNotebooks#spliceNotebookCellOutputs', resource.path, cellHandle); const textModel = this._notebookService.getNotebookTextModel(URI.from(resource)); diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index e5e5b3cc88d39..46a0b734f5be1 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -971,6 +971,10 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I checkProposedApiEnabled(extension); return extHostNotebook.onDidChangeActiveNotebookEditor(listener, thisArgs, disposables); }, + onDidChangeNotebookDocumentMetadata(listener, thisArgs?, disposables?) { + checkProposedApiEnabled(extension); + return extHostNotebook.onDidChangeNotebookDocumentMetadata(listener, thisArgs, disposables); + }, onDidChangeNotebookCells(listener, thisArgs?, disposables?) { checkProposedApiEnabled(extension); return extHostNotebook.onDidChangeNotebookCells(listener, thisArgs, disposables); diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 799bf64eb544a..2f84c7625b141 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -743,7 +743,6 @@ export interface MainThreadNotebookShape extends IDisposable { $onNotebookKernelChange(handle: number, uri: UriComponents | undefined): void; $tryApplyEdits(viewType: string, resource: UriComponents, modelVersionId: number, edits: ICellEditOperation[], metadata: NotebookDocumentMetadata | undefined): Promise; $updateNotebookLanguages(viewType: string, resource: UriComponents, languages: string[]): Promise; - $updateNotebookMetadata(viewType: string, resource: UriComponents, metadata: NotebookDocumentMetadata): Promise; $spliceNotebookCellOutputs(viewType: string, resource: UriComponents, cellHandle: number, splices: NotebookCellOutputsSplice[]): Promise; $postMessage(editorId: string, forRendererId: string | undefined, value: any): Promise; $setStatusBarEntry(id: number, statusBarEntry: INotebookCellStatusBarEntryDto): Promise; diff --git a/src/vs/workbench/api/common/extHostNotebook.ts b/src/vs/workbench/api/common/extHostNotebook.ts index 56be870c0989c..128cdec9c14a7 100644 --- a/src/vs/workbench/api/common/extHostNotebook.ts +++ b/src/vs/workbench/api/common/extHostNotebook.ts @@ -203,6 +203,8 @@ export class ExtHostNotebookController implements ExtHostNotebookShape, ExtHostN readonly onDidChangeNotebookEditorSelection = this._onDidChangeNotebookEditorSelection.event; private readonly _onDidChangeNotebookEditorVisibleRanges = new Emitter(); readonly onDidChangeNotebookEditorVisibleRanges = this._onDidChangeNotebookEditorVisibleRanges.event; + private readonly _onDidChangeNotebookDocumentMetadata = new Emitter(); + readonly onDidChangeNotebookDocumentMetadata = this._onDidChangeNotebookDocumentMetadata.event; private readonly _onDidChangeNotebookCells = new Emitter(); readonly onDidChangeNotebookCells = this._onDidChangeNotebookCells.event; private readonly _onDidChangeCellOutputs = new Emitter(); @@ -604,10 +606,7 @@ export class ExtHostNotebookController implements ExtHostNotebookShape, ExtHostN } if (data.metadata) { - editor.editor.notebookData.notebookDocument.metadata = { - ...notebookDocumentMetadataDefaults, - ...data.metadata - }; + editor.editor.notebookData.acceptDocumentPropertiesChanged(data); } } @@ -693,6 +692,9 @@ export class ExtHostNotebookController implements ExtHostNotebookShape, ExtHostN }, emitCellMetadataChange(event: vscode.NotebookCellMetadataChangeEvent): void { that._onDidChangeCellMetadata.fire(event); + }, + emitDocumentMetadataChange(event: vscode.NotebookDocumentMetadataChangeEvent): void { + that._onDidChangeNotebookDocumentMetadata.fire(event); } }, viewType, { ...notebookDocumentMetadataDefaults, ...modelData.metadata }, uri, storageRoot); diff --git a/src/vs/workbench/api/common/extHostNotebookDocument.ts b/src/vs/workbench/api/common/extHostNotebookDocument.ts index f7f77c9e9f2c8..ac83b9de09362 100644 --- a/src/vs/workbench/api/common/extHostNotebookDocument.ts +++ b/src/vs/workbench/api/common/extHostNotebookDocument.ts @@ -11,7 +11,7 @@ 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, IWorkspaceCellEditDto, MainThreadBulkEditsShape, MainThreadNotebookShape, NotebookCellOutputsSplice, WorkspaceEditType } from 'vs/workbench/api/common/extHost.protocol'; +import { CellKind, INotebookDocumentPropertiesChangeData, IWorkspaceCellEditDto, MainThreadBulkEditsShape, MainThreadNotebookShape, NotebookCellOutputsSplice, WorkspaceEditType } from 'vs/workbench/api/common/extHost.protocol'; import { ExtHostDocumentsAndEditors, IExtHostModelAddedData } from 'vs/workbench/api/common/extHostDocumentsAndEditors'; import { CellEditType, CellOutputKind, diff, IMainCellDto, IProcessedOutput, NotebookCellMetadata, NotebookCellsChangedEventDto, NotebookCellsChangeType, NotebookCellsSplice2, notebookDocumentMetadataDefaults } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import * as vscode from 'vscode'; @@ -191,13 +191,13 @@ export class ExtHostCell extends Disposable { edit: { editType: CellEditType.Metadata, index, metadata: this._metadata } }; - console.log('_updateMetadata', this._metadata); return this._mainThreadBulkEdits.$tryApplyWorkspaceEdit({ edits: [edit] }); } } export interface INotebookEventEmitter { emitModelChange(events: vscode.NotebookCellsChangeEvent): void; + emitDocumentMetadataChange(event: vscode.NotebookDocumentMetadataChangeEvent): void; emitCellOutputsChange(event: vscode.NotebookCellOutputsChangeEvent): void; emitCellLanguageChange(event: vscode.NotebookCellLanguageChangeEvent): void; emitCellMetadataChange(event: vscode.NotebookCellMetadataChangeEvent): void; @@ -274,8 +274,17 @@ export class ExtHostNotebookDocument extends Disposable { } private _tryUpdateMetadata() { - this._proxy.$updateNotebookMetadata(this._viewType, this.uri, this._metadata); + const edit: IWorkspaceCellEditDto = { + _type: WorkspaceEditType.Cell, + metadata: undefined, + notebookMetadata: this._metadata, + resource: this.uri, + notebookVersionId: this.notebookDocument.version, + }; + + return this._mainThreadBulkEdits.$tryApplyWorkspaceEdit({ edits: [edit] }); } + get notebookDocument(): vscode.NotebookDocument { if (!this._notebook) { const that = this; @@ -319,6 +328,25 @@ export class ExtHostNotebookDocument extends Disposable { this._backup = undefined; } + acceptDocumentPropertiesChanged(data: INotebookDocumentPropertiesChangeData) { + const newMetadata = { + ...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._emitter.emitDocumentMetadataChange({ document: this.notebookDocument }); + } + acceptModelChanged(event: NotebookCellsChangedEventDto, isDirty: boolean): void { this._versionId = event.versionId; this._isDirty = isDirty; diff --git a/src/vs/workbench/api/common/extHostNotebookEditor.ts b/src/vs/workbench/api/common/extHostNotebookEditor.ts index f12f9f4f78e01..4b98b86ae662a 100644 --- a/src/vs/workbench/api/common/extHostNotebookEditor.ts +++ b/src/vs/workbench/api/common/extHostNotebookEditor.ts @@ -223,7 +223,8 @@ export class ExtHostNotebookEditor extends Disposable implements vscode.Notebook const prev = compressedEdits[prevIndex]; if (prev.editType === CellEditType.Replace && editData.cellEdits[i].editType === CellEditType.Replace) { - if (prev.index === editData.cellEdits[i].index) { + const edit = editData.cellEdits[i]; + if (edit.editType !== CellEditType.DocumentMetadata && prev.index === edit.index) { prev.cells.push(...(editData.cellEdits[i] as ICellReplaceEdit).cells); prev.count += (editData.cellEdits[i] as ICellReplaceEdit).count; continue; diff --git a/src/vs/workbench/contrib/bulkEdit/browser/bulkCellEdits.ts b/src/vs/workbench/contrib/bulkEdit/browser/bulkCellEdits.ts index 41ab95fd25efb..6db8054a5f3a4 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/bulkCellEdits.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/bulkCellEdits.ts @@ -9,7 +9,7 @@ import { URI } from 'vs/base/common/uri'; import { ResourceEdit } from 'vs/editor/browser/services/bulkEditService'; import { WorkspaceEditMetadata } from 'vs/editor/common/modes'; import { IProgress } from 'vs/platform/progress/common/progress'; -import { ICellEditOperation, NotebookDocumentMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellEditType, ICellEditOperation, NotebookDocumentMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookEditorModelResolverService } from 'vs/workbench/contrib/notebook/common/notebookEditorModelResolverService'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; @@ -60,7 +60,10 @@ export class BulkCellEdits { // set metadata if (newMetadata) { - ref.object.notebook.updateNotebookMetadata(newMetadata); + edits.push({ + editType: CellEditType.DocumentMetadata, + metadata: newMetadata + }); } // apply edits this._notebookService.transformEditsOutputs(ref.object.notebook, edits); diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts index a265a6d36aaf5..132928efb9c49 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts @@ -210,13 +210,13 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel this._increaseVersionId(); } - dispose() { this._onWillDispose.fire(); dispose(this._cellListeners.values()); dispose(this._cells); super.dispose(); } + createCellTextModel( source: string, language: string, @@ -251,10 +251,21 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel const edits = rawEdits.map((edit, index) => { return { edit, - end: edit.editType === CellEditType.Replace ? edit.index + edit.count : edit.index, + end: + edit.editType === CellEditType.DocumentMetadata + ? undefined + : (edit.editType === CellEditType.Replace ? edit.index + edit.count : edit.index), originalIndex: index, }; }).sort((a, b) => { + if (a.end === undefined) { + return -1; + } + + if (b.end === undefined) { + return -1; + } + return b.end - a.end || b.originalIndex - a.originalIndex; }); @@ -278,12 +289,50 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel this._assertIndex(edit.index); this._changeCellLanguage(this._cells[edit.index].handle, edit.language); break; + case CellEditType.DocumentMetadata: + this._updateNotebookMetadata(edit.metadata); + break; } } return true; } + + handleUnknownEdit(label: string | undefined, undo: () => void, redo: () => void): void { + this._operationManager.pushEditOperation({ + type: UndoRedoElementType.Resource, + resource: this.uri, + label: label ?? nls.localize('defaultEditLabel', "Edit"), + undo: async () => { + undo(); + }, + redo: async () => { + redo(); + }, + }); + + this._eventEmitter.emit({ + kind: NotebookCellsChangeType.Unknown, + transient: false, + synchronous: true, + versionId: this._versionId, + }); + } + + handleUnknownChange() { + this._eventEmitter.emit({ + kind: NotebookCellsChangeType.Unknown, + transient: false, + synchronous: true, + versionId: this._versionId, + }); + } + + createSnapshot(preserveBOM?: boolean): ITextSnapshot { + return new NotebookTextModelSnapshot(this); + } + private _replaceCells(index: number, count: number, cellDtos: ICellDto2[], synchronous: boolean): void { if (count === 0 && cellDtos.length === 0) { @@ -346,40 +395,6 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel }); } - handleUnknownEdit(label: string | undefined, undo: () => void, redo: () => void): void { - this._operationManager.pushEditOperation({ - type: UndoRedoElementType.Resource, - resource: this.uri, - label: label ?? nls.localize('defaultEditLabel', "Edit"), - undo: async () => { - undo(); - }, - redo: async () => { - redo(); - }, - }); - - this._eventEmitter.emit({ - kind: NotebookCellsChangeType.Unknown, - transient: false, - synchronous: true, - versionId: this._versionId, - }); - } - - handleUnknownChange() { - this._eventEmitter.emit({ - kind: NotebookCellsChangeType.Unknown, - transient: false, - synchronous: true, - versionId: this._versionId, - }); - } - - createSnapshot(preserveBOM?: boolean): ITextSnapshot { - return new NotebookTextModelSnapshot(this); - } - private _increaseVersionId(): void { this._versionId = this._versionId + 1; } @@ -395,7 +410,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel } } - updateNotebookMetadata(metadata: NotebookDocumentMetadata) { + private _updateNotebookMetadata(metadata: NotebookDocumentMetadata) { this.metadata = metadata; this._eventEmitter.emit({ kind: NotebookCellsChangeType.ChangeDocumentMetadata, versionId: this.versionId, metadata: this.metadata, synchronous: true, transient: false }); } diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 444d228c1f285..d409aacb92ed5 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -418,7 +418,8 @@ export const enum CellEditType { Replace = 1, Output = 2, Metadata = 3, - CellLanguage = 4 + CellLanguage = 4, + DocumentMetadata = 5, } export interface ICellDto2 { @@ -455,7 +456,12 @@ export interface ICellLanguageEdit { language: string; } -export type ICellEditOperation = ICellReplaceEdit | ICellOutputEdit | ICellMetadataEdit | ICellLanguageEdit; +export interface IDocumentMetadataEdit { + editType: CellEditType.DocumentMetadata; + metadata: NotebookDocumentMetadata; +} + +export type ICellEditOperation = ICellReplaceEdit | ICellOutputEdit | ICellMetadataEdit | ICellLanguageEdit | IDocumentMetadataEdit; export interface INotebookEditData { documentVersionId: number;