outputs now updating correctly when output items are updated.

Stdout and StdError output items are concatinated Signed-off-by: Jonah Iden <[email protected]>
eclipse-theia · Oct 24, 2023 · 1055329 · 1055329
1 parent 69dbf42
commit 1055329
Show file tree

Hide file tree

Showing 10 changed files with 70 additions and 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
@@ -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
@@ -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<NotebookCellOutputsSplice>();
     readonly onDidChangeOutputs: Event<NotebookCellOutputsSplice> = this.onDidChangeOutputsEmitter.event;
 
-    protected readonly onDidChangeOutputItemsEmitter = new Emitter<void>();
-    readonly onDidChangeOutputItems: Event<void> = this.onDidChangeOutputItemsEmitter.event;
+    protected readonly onDidChangeOutputItemsEmitter = new Emitter<CellOutput>();
+    readonly onDidChangeOutputItems: Event<CellOutput> = 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
@@ -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<string, BinaryBuffer[]>();
+            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
@@ -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
@@ -98,7 +98,7 @@ export interface NotebookCell {
     internalMetadata: NotebookCellInternalMetadata;
     text: string;
     onDidChangeOutputs?: Event<NotebookCellOutputsSplice>;
-    onDidChangeOutputItems?: Event<void>;
+    onDidChangeOutputItems?: Event<CellOutput>;
     onDidChangeLanguage: Event<string>;
     onDidChangeMetadata: Event<void>;
     onDidChangeInternalMetadata: Event<CellInternalMetadataChangedEvent>;
@@ -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
@@ -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
@@ -75,6 +75,9 @@ export class CellOutputWebviewImpl implements CellOutputWebview, Disposable {
     @postConstruct()
     protected async init(): Promise<void> {
         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
@@ -381,6 +381,7 @@ export async function outputWebviewPreload(ctx: PreloadContext): Promise<void> {
     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
@@ -483,11 +483,13 @@ class NotebookCellExecutionTask implements Disposable {
             });
     }
 
-    private async updateOutputItems(items: theia.NotebookCellOutputItem | theia.NotebookCellOutputItem[], output: theia.NotebookCellOutput, append: boolean): Promise<void> {
+    private async updateOutputItems(items: theia.NotebookCellOutputItem | theia.NotebookCellOutputItem[],
+        output: theia.NotebookCellOutput, append: boolean): Promise<void> {
         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
@@ -1722,6 +1722,7 @@ export namespace NotebookDto {
             return {
                 editType: data.editType,
                 append: data.append,
+                outputId: data.outputId,
                 items: data.items.map(fromNotebookOutputItemDto)
             };
         } else {