Skip to content

Commit

Permalink
make sure replaceOutput updates the extension host side, update tests #…
Browse files Browse the repository at this point in the history 
…105283
  • Loading branch information
@jrieken
jrieken committed Aug 28, 2020
1 parent a3f414c commit 807eb37
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 31 deletions.
9 changes: 2 additions & 7 deletions extensions/vscode-notebook-tests/src/notebook.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -418,9 +418,6 @@ suite('Notebook API tests', () => {
});

test('edit API (replaceOutput)', async function () {
// no output events yet...
this.skip();

assertInitalState();
const resource = await createRandomFile('', undefined, 'first', '.vsctestnb');
await vscode.commands.executeCommand('vscode.openWith', resource, 'notebookCoreTest');
Expand All @@ -430,7 +427,7 @@ suite('Notebook API tests', () => {
});

const document = vscode.notebook.activeNotebookEditor?.document!;
assert.strictEqual(document.isDirty, false);
assert.strictEqual(document.isDirty, true);
assert.strictEqual(document.cells.length, 1);
assert.strictEqual(document.cells[0].outputs.length, 1);
assert.strictEqual(document.cells[0].outputs[0].outputKind, vscode.CellOutputKind.Rich);
Expand All @@ -439,9 +436,6 @@ suite('Notebook API tests', () => {
});

test('edit API (replaceOutput, event)', async function () {
// no output events yet...
this.skip();

assertInitalState();
const resource = await createRandomFile('', undefined, 'first', '.vsctestnb');
await vscode.commands.executeCommand('vscode.openWith', resource, 'notebookCoreTest');
Expand All @@ -453,6 +447,7 @@ suite('Notebook API tests', () => {

const value = await outputChangeEvent;
assert.strictEqual(value.document === vscode.notebook.activeNotebookEditor?.document, true);
assert.strictEqual(value.document.isDirty, true);
assert.strictEqual(value.cells.length, 1);
assert.strictEqual(value.cells[0].outputs.length, 1);
assert.strictEqual(value.cells[0].outputs[0].outputKind, vscode.CellOutputKind.Rich);
Expand Down
12 changes: 12 additions & 0 deletions src/vs/workbench/api/common/extHostNotebook.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ export class ExtHostCell extends Disposable {
this._onDidDispose.fire();
}

setOutputs(newOutputs: vscode.CellOutput[]): void {
this._outputs = newOutputs;
}

private _updateOutputs(newOutputs: vscode.CellOutput[]) {
const rawDiffs = diff<vscode.CellOutput>(this._outputs || [], newOutputs || [], (a) => {
return this._outputMapping.has(a);
Expand Down Expand Up @@ -321,6 +325,8 @@ export class ExtHostNotebookDocument extends Disposable {
this._spliceNotebookCells(event.changes, false);
} else if (event.kind === NotebookCellsChangeType.Move) {
this._moveCell(event.index, event.newIdx);
} else if (event.kind === NotebookCellsChangeType.Output) {
this._setCellOutputs(event.index, event.outputs);
} else if (event.kind === NotebookCellsChangeType.CellClearOutput) {
this._clearCellOutputs(event.index);
} else if (event.kind === NotebookCellsChangeType.CellsClearOutput) {
Expand Down Expand Up @@ -412,6 +418,12 @@ export class ExtHostNotebookDocument extends Disposable {
});
}

private _setCellOutputs(index: number, outputs: IProcessedOutput[]): void {
const cell = this._cells[index];
cell.setOutputs(outputs);
this._emitter.emitCellOutputsChange({ document: this.notebookDocument, cells: [cell.cell] });
}

private _clearCellOutputs(index: number): void {
const cell = this._cells[index].cell;
cell.outputs = [];
Expand Down
21 changes: 16 additions & 5 deletions src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -505,13 +505,24 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
// TODO@rebornix should this trigger content change event?
spliceNotebookCellOutputs(cellHandle: number, splices: NotebookCellOutputsSplice[]): void {
const cell = this._mapping.get(cellHandle);
cell?.spliceNotebookCellOutputs(splices);
if (cell) {

if (!this.transientOptions.transientOutputs) {
this._increaseVersionId();
this.setDirty(true);
this._onDidChangeContent.fire();
cell.spliceNotebookCellOutputs(splices);

if (!this.transientOptions.transientOutputs) {
this._increaseVersionId();
this.setDirty(true);
this._onDidChangeContent.fire();
}

this._onDidModelChangeProxy.fire({
kind: NotebookCellsChangeType.Output,
versionId: this.versionId,
index: this.cells.indexOf(cell),
outputs: cell.outputs ?? []
});
}

}

clearCellOutput(handle: number) {
Expand Down
18 changes: 13 additions & 5 deletions src/vs/workbench/contrib/notebook/common/notebookCommon.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,15 +166,15 @@ export interface IErrorOutput {
/**
* Exception Name
*/
ename?: string;
ename: string;
/**
* Exception Value
*/
evalue?: string;
evalue: string;
/**
* Exception call stacks
*/
traceback?: string[];
traceback: string[];
}

export interface NotebookCellOutputMetadata {
Expand Down Expand Up @@ -366,7 +366,8 @@ export enum NotebookCellsChangeType {
CellsClearOutput = 4,
ChangeLanguage = 5,
Initialize = 6,
ChangeMetadata = 7
ChangeMetadata = 7,
Output = 8,
}

export interface NotebookCellsInitializeEvent {
Expand All @@ -388,6 +389,13 @@ export interface NotebookCellsModelMoveEvent {
readonly versionId: number;
}

export interface NotebookOutputChangedEvent {
readonly kind: NotebookCellsChangeType.Output;
readonly index: number;
readonly versionId: number;
readonly outputs: IProcessedOutput[];
}

export interface NotebookCellClearOutputEvent {
readonly kind: NotebookCellsChangeType.CellClearOutput;
readonly index: number;
Expand All @@ -413,7 +421,7 @@ export interface NotebookCellsChangeMetadataEvent {
readonly metadata: NotebookCellMetadata | undefined;
}

export type NotebookCellsChangedEvent = NotebookCellsInitializeEvent | NotebookCellsModelChangedEvent | NotebookCellsModelMoveEvent | NotebookCellClearOutputEvent | NotebookCellsClearOutputEvent | NotebookCellsChangeLanguageEvent | NotebookCellsChangeMetadataEvent;
export type NotebookCellsChangedEvent = NotebookCellsInitializeEvent | NotebookCellsModelChangedEvent | NotebookCellsModelMoveEvent | NotebookOutputChangedEvent | NotebookCellClearOutputEvent | NotebookCellsClearOutputEvent | NotebookCellsChangeLanguageEvent | NotebookCellsChangeMetadataEvent;

export const enum CellEditType {
Replace = 1,
Expand Down
4 changes: 2 additions & 2 deletions src/vs/workbench/test/browser/api/extHostNotebook.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ suite('NotebookCell#Document', function () {
cellKind: CellKind.Code,
outputs: [],
}]]]
});
}, false);

await p;

Expand Down Expand Up @@ -234,7 +234,7 @@ suite('NotebookCell#Document', function () {
kind: NotebookCellsChangeType.ModelChange,
versionId: 2,
changes: [[0, 1, []]]
});
}, false);

assert.equal(notebook.notebookDocument.cells.length, 1);
assert.equal(cell1.document.isClosed, true); // ref still alive!
Expand Down
24 changes: 12 additions & 12 deletions src/vs/workbench/test/browser/api/extHostNotebookConcatDocument.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ suite('NotebookConcatDocument', function () {
cellKind: CellKind.Code,
outputs: [],
}]]]
});
}, false);


assert.equal(notebook.notebookDocument.cells.length, 1 + 2); // markdown and code
Expand Down Expand Up @@ -177,7 +177,7 @@ suite('NotebookConcatDocument', function () {
cellKind: CellKind.Code,
outputs: [],
}]]]
});
}, false);


assert.equal(notebook.notebookDocument.cells.length, 1 + 2); // markdown and code
Expand Down Expand Up @@ -210,7 +210,7 @@ suite('NotebookConcatDocument', function () {
cellKind: CellKind.Code,
outputs: [],
}]]]
});
}, false);
assert.equal(notebook.notebookDocument.cells.length, 1 + 1);
assert.equal(doc.version, 1);
assertLines(doc, 'Hello', 'World', 'Hello World!');
Expand All @@ -233,7 +233,7 @@ suite('NotebookConcatDocument', function () {
cellKind: CellKind.Code,
outputs: [],
}]]]
});
}, false);

assert.equal(notebook.notebookDocument.cells.length, 1 + 2);
assert.equal(doc.version, 2);
Expand All @@ -249,7 +249,7 @@ suite('NotebookConcatDocument', function () {
kind: NotebookCellsChangeType.ModelChange,
versionId: notebook.notebookDocument.version + 1,
changes: [[1, 1, []]]
});
}, false);
assert.equal(notebook.notebookDocument.cells.length, 1 + 1);
assert.equal(doc.version, 3);
assertLines(doc, 'Hello', 'World', 'Hello World!');
Expand Down Expand Up @@ -283,7 +283,7 @@ suite('NotebookConcatDocument', function () {
cellKind: CellKind.Code,
outputs: [],
}]]]
});
}, false);
assert.equal(notebook.notebookDocument.cells.length, 1 + 2);
assert.equal(doc.version, 1);

Expand Down Expand Up @@ -337,7 +337,7 @@ suite('NotebookConcatDocument', function () {
cellKind: CellKind.Code,
outputs: [],
}]]]
});
}, false);

const mixedDoc = new ExtHostNotebookConcatDocument(extHostNotebooks, extHostDocuments, notebook.notebookDocument, undefined);
const fooLangDoc = new ExtHostNotebookConcatDocument(extHostNotebooks, extHostDocuments, notebook.notebookDocument, 'fooLang');
Expand All @@ -359,7 +359,7 @@ suite('NotebookConcatDocument', function () {
cellKind: CellKind.Code,
outputs: [],
}]]]
});
}, false);

assertLines(mixedDoc, 'fooLang-document', 'barLang-document', 'barLang-document2');
assertLines(fooLangDoc, 'fooLang-document');
Expand Down Expand Up @@ -401,7 +401,7 @@ suite('NotebookConcatDocument', function () {
cellKind: CellKind.Code,
outputs: [],
}]]]
});
}, false);

assert.equal(notebook.notebookDocument.cells.length, 1 + 2); // markdown and code

Expand Down Expand Up @@ -454,7 +454,7 @@ suite('NotebookConcatDocument', function () {
cellKind: CellKind.Code,
outputs: [],
}]]]
});
}, false);

assert.equal(notebook.notebookDocument.cells.length, 1 + 2); // markdown and code

Expand Down Expand Up @@ -491,7 +491,7 @@ suite('NotebookConcatDocument', function () {
cellKind: CellKind.Code,
outputs: [],
}]]]
});
}, false);

assert.equal(notebook.notebookDocument.cells.length, 1 + 2); // markdown and code

Expand Down Expand Up @@ -525,7 +525,7 @@ suite('NotebookConcatDocument', function () {
cellKind: CellKind.Code,
outputs: [],
}]]]
});
}, false);

assert.equal(notebook.notebookDocument.cells.length, 1 + 2); // markdown and code

Expand Down

0 comments on commit 807eb37

Please sign in to comment.