Skip to content

Commit

Permalink
re microsoft#105735. no more udpateMetadata api.
Browse files Browse the repository at this point in the history
  • Loading branch information
rebornix committed Sep 9, 2020
1 parent 73a3c9d commit 03960a5
Show file tree
Hide file tree
Showing 11 changed files with 128 additions and 60 deletions.
6 changes: 6 additions & 0 deletions extensions/vscode-notebook-tests/src/notebook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -718,11 +718,17 @@ suite('notebook workflow', () => {

const cell = editor.document.cells[0];
assert.equal(cell.outputs.length, 0);
let metadataChangeEvent = getEventOncePromise<vscode.NotebookDocumentMetadataChangeEvent>(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.NotebookDocumentMetadataChangeEvent>(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

Expand Down
5 changes: 5 additions & 0 deletions src/vs/vscode.proposed.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1470,6 +1470,10 @@ declare module 'vscode' {
outputId: string;
}

export interface NotebookDocumentMetadataChangeEvent {
readonly document: NotebookDocument;
}

export interface NotebookCellsChangeData {
readonly start: number;
readonly deletedCount: number;
Expand Down Expand Up @@ -1747,6 +1751,7 @@ declare module 'vscode' {
export const onDidChangeActiveNotebookEditor: Event<NotebookEditor | undefined>;
export const onDidChangeNotebookEditorSelection: Event<NotebookEditorSelectionChangeEvent>;
export const onDidChangeNotebookEditorVisibleRanges: Event<NotebookEditorVisibleRangesChangeEvent>;
export const onDidChangeNotebookDocumentMetadata: Event<NotebookDocumentMetadataChangeEvent>;
export const onDidChangeNotebookCells: Event<NotebookCellsChangeEvent>;
export const onDidChangeCellOutputs: Event<NotebookCellOutputsChangeEvent>;
export const onDidChangeCellLanguage: Event<NotebookCellLanguageChangeEvent>;
Expand Down
19 changes: 9 additions & 10 deletions src/vs/workbench/api/browser/mainThreadNotebook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
};
Expand Down Expand Up @@ -560,12 +565,6 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
textModel?.updateLanguages(languages);
}

async $updateNotebookMetadata(viewType: string, resource: UriComponents, metadata: NotebookDocumentMetadata): Promise<void> {
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<void> {
this.logService.debug('MainThreadNotebooks#spliceNotebookCellOutputs', resource.path, cellHandle);
const textModel = this._notebookService.getNotebookTextModel(URI.from(resource));
Expand Down
4 changes: 4 additions & 0 deletions src/vs/workbench/api/common/extHost.api.impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion src/vs/workbench/api/common/extHost.protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<boolean>;
$updateNotebookLanguages(viewType: string, resource: UriComponents, languages: string[]): Promise<void>;
$updateNotebookMetadata(viewType: string, resource: UriComponents, metadata: NotebookDocumentMetadata): Promise<void>;
$spliceNotebookCellOutputs(viewType: string, resource: UriComponents, cellHandle: number, splices: NotebookCellOutputsSplice[]): Promise<void>;
$postMessage(editorId: string, forRendererId: string | undefined, value: any): Promise<boolean>;
$setStatusBarEntry(id: number, statusBarEntry: INotebookCellStatusBarEntryDto): Promise<void>;
Expand Down
10 changes: 6 additions & 4 deletions src/vs/workbench/api/common/extHostNotebook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,8 @@ export class ExtHostNotebookController implements ExtHostNotebookShape, ExtHostN
readonly onDidChangeNotebookEditorSelection = this._onDidChangeNotebookEditorSelection.event;
private readonly _onDidChangeNotebookEditorVisibleRanges = new Emitter<vscode.NotebookEditorVisibleRangesChangeEvent>();
readonly onDidChangeNotebookEditorVisibleRanges = this._onDidChangeNotebookEditorVisibleRanges.event;
private readonly _onDidChangeNotebookDocumentMetadata = new Emitter<vscode.NotebookDocumentMetadataChangeEvent>();
readonly onDidChangeNotebookDocumentMetadata = this._onDidChangeNotebookDocumentMetadata.event;
private readonly _onDidChangeNotebookCells = new Emitter<vscode.NotebookCellsChangeEvent>();
readonly onDidChangeNotebookCells = this._onDidChangeNotebookCells.event;
private readonly _onDidChangeCellOutputs = new Emitter<vscode.NotebookCellOutputsChangeEvent>();
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);

Expand Down
34 changes: 31 additions & 3 deletions src/vs/workbench/api/common/extHostNotebookDocument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion src/vs/workbench/api/common/extHostNotebookEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 5 additions & 2 deletions src/vs/workbench/contrib/bulkEdit/browser/bulkCellEdits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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);
Expand Down
89 changes: 52 additions & 37 deletions src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
});

Expand All @@ -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) {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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 });
}
Expand Down
10 changes: 8 additions & 2 deletions src/vs/workbench/contrib/notebook/common/notebookCommon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,8 @@ export const enum CellEditType {
Replace = 1,
Output = 2,
Metadata = 3,
CellLanguage = 4
CellLanguage = 4,
DocumentMetadata = 5,
}

export interface ICellDto2 {
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 03960a5

Please sign in to comment.