From 1055329edd34e51b9f2bf4b3428efe69c9bf4a21 Mon Sep 17 00:00:00 2001 From: Jonah Iden Date: Tue, 24 Oct 2023 10:04:41 +0200 Subject: [PATCH] outputs now updating correctly when output items are updated. Stdout and StdError output items are concatinated Signed-off-by: Jonah Iden --- .../notebook-execution-state-service.ts | 1 + .../browser/view-model/notebook-cell-model.ts | 30 +++++++++++++---- .../view-model/notebook-cell-output-model.ts | 32 ++++++++++++++++++- .../src/browser/view-model/notebook-model.ts | 3 ++ .../notebook/src/common/notebook-common.ts | 4 ++- .../plugin-ext/src/common/plugin-api-rpc.ts | 1 + .../renderers/cell-output-webview.tsx | 3 ++ .../renderers/output-webview-internal.ts | 1 + .../src/plugin/notebook/notebook-kernels.ts | 4 ++- .../plugin-ext/src/plugin/type-converters.ts | 1 + 10 files changed, 70 insertions(+), 10 deletions(-) diff --git a/packages/notebook/src/browser/service/notebook-execution-state-service.ts b/packages/notebook/src/browser/service/notebook-execution-state-service.ts index 4eee51691f278..51ed8dc2059c5 100644 --- a/packages/notebook/src/browser/service/notebook-execution-state-service.ts +++ b/packages/notebook/src/browser/service/notebook-execution-state-service.ts @@ -293,6 +293,7 @@ export function updateToEdit(update: CellExecuteUpdate, cellHandle: number): Cel return { editType: CellEditType.OutputItems, items: update.items, + outputId: update.outputId, append: update.append, }; } else if (update.editType === CellExecutionUpdateType.ExecutionState) { diff --git a/packages/notebook/src/browser/view-model/notebook-cell-model.ts b/packages/notebook/src/browser/view-model/notebook-cell-model.ts index 6c5fe56ff6588..bd804be68afbd 100644 --- a/packages/notebook/src/browser/view-model/notebook-cell-model.ts +++ b/packages/notebook/src/browser/view-model/notebook-cell-model.ts @@ -24,7 +24,7 @@ import { MonacoEditorModel } from '@theia/monaco/lib/browser/monaco-editor-model import { MonacoTextModelService } from '@theia/monaco/lib/browser/monaco-text-model-service'; import { CellInternalMetadataChangedEvent, CellKind, NotebookCellCollapseState, NotebookCellInternalMetadata, - NotebookCellMetadata, NotebookCellOutputsSplice, CellOutput, CellData, NotebookCell + NotebookCellMetadata, NotebookCellOutputsSplice, CellOutput, CellData, NotebookCell, CellOutputItem } from '../../common'; import { NotebookCellOutputModel } from './notebook-cell-output-model'; @@ -70,8 +70,8 @@ export class NotebookCellModel implements NotebookCell, Disposable { protected readonly onDidChangeOutputsEmitter = new Emitter(); readonly onDidChangeOutputs: Event = this.onDidChangeOutputsEmitter.event; - protected readonly onDidChangeOutputItemsEmitter = new Emitter(); - readonly onDidChangeOutputItems: Event = this.onDidChangeOutputItemsEmitter.event; + protected readonly onDidChangeOutputItemsEmitter = new Emitter(); + readonly onDidChangeOutputItems: Event = this.onDidChangeOutputItemsEmitter.event; protected readonly onDidChangeContentEmitter = new Emitter<'content' | 'language' | 'mime'>(); readonly onDidChangeContent: Event<'content' | 'language' | 'mime'> = this.onDidChangeContentEmitter.event; @@ -216,7 +216,7 @@ export class NotebookCellModel implements NotebookCell, Disposable { const currentOutput = this.outputs[splice.start + i]; const newOutput = splice.newOutputs[i]; - this.replaceOutputItems(currentOutput.outputId, newOutput); + this.replaceOutputData(currentOutput.outputId, newOutput); } this.outputs.splice(splice.start + commonLen, splice.deleteCount - commonLen, ...splice.newOutputs.slice(commonLen).map(op => new NotebookCellOutputModel(op))); @@ -227,15 +227,31 @@ export class NotebookCellModel implements NotebookCell, Disposable { } } - replaceOutputItems(outputId: string, newOutputItem: CellOutput): boolean { + replaceOutputData(outputId: string, newOutputData: CellOutput): boolean { const output = this.outputs.find(out => out.outputId === outputId); if (!output) { return false; } - output.replaceData(newOutputItem); - this.onDidChangeOutputItemsEmitter.fire(); + output.replaceData(newOutputData); + this.onDidChangeOutputItemsEmitter.fire(output); + return true; + } + + changeOutputItems(outputId: string, append: boolean, items: CellOutputItem[]): boolean { + const output = this.outputs.find(out => out.outputId === outputId); + + if (!output) { + return false; + } + + if (append) { + output.appendData(items); + } else { + output.replaceData({ outputId: outputId, outputs: items, metadata: output.metadata }); + } + this.onDidChangeOutputItemsEmitter.fire(output); return true; } diff --git a/packages/notebook/src/browser/view-model/notebook-cell-output-model.ts b/packages/notebook/src/browser/view-model/notebook-cell-output-model.ts index 0e9205c4fcaf2..0e4dbe936f366 100644 --- a/packages/notebook/src/browser/view-model/notebook-cell-output-model.ts +++ b/packages/notebook/src/browser/view-model/notebook-cell-output-model.ts @@ -15,7 +15,8 @@ // ***************************************************************************** import { Disposable, Emitter } from '@theia/core'; -import { CellOutput, CellOutputItem } from '../../common'; +import { BinaryBuffer } from '@theia/core/lib/common/buffer'; +import { CellOutput, CellOutputItem, isTextStreamMime } from '../../common'; export class NotebookCellOutputModel implements Disposable { @@ -41,11 +42,13 @@ export class NotebookCellOutputModel implements Disposable { replaceData(rawData: CellOutput): void { this.rawOutput = rawData; + this.optimizeOutputItems(); this.didChangeDataEmitter.fire(); } appendData(items: CellOutputItem[]): void { this.rawOutput.outputs.push(...items); + this.optimizeOutputItems(); this.didChangeDataEmitter.fire(); } @@ -66,4 +69,31 @@ export class NotebookCellOutputModel implements Disposable { }; } + private optimizeOutputItems(): void { + if (this.outputs.length > 1 && this.outputs.every(item => isTextStreamMime(item.mime))) { + // Look for the mimes in the items, and keep track of their order. + // Merge the streams into one output item, per mime type. + const mimeOutputs = new Map(); + const mimeTypes: string[] = []; + this.outputs.forEach(item => { + let items: BinaryBuffer[]; + if (mimeOutputs.has(item.mime)) { + items = mimeOutputs.get(item.mime)!; + } else { + items = []; + mimeOutputs.set(item.mime, items); + mimeTypes.push(item.mime); + } + items.push(item.data); + }); + this.outputs.length = 0; + mimeTypes.forEach(mime => { + this.outputs.push({ + mime, + data: BinaryBuffer.concat(mimeOutputs.get(mime)!) + }); + }); + } + } + } diff --git a/packages/notebook/src/browser/view-model/notebook-model.ts b/packages/notebook/src/browser/view-model/notebook-model.ts index cf1f2359d6b1f..e8f1b897e3add 100644 --- a/packages/notebook/src/browser/view-model/notebook-model.ts +++ b/packages/notebook/src/browser/view-model/notebook-model.ts @@ -223,6 +223,8 @@ export class NotebookModel implements Saveable, Disposable { cellIndex = edit.index; } else if ('handle' in edit) { cellIndex = this.getCellIndexByHandle(edit.handle); + } else if ('outputId' in edit) { + cellIndex = this.cells.findIndex(cell => cell.outputs.some(output => output.outputId === edit.outputId)); } return { @@ -254,6 +256,7 @@ export class NotebookModel implements Saveable, Disposable { break; } case CellEditType.OutputItems: + cell.changeOutputItems(edit.outputId, !!edit.append, edit.items); break; case CellEditType.Metadata: this.updateNotebookMetadata(edit.metadata, computeUndoRedo); diff --git a/packages/notebook/src/common/notebook-common.ts b/packages/notebook/src/common/notebook-common.ts index 6d60da581f57f..7fb5a4dbc26cc 100644 --- a/packages/notebook/src/common/notebook-common.ts +++ b/packages/notebook/src/common/notebook-common.ts @@ -98,7 +98,7 @@ export interface NotebookCell { internalMetadata: NotebookCellInternalMetadata; text: string; onDidChangeOutputs?: Event; - onDidChangeOutputItems?: Event; + onDidChangeOutputItems?: Event; onDidChangeLanguage: Event; onDidChangeMetadata: Event; onDidChangeInternalMetadata: Event; @@ -309,6 +309,7 @@ export interface CellExecuteOutputEdit { export interface CellExecuteOutputItemEdit { editType: CellExecutionUpdateType.OutputItems; append?: boolean; + outputId: string, items: CellOutputItem[]; } @@ -337,6 +338,7 @@ export interface CellOutputEditByHandle { export interface CellOutputItemEdit { editType: CellEditType.OutputItems; items: CellOutputItem[]; + outputId: string; append?: boolean; } diff --git a/packages/plugin-ext/src/common/plugin-api-rpc.ts b/packages/plugin-ext/src/common/plugin-api-rpc.ts index bba09a687b62c..8051cd8fe3aa7 100644 --- a/packages/plugin-ext/src/common/plugin-api-rpc.ts +++ b/packages/plugin-ext/src/common/plugin-api-rpc.ts @@ -2423,6 +2423,7 @@ export interface CellExecuteOutputEditDto { export interface CellExecuteOutputItemEditDto { editType: CellExecutionUpdateType.OutputItems; append?: boolean; + outputId: string; items: NotebookOutputItemDto[]; } diff --git a/packages/plugin-ext/src/main/browser/notebooks/renderers/cell-output-webview.tsx b/packages/plugin-ext/src/main/browser/notebooks/renderers/cell-output-webview.tsx index 2568749ea440f..987367cb33abc 100644 --- a/packages/plugin-ext/src/main/browser/notebooks/renderers/cell-output-webview.tsx +++ b/packages/plugin-ext/src/main/browser/notebooks/renderers/cell-output-webview.tsx @@ -75,6 +75,9 @@ export class CellOutputWebviewImpl implements CellOutputWebview, Disposable { @postConstruct() protected async init(): Promise { this.cell.onDidChangeOutputs(outputChange => this.updateOutput(outputChange)); + this.cell.onDidChangeOutputItems(output => { + this.updateOutput({start: this.cell.outputs.findIndex(o => o.getData().outputId === o.outputId), deleteCount: 1, newOutputs: [output]}); + }); this.webviewWidget = await this.widgetManager.getOrCreateWidget(WebviewWidget.FACTORY_ID, { id: this.id }); this.webviewWidget.setContentOptions({ allowScripts: true }); diff --git a/packages/plugin-ext/src/main/browser/notebooks/renderers/output-webview-internal.ts b/packages/plugin-ext/src/main/browser/notebooks/renderers/output-webview-internal.ts index 81c87f1b6d622..b8ac9bdd68939 100644 --- a/packages/plugin-ext/src/main/browser/notebooks/renderers/output-webview-internal.ts +++ b/packages/plugin-ext/src/main/browser/notebooks/renderers/output-webview-internal.ts @@ -381,6 +381,7 @@ export async function outputWebviewPreload(ctx: PreloadContext): Promise { function clearOutput(outputId: string): void { outputs.get(outputId)?.clear(); outputs.delete(outputId); + document.getElementById(outputId)?.remove(); } function outputsChanged(changedEvent: webviewCommunication.OutputChangedMessage): void { diff --git a/packages/plugin-ext/src/plugin/notebook/notebook-kernels.ts b/packages/plugin-ext/src/plugin/notebook/notebook-kernels.ts index 4f0e2d78ab8ef..f03038eabebde 100644 --- a/packages/plugin-ext/src/plugin/notebook/notebook-kernels.ts +++ b/packages/plugin-ext/src/plugin/notebook/notebook-kernels.ts @@ -483,11 +483,13 @@ class NotebookCellExecutionTask implements Disposable { }); } - private async updateOutputItems(items: theia.NotebookCellOutputItem | theia.NotebookCellOutputItem[], output: theia.NotebookCellOutput, append: boolean): Promise { + private async updateOutputItems(items: theia.NotebookCellOutputItem | theia.NotebookCellOutputItem[], + output: theia.NotebookCellOutput, append: boolean): Promise { items = NotebookCellOutputConverter.ensureUniqueMimeTypes(Array.isArray(items) ? items : [items], true); return this.updateSoon({ editType: CellExecutionUpdateType.OutputItems, items: items.map(NotebookCellOutputItem.from), + outputId: output instanceof NotebookCellOutput ? output.outputId : '', append }); } diff --git a/packages/plugin-ext/src/plugin/type-converters.ts b/packages/plugin-ext/src/plugin/type-converters.ts index a109a68f489f2..9b8626c48d912 100644 --- a/packages/plugin-ext/src/plugin/type-converters.ts +++ b/packages/plugin-ext/src/plugin/type-converters.ts @@ -1722,6 +1722,7 @@ export namespace NotebookDto { return { editType: data.editType, append: data.append, + outputId: data.outputId, items: data.items.map(fromNotebookOutputItemDto) }; } else {