From 1381b9e0c30cfd3cdeb68fd4d6fe72bd0e3263d5 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 15 Sep 2022 08:53:33 +1000 Subject: [PATCH 01/10] Revert "Compress notebook output streams before rendering (#160667)" This reverts commit 4230c22a08a0438af4809d33e4cfd561f714b127. --- extensions/ipynb/src/serializers.ts | 27 ++++---- extensions/ipynb/src/streamCompressor.ts | 63 ------------------- src/vs/workbench/api/common/extHostTypes.ts | 5 +- .../view/renderers/backLayerWebView.ts | 11 ++-- .../view/renderers/stdOutErrorPreProcessor.ts | 56 ----------------- .../contrib/notebook/common/notebookCommon.ts | 8 --- 6 files changed, 21 insertions(+), 149 deletions(-) delete mode 100644 extensions/ipynb/src/streamCompressor.ts delete mode 100644 src/vs/workbench/contrib/notebook/browser/view/renderers/stdOutErrorPreProcessor.ts diff --git a/extensions/ipynb/src/serializers.ts b/extensions/ipynb/src/serializers.ts index f075737b4d865..4f33b07a1501e 100644 --- a/extensions/ipynb/src/serializers.ts +++ b/extensions/ipynb/src/serializers.ts @@ -7,7 +7,6 @@ import type * as nbformat from '@jupyterlab/nbformat'; import { NotebookCell, NotebookCellData, NotebookCellKind, NotebookCellOutput } from 'vscode'; import { CellOutputMetadata } from './common'; import { textMimeTypes } from './deserializers'; -import { compressOutputItemStreams } from './streamCompressor'; const textDecoder = new TextDecoder(); @@ -277,17 +276,21 @@ type JupyterOutput = function convertStreamOutput(output: NotebookCellOutput): JupyterOutput { const outputs: string[] = []; - const compressedStream = output.items.length ? new TextDecoder().decode(compressOutputItemStreams(output.items[0].mime, output.items)) : ''; - // Ensure each line is a separate entry in an array (ending with \n). - const lines = compressedStream.split('\n'); - // If the last item in `outputs` is not empty and the first item in `lines` is not empty, then concate them. - // As they are part of the same line. - if (outputs.length && lines.length && lines[0].length > 0) { - outputs[outputs.length - 1] = `${outputs[outputs.length - 1]}${lines.shift()!}`; - } - for (const line of lines) { - outputs.push(line); - } + output.items + .filter((opit) => opit.mime === CellOutputMimeTypes.stderr || opit.mime === CellOutputMimeTypes.stdout) + .map((opit) => textDecoder.decode(opit.data)) + .forEach(value => { + // Ensure each line is a seprate entry in an array (ending with \n). + const lines = value.split('\n'); + // If the last item in `outputs` is not empty and the first item in `lines` is not empty, then concate them. + // As they are part of the same line. + if (outputs.length && lines.length && lines[0].length > 0) { + outputs[outputs.length - 1] = `${outputs[outputs.length - 1]}${lines.shift()!}`; + } + for (const line of lines) { + outputs.push(line); + } + }); for (let index = 0; index < (outputs.length - 1); index++) { outputs[index] = `${outputs[index]}\n`; diff --git a/extensions/ipynb/src/streamCompressor.ts b/extensions/ipynb/src/streamCompressor.ts deleted file mode 100644 index cea3184e16ce6..0000000000000 --- a/extensions/ipynb/src/streamCompressor.ts +++ /dev/null @@ -1,63 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import type { NotebookCellOutputItem } from 'vscode'; - - -/** - * Given a stream of individual stdout outputs, this function will return the compressed lines, escaping some of the common terminal escape codes. - * E.g. some terminal escape codes would result in the previous line getting cleared, such if we had 3 lines and - * last line contained such a code, then the result string would be just the first two lines. - */ -export function compressOutputItemStreams(mimeType: string, outputs: NotebookCellOutputItem[]) { - // return outputs.find(op => op.mime === mimeType)!.data.buffer; - - const buffers: Uint8Array[] = []; - let startAppending = false; - // Pick the first set of outputs with the same mime type. - for (const output of outputs) { - if (output.mime === mimeType) { - if ((buffers.length === 0 || startAppending)) { - buffers.push(output.data); - startAppending = true; - } - } else if (startAppending) { - startAppending = false; - } - } - compressStreamBuffer(buffers); - const totalBytes = buffers.reduce((p, c) => p + c.byteLength, 0); - const combinedBuffer = new Uint8Array(totalBytes); - let offset = 0; - for (const buffer of buffers) { - combinedBuffer.set(buffer, offset); - offset = offset + buffer.byteLength; - } - return combinedBuffer; -} -const MOVE_CURSOR_1_LINE_COMMAND = `${String.fromCharCode(27)}[A`; -const MOVE_CURSOR_1_LINE_COMMAND_BYTES = MOVE_CURSOR_1_LINE_COMMAND.split('').map(c => c.charCodeAt(0)); -const LINE_FEED = 10; -function compressStreamBuffer(streams: Uint8Array[]) { - streams.forEach((stream, index) => { - if (index === 0 || stream.length < MOVE_CURSOR_1_LINE_COMMAND.length) { - return; - } - - const previousStream = streams[index - 1]; - - // Remove the previous line if required. - const command = stream.subarray(0, MOVE_CURSOR_1_LINE_COMMAND.length); - if (command[0] === MOVE_CURSOR_1_LINE_COMMAND_BYTES[0] && command[1] === MOVE_CURSOR_1_LINE_COMMAND_BYTES[1] && command[2] === MOVE_CURSOR_1_LINE_COMMAND_BYTES[2]) { - const lastIndexOfLineFeed = previousStream.lastIndexOf(LINE_FEED); - if (lastIndexOfLineFeed === -1) { - return; - } - streams[index - 1] = previousStream.subarray(0, lastIndexOfLineFeed); - streams[index] = stream.subarray(MOVE_CURSOR_1_LINE_COMMAND.length); - } - }); - return streams; -} diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index f7b89d987a907..a717fa90515a1 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -17,7 +17,7 @@ import { IExtensionDescription } from 'vs/platform/extensions/common/extensions' import { FileSystemProviderErrorCode, markAsFileSystemProviderError } from 'vs/platform/files/common/files'; import { RemoteAuthorityResolverErrorCode } from 'vs/platform/remote/common/remoteAuthorityResolver'; import { IRelativePatternDto } from 'vs/workbench/api/common/extHost.protocol'; -import { CellEditType, ICellPartialMetadataEdit, IDocumentMetadataEdit, isTextStreamMime } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellEditType, ICellPartialMetadataEdit, IDocumentMetadataEdit } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { checkProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions'; import type * as vscode from 'vscode'; @@ -3545,8 +3545,7 @@ export class NotebookCellOutput { for (let i = 0; i < items.length; i++) { const item = items[i]; const normalMime = normalizeMimeType(item.mime); - // We can have multiple text stream mime types in the same output. - if (!seen.has(normalMime) || isTextStreamMime(normalMime)) { + if (!seen.has(normalMime)) { seen.add(normalMime); continue; } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index a32ebc7dc1a93..a7eb492ad43c6 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -35,7 +35,7 @@ import { NOTEBOOK_WEBVIEW_BOUNDARY } from 'vs/workbench/contrib/notebook/browser import { preloadsScriptStr } from 'vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads'; import { transformWebviewThemeVars } from 'vs/workbench/contrib/notebook/browser/view/renderers/webviewThemeMapping'; import { MarkupCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel'; -import { CellUri, INotebookRendererInfo, isTextStreamMime, NotebookSetting, RendererMessagingSpec } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellUri, INotebookRendererInfo, NotebookSetting, RendererMessagingSpec } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookKernel } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { IScopedRendererMessaging } from 'vs/workbench/contrib/notebook/common/notebookRendererMessagingService'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; @@ -44,7 +44,6 @@ import { WebviewWindowDragMonitor } from 'vs/workbench/contrib/webview/browser/w import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService'; import { IWorkbenchEnvironmentService } from 'vs/workbench/services/environment/common/environmentService'; import { FromWebviewMessage, IAckOutputHeight, IClickedDataUrlMessage, ICodeBlockHighlightRequest, IContentWidgetTopRequest, IControllerPreload, ICreationContent, ICreationRequestMessage, IFindMatch, IMarkupCellInitialization, RendererMetadata, ToWebviewMessage } from './webviewMessages'; -import { compressOutputItemStreams } from 'vs/workbench/contrib/notebook/browser/view/renderers/stdOutErrorPreProcessor'; import { DeferredPromise } from 'vs/base/common/async'; export interface ICachedInset { @@ -1277,14 +1276,12 @@ var requirejs = (function() { let updatedContent: ICreationContent | undefined = undefined; if (content.type === RenderOutputType.Extension) { const output = content.source.model; - const firstBuffer = isTextStreamMime(content.mimeType) ? - compressOutputItemStreams(content.mimeType, output.outputs) : - output.outputs.find(op => op.mime === content.mimeType)!.data.buffer; + const first = output.outputs.find(op => op.mime === content.mimeType)!; updatedContent = { type: RenderOutputType.Extension, outputId: outputCache.outputId, - mimeType: content.mimeType, - valueBytes: firstBuffer, + mimeType: first.mime, + valueBytes: first.data.buffer, metadata: output.metadata, }; } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/stdOutErrorPreProcessor.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/stdOutErrorPreProcessor.ts deleted file mode 100644 index 7f20c6316fc1d..0000000000000 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/stdOutErrorPreProcessor.ts +++ /dev/null @@ -1,56 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import { VSBuffer } from 'vs/base/common/buffer'; -import type { IOutputItemDto } from 'vs/workbench/contrib/notebook/common/notebookCommon'; - - -/** - * Given a stream of individual stdout outputs, this function will return the compressed lines, escaping some of the common terminal escape codes. - * E.g. some terminal escape codes would result in the previous line getting cleared, such if we had 3 lines and - * last line contained such a code, then the result string would be just the first two lines. - */ -export function compressOutputItemStreams(mimeType: string, outputs: IOutputItemDto[]) { - const buffers: Uint8Array[] = []; - let startAppending = false; - - // Pick the first set of outputs with the same mime type. - for (const output of outputs) { - if (output.mime === mimeType) { - if ((buffers.length === 0 || startAppending)) { - buffers.push(output.data.buffer); - startAppending = true; - } - } else if (startAppending) { - startAppending = false; - } - } - compressStreamBuffer(buffers); - return VSBuffer.concat(buffers.map(buffer => VSBuffer.wrap(buffer))).buffer; -} -const MOVE_CURSOR_1_LINE_COMMAND = `${String.fromCharCode(27)}[A`; -const MOVE_CURSOR_1_LINE_COMMAND_BYTES = MOVE_CURSOR_1_LINE_COMMAND.split('').map(c => c.charCodeAt(0)); -const LINE_FEED = 10; -function compressStreamBuffer(streams: Uint8Array[]) { - streams.forEach((stream, index) => { - if (index === 0 || stream.length < MOVE_CURSOR_1_LINE_COMMAND.length) { - return; - } - - const previousStream = streams[index - 1]; - - // Remove the previous line if required. - const command = stream.subarray(0, MOVE_CURSOR_1_LINE_COMMAND.length); - if (command[0] === MOVE_CURSOR_1_LINE_COMMAND_BYTES[0] && command[1] === MOVE_CURSOR_1_LINE_COMMAND_BYTES[1] && command[2] === MOVE_CURSOR_1_LINE_COMMAND_BYTES[2]) { - const lastIndexOfLineFeed = previousStream.lastIndexOf(LINE_FEED); - if (lastIndexOfLineFeed === -1) { - return; - } - streams[index - 1] = previousStream.subarray(0, lastIndexOfLineFeed); - streams[index] = stream.subarray(MOVE_CURSOR_1_LINE_COMMAND.length); - } - }); - return streams; -} diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index ab73eaf88b9f0..e77239c0a013e 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -949,11 +949,3 @@ export interface NotebookExtensionDescription { readonly id: ExtensionIdentifier; readonly location: UriComponents | undefined; } - -/** - * Whether the provided mime type is a text streamn like `stdout`, `stderr`. - */ -export function isTextStreamMime(mimeType: string) { - return ['application/vnd.code.notebook.stdout', 'application/x.notebook.stdout', 'application/x.notebook.stream', 'application/vnd.code.notebook.stderr', 'application/x.notebook.stderr'].includes(mimeType); -} - From 47f133b55181e037d92aa6eff3154bbce1e1c5f3 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 16 Sep 2022 09:11:33 +1000 Subject: [PATCH 02/10] Compress stream output items --- extensions/ipynb/src/serializers.ts | 2 +- .../api/common/extHostNotebookDocument.ts | 8 ++ .../view/renderers/backLayerWebView.ts | 10 ++- .../common/model/notebookCellTextModel.ts | 11 ++- .../contrib/notebook/common/notebookCommon.ts | 89 +++++++++++++++++++ 5 files changed, 114 insertions(+), 6 deletions(-) diff --git a/extensions/ipynb/src/serializers.ts b/extensions/ipynb/src/serializers.ts index 4f33b07a1501e..96da8f2c6178c 100644 --- a/extensions/ipynb/src/serializers.ts +++ b/extensions/ipynb/src/serializers.ts @@ -280,7 +280,7 @@ function convertStreamOutput(output: NotebookCellOutput): JupyterOutput { .filter((opit) => opit.mime === CellOutputMimeTypes.stderr || opit.mime === CellOutputMimeTypes.stdout) .map((opit) => textDecoder.decode(opit.data)) .forEach(value => { - // Ensure each line is a seprate entry in an array (ending with \n). + // Ensure each line is a separate entry in an array (ending with \n). const lines = value.split('\n'); // If the last item in `outputs` is not empty and the first item in `lines` is not empty, then concate them. // As they are part of the same line. diff --git a/src/vs/workbench/api/common/extHostNotebookDocument.ts b/src/vs/workbench/api/common/extHostNotebookDocument.ts index c3ebc53b3d6cb..7a687597b080f 100644 --- a/src/vs/workbench/api/common/extHostNotebookDocument.ts +++ b/src/vs/workbench/api/common/extHostNotebookDocument.ts @@ -110,6 +110,14 @@ export class ExtHostCell { output.items.length = 0; } output.items.push(...newItems); + if (output.items.every(item => notebookCommon.isTextStreamMime(item.mime))) { + const compressed = notebookCommon.compressOutputItemStreams(newOutputItems[0].mime, output.items.map(item => item.data)); + output.items.length = 0; + output.items.push({ + mime: newOutputItems[0].mime, + data: compressed.buffer + }); + } } } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index a7eb492ad43c6..b64d96b817ccd 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -35,7 +35,7 @@ import { NOTEBOOK_WEBVIEW_BOUNDARY } from 'vs/workbench/contrib/notebook/browser import { preloadsScriptStr } from 'vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads'; import { transformWebviewThemeVars } from 'vs/workbench/contrib/notebook/browser/view/renderers/webviewThemeMapping'; import { MarkupCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel'; -import { CellUri, INotebookRendererInfo, NotebookSetting, RendererMessagingSpec } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellUri, INotebookRendererInfo, isTextStreamMime, NotebookSetting, RendererMessagingSpec, compressOutputItemStreams } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookKernel } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { IScopedRendererMessaging } from 'vs/workbench/contrib/notebook/common/notebookRendererMessagingService'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; @@ -1276,12 +1276,14 @@ var requirejs = (function() { let updatedContent: ICreationContent | undefined = undefined; if (content.type === RenderOutputType.Extension) { const output = content.source.model; - const first = output.outputs.find(op => op.mime === content.mimeType)!; + const firstBuffer = isTextStreamMime(content.mimeType) ? + compressOutputItemStreams(content.mimeType, output.outputs.map(item => item.data.buffer)) : + output.outputs.find(op => op.mime === content.mimeType)!.data; updatedContent = { type: RenderOutputType.Extension, outputId: outputCache.outputId, - mimeType: first.mime, - valueBytes: first.data.buffer, + mimeType: content.mimeType, + valueBytes: firstBuffer.buffer, metadata: output.metadata, }; } diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts index 7890d2728929c..9e52dcc97fea4 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts @@ -16,7 +16,7 @@ import { TextModel } from 'vs/editor/common/model/textModel'; import { PLAINTEXT_LANGUAGE_ID } from 'vs/editor/common/languages/modesRegistry'; import { ILanguageService } from 'vs/editor/common/languages/language'; import { NotebookCellOutputTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellOutputTextModel'; -import { CellInternalMetadataChangedEvent, CellKind, ICell, ICellDto2, ICellOutput, IOutputDto, IOutputItemDto, NotebookCellCollapseState, NotebookCellInternalMetadata, NotebookCellMetadata, NotebookCellOutputsSplice, TransientOptions } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellInternalMetadataChangedEvent, CellKind, compressOutputItemStreams, ICell, ICellDto2, ICellOutput, IOutputDto, IOutputItemDto, isTextStreamMime, NotebookCellCollapseState, NotebookCellInternalMetadata, NotebookCellMetadata, NotebookCellOutputsSplice, TransientOptions } from 'vs/workbench/contrib/notebook/common/notebookCommon'; export class NotebookCellTextModel extends Disposable implements ICell { private readonly _onDidChangeOutputs = this._register(new Emitter()); @@ -297,6 +297,15 @@ export class NotebookCellTextModel extends Disposable implements ICell { } else { output.replaceData(items); } + if (output.outputs.every(item => isTextStreamMime(item.mime))) { + const mime = output.outputs[0].mime; + const compressed = compressOutputItemStreams(output.outputs[0].mime, output.outputs.map(item => item.data.buffer)); + output.outputs.length = 0; + output.outputs.push({ + mime, + data: compressed + }); + } this._onDidChangeOutputItems.fire(); return true; diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index e77239c0a013e..6f56094ae2958 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -949,3 +949,92 @@ export interface NotebookExtensionDescription { readonly id: ExtensionIdentifier; readonly location: UriComponents | undefined; } + +/** + * Whether the provided mime type is a text streamn like `stdout`, `stderr`. + */ +export function isTextStreamMime(mimeType: string) { + return ['application/vnd.code.notebook.stdout', 'application/x.notebook.stdout', 'application/x.notebook.stream', 'application/vnd.code.notebook.stderr', 'application/x.notebook.stderr'].includes(mimeType); +} + + + +/** + * Given a stream of individual stdout outputs, this function will return the compressed lines, escaping some of the common terminal escape codes. + * E.g. some terminal escape codes would result in the previous line getting cleared, such if we had 3 lines and + * last line contained such a code, then the result string would be just the first two lines. + */ +const textDecoder = new TextDecoder(); +export function compressOutputItemStreams(mimeType: string, outputs: Uint8Array[]) { + const buffers: Uint8Array[] = []; + let startAppending = false; + + // Pick the first set of outputs with the same mime type. + for (const output of outputs) { + if ((buffers.length === 0 || startAppending)) { + buffers.push(output); + startAppending = true; + } + } + compressStreamBuffer(buffers); + const value = textDecoder.decode(VSBuffer.concat(buffers.map(buffer => VSBuffer.wrap(buffer))).buffer); + return VSBuffer.fromString(formatStreamText(value)); +} +const MOVE_CURSOR_1_LINE_COMMAND = `${String.fromCharCode(27)}[A`; +const MOVE_CURSOR_1_LINE_COMMAND_BYTES = MOVE_CURSOR_1_LINE_COMMAND.split('').map(c => c.charCodeAt(0)); +const LINE_FEED = 10; +function compressStreamBuffer(streams: Uint8Array[]) { + streams.forEach((stream, index) => { + if (index === 0 || stream.length < MOVE_CURSOR_1_LINE_COMMAND.length) { + return; + } + + const previousStream = streams[index - 1]; + + // Remove the previous line if required. + const command = stream.subarray(0, MOVE_CURSOR_1_LINE_COMMAND.length); + if (command[0] === MOVE_CURSOR_1_LINE_COMMAND_BYTES[0] && command[1] === MOVE_CURSOR_1_LINE_COMMAND_BYTES[1] && command[2] === MOVE_CURSOR_1_LINE_COMMAND_BYTES[2]) { + const lastIndexOfLineFeed = previousStream.lastIndexOf(LINE_FEED); + if (lastIndexOfLineFeed === -1) { + return; + } + streams[index - 1] = previousStream.subarray(0, lastIndexOfLineFeed); + streams[index] = stream.subarray(MOVE_CURSOR_1_LINE_COMMAND.length); + } + }); + return streams; +} + + + +// Took this from jupyter/notebook +// https://github.com/jupyter/notebook/blob/b8b66332e2023e83d2ee04f83d8814f567e01a4e/notebook/static/base/js/utils.js +// Remove characters that are overridden by backspace characters +function fixBackspace(txt: string) { + let tmp = txt; + do { + txt = tmp; + // Cancel out anything-but-newline followed by backspace + tmp = txt.replace(/[^\n]\x08/gm, ''); + } while (tmp.length < txt.length); + return txt; +} + +// Remove chunks that should be overridden by the effect of +// carriage return characters +// From https://github.com/jupyter/notebook/blob/master/notebook/static/base/js/utils.js +function fixCarriageReturn(txt: string) { + txt = txt.replace(/\r+\n/gm, '\n'); // \r followed by \n --> newline + while (txt.search(/\r[^$]/g) > -1) { + const base = txt.match(/^(.*)\r+/m)![1]; + let insert = txt.match(/\r+(.*)$/m)![1]; + insert = insert + base.slice(insert.length, base.length); + txt = txt.replace(/\r+.*$/m, '\r').replace(/^.*\r/m, insert); + } + return txt; +} + +function formatStreamText(str: string): string { + // Do the same thing jupyter is doing + return fixCarriageReturn(fixBackspace(str)); +} From 3ade74ad0acc8700bdd930fc44a22db2362f2b99 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 19 Sep 2022 15:05:35 +1000 Subject: [PATCH 03/10] Minor perf improvements --- .../api/common/extHostNotebookDocument.ts | 28 +++++++++++++++---- .../view/renderers/backLayerWebView.ts | 6 ++-- .../common/model/notebookCellTextModel.ts | 28 +++++++++++++++---- .../contrib/notebook/common/notebookCommon.ts | 16 +++++++---- 4 files changed, 57 insertions(+), 21 deletions(-) diff --git a/src/vs/workbench/api/common/extHostNotebookDocument.ts b/src/vs/workbench/api/common/extHostNotebookDocument.ts index 7a687597b080f..38437605cec08 100644 --- a/src/vs/workbench/api/common/extHostNotebookDocument.ts +++ b/src/vs/workbench/api/common/extHostNotebookDocument.ts @@ -110,12 +110,30 @@ export class ExtHostCell { output.items.length = 0; } output.items.push(...newItems); - if (output.items.every(item => notebookCommon.isTextStreamMime(item.mime))) { - const compressed = notebookCommon.compressOutputItemStreams(newOutputItems[0].mime, output.items.map(item => item.data)); + + if (output.items.length > 1 && output.items.every(item => notebookCommon.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[] = []; + output.items.forEach(item => { + let items: Uint8Array[]; + 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); + }); output.items.length = 0; - output.items.push({ - mime: newOutputItems[0].mime, - data: compressed.buffer + mimeTypes.forEach(mime => { + const compressed = notebookCommon.compressOutputItemStreams(mimeOutputs.get(mime)!); + output.items.push({ + mime, + data: compressed.buffer + }); }); } } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index b64d96b817ccd..0facd39e9e110 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -35,7 +35,7 @@ import { NOTEBOOK_WEBVIEW_BOUNDARY } from 'vs/workbench/contrib/notebook/browser import { preloadsScriptStr } from 'vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads'; import { transformWebviewThemeVars } from 'vs/workbench/contrib/notebook/browser/view/renderers/webviewThemeMapping'; import { MarkupCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel'; -import { CellUri, INotebookRendererInfo, isTextStreamMime, NotebookSetting, RendererMessagingSpec, compressOutputItemStreams } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellUri, INotebookRendererInfo, isTextStreamMime, NotebookSetting, RendererMessagingSpec } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookKernel } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { IScopedRendererMessaging } from 'vs/workbench/contrib/notebook/common/notebookRendererMessagingService'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; @@ -1276,9 +1276,7 @@ var requirejs = (function() { let updatedContent: ICreationContent | undefined = undefined; if (content.type === RenderOutputType.Extension) { const output = content.source.model; - const firstBuffer = isTextStreamMime(content.mimeType) ? - compressOutputItemStreams(content.mimeType, output.outputs.map(item => item.data.buffer)) : - output.outputs.find(op => op.mime === content.mimeType)!.data; + const firstBuffer = output.outputs.find(op => op.mime === content.mimeType)!.data; updatedContent = { type: RenderOutputType.Extension, outputId: outputCache.outputId, diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts index 9e52dcc97fea4..e8576c8234af9 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts @@ -297,13 +297,29 @@ export class NotebookCellTextModel extends Disposable implements ICell { } else { output.replaceData(items); } - if (output.outputs.every(item => isTextStreamMime(item.mime))) { - const mime = output.outputs[0].mime; - const compressed = compressOutputItemStreams(output.outputs[0].mime, output.outputs.map(item => item.data.buffer)); + if (output.outputs.length > 1 && output.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[] = []; + output.outputs.forEach(item => { + let items: Uint8Array[]; + 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.buffer); + }); output.outputs.length = 0; - output.outputs.push({ - mime, - data: compressed + mimeTypes.forEach(mime => { + const compressed = compressOutputItemStreams(mimeOutputs.get(mime)!); + output.outputs.push({ + mime, + data: compressed + }); }); } diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 6f56094ae2958..27e5e0fe33c50 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -965,7 +965,7 @@ export function isTextStreamMime(mimeType: string) { * last line contained such a code, then the result string would be just the first two lines. */ const textDecoder = new TextDecoder(); -export function compressOutputItemStreams(mimeType: string, outputs: Uint8Array[]) { +export function compressOutputItemStreams(outputs: Uint8Array[]) { const buffers: Uint8Array[] = []; let startAppending = false; @@ -977,8 +977,7 @@ export function compressOutputItemStreams(mimeType: string, outputs: Uint8Array[ } } compressStreamBuffer(buffers); - const value = textDecoder.decode(VSBuffer.concat(buffers.map(buffer => VSBuffer.wrap(buffer))).buffer); - return VSBuffer.fromString(formatStreamText(value)); + return formatStreamText(VSBuffer.concat(buffers.map(buffer => VSBuffer.wrap(buffer)))); } const MOVE_CURSOR_1_LINE_COMMAND = `${String.fromCharCode(27)}[A`; const MOVE_CURSOR_1_LINE_COMMAND_BYTES = MOVE_CURSOR_1_LINE_COMMAND.split('').map(c => c.charCodeAt(0)); @@ -1002,7 +1001,6 @@ function compressStreamBuffer(streams: Uint8Array[]) { streams[index] = stream.subarray(MOVE_CURSOR_1_LINE_COMMAND.length); } }); - return streams; } @@ -1034,7 +1032,13 @@ function fixCarriageReturn(txt: string) { return txt; } -function formatStreamText(str: string): string { +const BACKSPACE_CHARACTER = '\b'.charCodeAt(0); +const CARRIAGE_RETURN_CHARACTER = '\r'.charCodeAt(0); +function formatStreamText(buffer: VSBuffer): VSBuffer { + // We have special handling for backspace and carriage return characters. + if (!buffer.buffer.includes(BACKSPACE_CHARACTER) && !buffer.buffer.includes(CARRIAGE_RETURN_CHARACTER)) { + return buffer; + } // Do the same thing jupyter is doing - return fixCarriageReturn(fixBackspace(str)); + return VSBuffer.fromString(fixCarriageReturn(fixBackspace(textDecoder.decode(buffer.buffer)))); } From 2d6ce1ab2243affba4dc7f831ae673b5ebdcc967 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 19 Sep 2022 15:12:12 +1000 Subject: [PATCH 04/10] Misc --- .../contrib/notebook/browser/view/renderers/backLayerWebView.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index 0facd39e9e110..f01090a0561e8 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -35,7 +35,7 @@ import { NOTEBOOK_WEBVIEW_BOUNDARY } from 'vs/workbench/contrib/notebook/browser import { preloadsScriptStr } from 'vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads'; import { transformWebviewThemeVars } from 'vs/workbench/contrib/notebook/browser/view/renderers/webviewThemeMapping'; import { MarkupCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel'; -import { CellUri, INotebookRendererInfo, isTextStreamMime, NotebookSetting, RendererMessagingSpec } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellUri, INotebookRendererInfo, NotebookSetting, RendererMessagingSpec } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookKernel } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { IScopedRendererMessaging } from 'vs/workbench/contrib/notebook/common/notebookRendererMessagingService'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; From 554fc2cbca1c9b44078d46baee6d93f46e358bbc Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 19 Sep 2022 15:22:56 +1000 Subject: [PATCH 05/10] Comments --- src/vs/workbench/contrib/notebook/common/notebookCommon.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 27e5e0fe33c50..ade4d44030c78 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -1036,6 +1036,7 @@ const BACKSPACE_CHARACTER = '\b'.charCodeAt(0); const CARRIAGE_RETURN_CHARACTER = '\r'.charCodeAt(0); function formatStreamText(buffer: VSBuffer): VSBuffer { // We have special handling for backspace and carriage return characters. + // Don't unnecessary decode the bytes if we don't need to perform any processing. if (!buffer.buffer.includes(BACKSPACE_CHARACTER) && !buffer.buffer.includes(CARRIAGE_RETURN_CHARACTER)) { return buffer; } From 9ddbe3bb1930431be3f0fdbdd5ad738b62881a98 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 26 Sep 2022 12:30:41 +1000 Subject: [PATCH 06/10] Added tests --- src/vs/workbench/api/common/extHostTypes.ts | 4 +- .../api/test/browser/extHostNotebook.test.ts | 75 ++++++++++++- .../contrib/notebook/common/notebookCommon.ts | 2 +- .../test/browser/notebookTextModel.test.ts | 106 ++++++++++++++++++ 4 files changed, 183 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index a717fa90515a1..f63260aa9fed2 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -17,7 +17,7 @@ import { IExtensionDescription } from 'vs/platform/extensions/common/extensions' import { FileSystemProviderErrorCode, markAsFileSystemProviderError } from 'vs/platform/files/common/files'; import { RemoteAuthorityResolverErrorCode } from 'vs/platform/remote/common/remoteAuthorityResolver'; import { IRelativePatternDto } from 'vs/workbench/api/common/extHost.protocol'; -import { CellEditType, ICellPartialMetadataEdit, IDocumentMetadataEdit } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellEditType, ICellPartialMetadataEdit, IDocumentMetadataEdit, isTextStreamMime } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { checkProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions'; import type * as vscode from 'vscode'; @@ -3545,7 +3545,7 @@ export class NotebookCellOutput { for (let i = 0; i < items.length; i++) { const item = items[i]; const normalMime = normalizeMimeType(item.mime); - if (!seen.has(normalMime)) { + if (!seen.has(normalMime) && !isTextStreamMime(item.mime)) { seen.add(normalMime); continue; } diff --git a/src/vs/workbench/api/test/browser/extHostNotebook.test.ts b/src/vs/workbench/api/test/browser/extHostNotebook.test.ts index 09e785c307b25..60f453ec1f3c1 100644 --- a/src/vs/workbench/api/test/browser/extHostNotebook.test.ts +++ b/src/vs/workbench/api/test/browser/extHostNotebook.test.ts @@ -10,7 +10,7 @@ import { TestRPCProtocol } from 'vs/workbench/api/test/common/testRPCProtocol'; import { DisposableStore } from 'vs/base/common/lifecycle'; import { NullLogService } from 'vs/platform/log/common/log'; import { mock } from 'vs/base/test/common/mock'; -import { IModelAddedData, MainContext, MainThreadCommandsShape, MainThreadNotebookShape } from 'vs/workbench/api/common/extHost.protocol'; +import { IModelAddedData, MainContext, MainThreadCommandsShape, MainThreadNotebookShape, NotebookCellsChangedEventDto, NotebookOutputItemDto } from 'vs/workbench/api/common/extHost.protocol'; import { ExtHostNotebookController } from 'vs/workbench/api/common/extHostNotebook'; import { ExtHostNotebookDocument } from 'vs/workbench/api/common/extHostNotebookDocument'; import { CellKind, CellUri, NotebookCellsChangeType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; @@ -542,4 +542,77 @@ suite('NotebookCell#Document', function () { assert.ok(cellChange.metadata === undefined); assert.ok(cellChange.outputs === undefined); }); + + async function replaceOutputs(cellIndex: number, outputId: string, outputItems: NotebookOutputItemDto[]) { + const changeEvent = Event.toPromise(extHostNotebookDocuments.onDidChangeNotebookDocument); + extHostNotebookDocuments.$acceptModelChanged(notebook.uri, new SerializableObjectWithBuffers({ + versionId: notebook.apiNotebook.version + 1, + rawEvents: [{ + kind: NotebookCellsChangeType.Output, + index: cellIndex, + outputs: [{ outputId, items: outputItems }] + }] + }), false); + await changeEvent; + } + async function appendOutputItem(cellIndex: number, outputId: string, outputItems: NotebookOutputItemDto[]) { + const changeEvent = Event.toPromise(extHostNotebookDocuments.onDidChangeNotebookDocument); + extHostNotebookDocuments.$acceptModelChanged(notebook.uri, new SerializableObjectWithBuffers({ + versionId: notebook.apiNotebook.version + 1, + rawEvents: [{ + kind: NotebookCellsChangeType.OutputItem, + index: cellIndex, + append: true, + outputId, + outputItems + }] + }), false); + await changeEvent; + } + test('Append multiple text/plain output items', async function () { + await replaceOutputs(1, '1', [{ mime: 'text/plain', valueBytes: VSBuffer.fromString('foo') }]); + await appendOutputItem(1, '1', [{ mime: 'text/plain', valueBytes: VSBuffer.fromString('bar') }]); + await appendOutputItem(1, '1', [{ mime: 'text/plain', valueBytes: VSBuffer.fromString('baz') }]); + + + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs.length, 1); + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs[0].items.length, 3); + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs[0].items[0].mime, 'text/plain'); + assert.strictEqual(VSBuffer.wrap(notebook.apiNotebook.cellAt(1).outputs[0].items[0].data).toString(), 'foo'); + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs[0].items[1].mime, 'text/plain'); + assert.strictEqual(VSBuffer.wrap(notebook.apiNotebook.cellAt(1).outputs[0].items[1].data).toString(), 'bar'); + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs[0].items[2].mime, 'text/plain'); + assert.strictEqual(VSBuffer.wrap(notebook.apiNotebook.cellAt(1).outputs[0].items[2].data).toString(), 'baz'); + }); + test('Append multiple stdout stream output items to an output with another mime', async function () { + await replaceOutputs(1, '1', [{ mime: 'text/plain', valueBytes: VSBuffer.fromString('foo') }]); + await appendOutputItem(1, '1', [{ mime: 'application/vnd.code.notebook.stdout', valueBytes: VSBuffer.fromString('bar') }]); + await appendOutputItem(1, '1', [{ mime: 'application/vnd.code.notebook.stdout', valueBytes: VSBuffer.fromString('baz') }]); + + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs.length, 1); + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs[0].items.length, 3); + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs[0].items[0].mime, 'text/plain'); + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs[0].items[1].mime, 'application/vnd.code.notebook.stdout'); + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs[0].items[2].mime, 'application/vnd.code.notebook.stdout'); + }); + test('Compress multiple stdout stream output items', async function () { + await replaceOutputs(1, '1', [{ mime: 'application/vnd.code.notebook.stdout', valueBytes: VSBuffer.fromString('foo') }]); + await appendOutputItem(1, '1', [{ mime: 'application/vnd.code.notebook.stdout', valueBytes: VSBuffer.fromString('bar') }]); + await appendOutputItem(1, '1', [{ mime: 'application/vnd.code.notebook.stdout', valueBytes: VSBuffer.fromString('baz') }]); + + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs.length, 1); + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs[0].items.length, 1); + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs[0].items[0].mime, 'application/vnd.code.notebook.stdout'); + assert.strictEqual(VSBuffer.wrap(notebook.apiNotebook.cellAt(1).outputs[0].items[0].data).toString(), 'foobarbaz'); + }); + test('Compress multiple stderr stream output items', async function () { + await replaceOutputs(1, '1', [{ mime: 'application/vnd.code.notebook.stderr', valueBytes: VSBuffer.fromString('foo') }]); + await appendOutputItem(1, '1', [{ mime: 'application/vnd.code.notebook.stderr', valueBytes: VSBuffer.fromString('bar') }]); + await appendOutputItem(1, '1', [{ mime: 'application/vnd.code.notebook.stderr', valueBytes: VSBuffer.fromString('baz') }]); + + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs.length, 1); + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs[0].items.length, 1); + assert.strictEqual(notebook.apiNotebook.cellAt(1).outputs[0].items[0].mime, 'application/vnd.code.notebook.stderr'); + assert.strictEqual(VSBuffer.wrap(notebook.apiNotebook.cellAt(1).outputs[0].items[0].data).toString(), 'foobarbaz'); + }); }); diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index ade4d44030c78..d7fb943b513dc 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -954,7 +954,7 @@ export interface NotebookExtensionDescription { * Whether the provided mime type is a text streamn like `stdout`, `stderr`. */ export function isTextStreamMime(mimeType: string) { - return ['application/vnd.code.notebook.stdout', 'application/x.notebook.stdout', 'application/x.notebook.stream', 'application/vnd.code.notebook.stderr', 'application/x.notebook.stderr'].includes(mimeType); + return ['application/vnd.code.notebook.stdout', 'application/vnd.code.notebook.stderr'].includes(mimeType); } diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookTextModel.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookTextModel.test.ts index 455d706b63745..4da03a9f24814 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookTextModel.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookTextModel.test.ts @@ -1043,4 +1043,110 @@ suite('NotebookTextModel', () => { assert.equal(model.cells[0].outputs[0].outputs[0].data.toString(), '_World_'); }); }); + test('Append multiple text/plain output items', async function () { + await withTestNotebook([ + ['var a = 1;', 'javascript', CellKind.Code, [{ + outputId: '1', + outputs: [{ mime: 'text/plain', data: valueBytesFromString('foo') }] + }], {}] + ], (editor) => { + const model = editor.textModel; + const edits: ICellEditOperation[] = [ + { + editType: CellEditType.OutputItems, + outputId: '1', + append: true, + items: [{ mime: 'text/plain', data: VSBuffer.fromString('bar') }, { mime: 'text/plain', data: VSBuffer.fromString('baz') }] + } + ]; + model.applyEdits(edits, true, undefined, () => undefined, undefined, true); + assert.equal(model.cells.length, 1); + assert.equal(model.cells[0].outputs.length, 1); + assert.equal(model.cells[0].outputs[0].outputs.length, 3); + assert.equal(model.cells[0].outputs[0].outputs[0].mime, 'text/plain'); + assert.equal(model.cells[0].outputs[0].outputs[0].data.toString(), 'foo'); + assert.equal(model.cells[0].outputs[0].outputs[1].mime, 'text/plain'); + assert.equal(model.cells[0].outputs[0].outputs[1].data.toString(), 'bar'); + assert.equal(model.cells[0].outputs[0].outputs[2].mime, 'text/plain'); + assert.equal(model.cells[0].outputs[0].outputs[2].data.toString(), 'baz'); + }); + }); + test('Append multiple stdout stream output items to an output with another mime', async function () { + await withTestNotebook([ + ['var a = 1;', 'javascript', CellKind.Code, [{ + outputId: '1', + outputs: [{ mime: 'text/plain', data: valueBytesFromString('foo') }] + }], {}] + ], (editor) => { + const model = editor.textModel; + const edits: ICellEditOperation[] = [ + { + editType: CellEditType.OutputItems, + outputId: '1', + append: true, + items: [{ mime: 'application/vnd.code.notebook.stdout', data: VSBuffer.fromString('bar') }, { mime: 'application/vnd.code.notebook.stdout', data: VSBuffer.fromString('baz') }] + } + ]; + model.applyEdits(edits, true, undefined, () => undefined, undefined, true); + assert.equal(model.cells.length, 1); + assert.equal(model.cells[0].outputs.length, 1); + assert.equal(model.cells[0].outputs[0].outputs.length, 3); + assert.equal(model.cells[0].outputs[0].outputs[0].mime, 'text/plain'); + assert.equal(model.cells[0].outputs[0].outputs[0].data.toString(), 'foo'); + assert.equal(model.cells[0].outputs[0].outputs[1].mime, 'application/vnd.code.notebook.stdout'); + assert.equal(model.cells[0].outputs[0].outputs[1].data.toString(), 'bar'); + assert.equal(model.cells[0].outputs[0].outputs[2].mime, 'application/vnd.code.notebook.stdout'); + assert.equal(model.cells[0].outputs[0].outputs[2].data.toString(), 'baz'); + }); + }); + test('Compress multiple stdout stream output items', async function () { + await withTestNotebook([ + ['var a = 1;', 'javascript', CellKind.Code, [{ + outputId: '1', + outputs: [{ mime: 'application/vnd.code.notebook.stdout', data: valueBytesFromString('foo') }] + }], {}] + ], (editor) => { + const model = editor.textModel; + const edits: ICellEditOperation[] = [ + { + editType: CellEditType.OutputItems, + outputId: '1', + append: true, + items: [{ mime: 'application/vnd.code.notebook.stdout', data: VSBuffer.fromString('bar') }, { mime: 'application/vnd.code.notebook.stdout', data: VSBuffer.fromString('baz') }] + } + ]; + model.applyEdits(edits, true, undefined, () => undefined, undefined, true); + assert.equal(model.cells.length, 1); + assert.equal(model.cells[0].outputs.length, 1); + assert.equal(model.cells[0].outputs[0].outputs.length, 1); + assert.equal(model.cells[0].outputs[0].outputs[0].mime, 'application/vnd.code.notebook.stdout'); + assert.equal(model.cells[0].outputs[0].outputs[0].data.toString(), 'foobarbaz'); + }); + + }); + test('Compress multiple stderr stream output items', async function () { + await withTestNotebook([ + ['var a = 1;', 'javascript', CellKind.Code, [{ + outputId: '1', + outputs: [{ mime: 'application/vnd.code.notebook.stderr', data: valueBytesFromString('foo') }] + }], {}] + ], (editor) => { + const model = editor.textModel; + const edits: ICellEditOperation[] = [ + { + editType: CellEditType.OutputItems, + outputId: '1', + append: true, + items: [{ mime: 'application/vnd.code.notebook.stderr', data: VSBuffer.fromString('bar') }, { mime: 'application/vnd.code.notebook.stderr', data: VSBuffer.fromString('baz') }] + } + ]; + model.applyEdits(edits, true, undefined, () => undefined, undefined, true); + assert.equal(model.cells.length, 1); + assert.equal(model.cells[0].outputs.length, 1); + assert.equal(model.cells[0].outputs[0].outputs.length, 1); + assert.equal(model.cells[0].outputs[0].outputs[0].mime, 'application/vnd.code.notebook.stderr'); + assert.equal(model.cells[0].outputs[0].outputs[0].data.toString(), 'foobarbaz'); + }); + + }); }); From 0a8e55dace4dcad79ed466899162ec822a315e83 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 26 Sep 2022 12:32:44 +1000 Subject: [PATCH 07/10] Merge issue --- src/vs/workbench/api/common/extHostTypes.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index f63260aa9fed2..ed7553b4adbcb 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -3545,7 +3545,8 @@ export class NotebookCellOutput { for (let i = 0; i < items.length; i++) { const item = items[i]; const normalMime = normalizeMimeType(item.mime); - if (!seen.has(normalMime) && !isTextStreamMime(item.mime)) { + // We can have multiple text stream mime types in the same output. + if (!seen.has(normalMime) || isTextStreamMime(item.mime)) { seen.add(normalMime); continue; } From 3219f4927ca2987b2e7d73378ec3ea5cd430c3d8 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 26 Sep 2022 12:33:38 +1000 Subject: [PATCH 08/10] More merge issues --- src/vs/workbench/api/common/extHostTypes.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index ed7553b4adbcb..f7b89d987a907 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -3546,7 +3546,7 @@ export class NotebookCellOutput { const item = items[i]; const normalMime = normalizeMimeType(item.mime); // We can have multiple text stream mime types in the same output. - if (!seen.has(normalMime) || isTextStreamMime(item.mime)) { + if (!seen.has(normalMime) || isTextStreamMime(normalMime)) { seen.add(normalMime); continue; } From 06f5422bec9943d3bc266caf362e9d991fea4fab Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Mon, 26 Sep 2022 13:08:02 +1000 Subject: [PATCH 09/10] Misc --- src/vs/workbench/contrib/notebook/common/notebookCommon.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index d7fb943b513dc..02b9e0ddcc2d4 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -951,7 +951,7 @@ export interface NotebookExtensionDescription { } /** - * Whether the provided mime type is a text streamn like `stdout`, `stderr`. + * Whether the provided mime type is a text stream like `stdout`, `stderr`. */ export function isTextStreamMime(mimeType: string) { return ['application/vnd.code.notebook.stdout', 'application/vnd.code.notebook.stderr'].includes(mimeType); From 42522fdedbd9343ab26610bc52e98422a2028f9b Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 12 Oct 2022 07:14:59 +1100 Subject: [PATCH 10/10] Address code review comments --- .../contrib/notebook/common/notebookCommon.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 02b9e0ddcc2d4..0e2f1fe9ea62a 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -958,13 +958,13 @@ export function isTextStreamMime(mimeType: string) { } +const textDecoder = new TextDecoder(); /** * Given a stream of individual stdout outputs, this function will return the compressed lines, escaping some of the common terminal escape codes. * E.g. some terminal escape codes would result in the previous line getting cleared, such if we had 3 lines and * last line contained such a code, then the result string would be just the first two lines. */ -const textDecoder = new TextDecoder(); export function compressOutputItemStreams(outputs: Uint8Array[]) { const buffers: Uint8Array[] = []; let startAppending = false; @@ -1005,9 +1005,11 @@ function compressStreamBuffer(streams: Uint8Array[]) { -// Took this from jupyter/notebook -// https://github.com/jupyter/notebook/blob/b8b66332e2023e83d2ee04f83d8814f567e01a4e/notebook/static/base/js/utils.js -// Remove characters that are overridden by backspace characters +/** + * Took this from jupyter/notebook + * https://github.com/jupyter/notebook/blob/b8b66332e2023e83d2ee04f83d8814f567e01a4e/notebook/static/base/js/utils.js + * Remove characters that are overridden by backspace characters + */ function fixBackspace(txt: string) { let tmp = txt; do { @@ -1018,9 +1020,10 @@ function fixBackspace(txt: string) { return txt; } -// Remove chunks that should be overridden by the effect of -// carriage return characters -// From https://github.com/jupyter/notebook/blob/master/notebook/static/base/js/utils.js +/** + * Remove chunks that should be overridden by the effect of carriage return characters + * From https://github.com/jupyter/notebook/blob/master/notebook/static/base/js/utils.js + */ function fixCarriageReturn(txt: string) { txt = txt.replace(/\r+\n/gm, '\n'); // \r followed by \n --> newline while (txt.search(/\r[^$]/g) > -1) {