From c397afa3bd361e11c5fbda762dc8664bdc5ff2df Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 24 Aug 2021 16:27:10 -0700 Subject: [PATCH] Revert "Merge pull request #131035 from DonJayamanne/issue129370" This reverts commit b23486ef7a22af1ee9879708787c52a89b2653f7, reversing changes made to 9e0732389bb21bf022e288fa764c6b4f045c1cf6. --- extensions/ipynb/src/deserializers.ts | 41 +--- extensions/ipynb/src/notebookSerializer.ts | 4 +- extensions/ipynb/src/serializers.ts | 30 +-- extensions/ipynb/src/test/serializers.test.ts | 205 +----------------- 4 files changed, 22 insertions(+), 258 deletions(-) diff --git a/extensions/ipynb/src/deserializers.ts b/extensions/ipynb/src/deserializers.ts index 20d6dabdbf7..af9ba73a20f 100644 --- a/extensions/ipynb/src/deserializers.ts +++ b/extensions/ipynb/src/deserializers.ts @@ -53,37 +53,18 @@ const orderOfMimeTypes = [ 'text/plain' ]; -function isEmptyVendoredMimeType(outputItem: NotebookCellOutputItem) { - if (outputItem.mime.startsWith('application/vnd.')) { - try { - return outputItem.data.byteLength === 0 || Buffer.from(outputItem.data).toString().length === 0; - } catch { } - } - return false; -} -function isMimeTypeMatch(value: string, compareWith: string) { - if (value.endsWith('.*')) { - value = value.substr(0, value.indexOf('.*')); - } - return compareWith.startsWith(value); -} - function sortOutputItemsBasedOnDisplayOrder(outputItems: NotebookCellOutputItem[]): NotebookCellOutputItem[] { - return outputItems - .map(item => { - let index = orderOfMimeTypes.findIndex((mime) => isMimeTypeMatch(mime, item.mime)); - // Sometimes we can have mime types with empty data, e.g. when using holoview we can have `application/vnd.holoviews_load.v0+json` with empty value. - // & in these cases we have HTML/JS and those take precedence. - // https://github.com/microsoft/vscode-jupyter/issues/6109 - if (isEmptyVendoredMimeType(item)) { - index = -1; + return outputItems.sort((outputItemA, outputItemB) => { + const isMimeTypeMatch = (value: string, compareWith: string) => { + if (value.endsWith('.*')) { + value = value.substr(0, value.indexOf('.*')); } - index = index === -1 ? 100 : index; - return { - item, index - }; - }) - .sort((outputItemA, outputItemB) => outputItemA.index - outputItemB.index).map(item => item.item); + return compareWith.startsWith(value); + }; + const indexOfMimeTypeA = orderOfMimeTypes.findIndex(mime => isMimeTypeMatch(outputItemA.mime, mime)); + const indexOfMimeTypeB = orderOfMimeTypes.findIndex(mime => isMimeTypeMatch(outputItemB.mime, mime)); + return indexOfMimeTypeA - indexOfMimeTypeB; + }); } @@ -256,7 +237,7 @@ cellOutputMappers.set('update_display_data', translateDisplayDataOutput); cellOutputMappers.set('error', translateErrorOutput); cellOutputMappers.set('stream', translateStreamOutput); -export function jupyterCellOutputToCellOutput(output: nbformat.IOutput): NotebookCellOutput { +function jupyterCellOutputToCellOutput(output: nbformat.IOutput): NotebookCellOutput { /** * Stream, `application/x.notebook.stream` * Error, `application/x.notebook.error-traceback` diff --git a/extensions/ipynb/src/notebookSerializer.ts b/extensions/ipynb/src/notebookSerializer.ts index fda91fddf04..381607b9254 100644 --- a/extensions/ipynb/src/notebookSerializer.ts +++ b/extensions/ipynb/src/notebookSerializer.ts @@ -40,8 +40,8 @@ export class NotebookSerializer implements vscode.NotebookSerializer { } } - // Then compute indent from the contents (only use first 1K characters as a perf optimization) - const indentAmount = contents ? detectIndent(contents.substring(0, 1_000)).indent : ' '; + // Then compute indent from the contents + const indentAmount = contents ? detectIndent(contents).indent : ' '; const preferredCellLanguage = getPreferredLanguage(json.metadata); // Ensure we always have a blank cell. diff --git a/extensions/ipynb/src/serializers.ts b/extensions/ipynb/src/serializers.ts index e6f1e2faaa4..ae273c0729f 100644 --- a/extensions/ipynb/src/serializers.ts +++ b/extensions/ipynb/src/serializers.ts @@ -94,7 +94,7 @@ function translateCellDisplayOutput(output: NotebookCellOutput): JupyterOutput { case 'display_data': { result = { output_type: 'display_data', - data: output.items.reduce((prev: any, curr) => { + data: output.items.reduceRight((prev: any, curr) => { prev[curr.mime] = convertOutputMimeToJupyterOutput(curr.mime, curr.data as Uint8Array); return prev; }, {}), @@ -105,7 +105,7 @@ function translateCellDisplayOutput(output: NotebookCellOutput): JupyterOutput { case 'execute_result': { result = { output_type: 'execute_result', - data: output.items.reduce((prev: any, curr) => { + data: output.items.reduceRight((prev: any, curr) => { prev[curr.mime] = convertOutputMimeToJupyterOutput(curr.mime, curr.data as Uint8Array); return prev; }, {}), @@ -118,7 +118,7 @@ function translateCellDisplayOutput(output: NotebookCellOutput): JupyterOutput { case 'update_display_data': { result = { output_type: 'update_display_data', - data: output.items.reduce((prev: any, curr) => { + data: output.items.reduceRight((prev: any, curr) => { prev[curr.mime] = convertOutputMimeToJupyterOutput(curr.mime, curr.data as Uint8Array); return prev; }, {}), @@ -163,7 +163,7 @@ function translateCellDisplayOutput(output: NotebookCellOutput): JupyterOutput { unknownOutput.metadata = customMetadata.metadata; } if (output.items.length > 0) { - unknownOutput.data = output.items.reduce((prev: any, curr) => { + unknownOutput.data = output.items.reduceRight((prev: any, curr) => { prev[curr.mime] = convertOutputMimeToJupyterOutput(curr.mime, curr.data as Uint8Array); return prev; }, {}); @@ -224,33 +224,17 @@ type JupyterOutput = | nbformat.IError; function convertStreamOutput(output: NotebookCellOutput): JupyterOutput { - const outputs: string[] = []; - output.items + const outputs = output.items .filter((opit) => opit.mime === CellOutputMimeTypes.stderr || opit.mime === CellOutputMimeTypes.stdout) .map((opit) => convertOutputMimeToJupyterOutput(opit.mime, opit.data as Uint8Array) as string) - .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); - } - }); - // Skip last one if empty (it's the only one that could be length 0) - if (outputs.length && outputs[outputs.length - 1].length === 0) { - outputs.pop(); - } + .reduceRight((prev, curr) => prev.concat(curr), []); const streamType = getOutputStreamType(output) || 'stdout'; return { output_type: 'stream', name: streamType, - text: outputs + text: splitMultilineString(outputs.join('')) }; } diff --git a/extensions/ipynb/src/test/serializers.test.ts b/extensions/ipynb/src/test/serializers.test.ts index c17ea992954..740eecc38d5 100644 --- a/extensions/ipynb/src/test/serializers.test.ts +++ b/extensions/ipynb/src/test/serializers.test.ts @@ -6,7 +6,7 @@ import { nbformat } from '@jupyterlab/coreutils'; import * as assert from 'assert'; import * as vscode from 'vscode'; -import { jupyterCellOutputToCellOutput, jupyterNotebookModelToNotebookData } from '../deserializers'; +import { jupyterNotebookModelToNotebookData } from '../deserializers'; function deepStripProperties(obj: any, props: string[]) { for (let prop in obj) { @@ -52,6 +52,7 @@ suite('ipynb serializer', () => { assert.deepStrictEqual(notebook.cells, [expectedCodeCell, expectedMarkdownCell]); }); + suite('Outputs', () => { function validateCellOutputTranslation( outputs: nbformat.IOutput[], @@ -106,81 +107,6 @@ suite('ipynb serializer', () => { ] ); }); - test('Multi-line Stream output', () => { - validateCellOutputTranslation( - [ - { - name: 'stdout', - output_type: 'stream', - text: [ - 'Epoch 1/5\n', - '...\n', - 'Epoch 2/5\n', - '...\n', - 'Epoch 3/5\n', - '...\n', - 'Epoch 4/5\n', - '...\n', - 'Epoch 5/5\n', - '...\n' - ] - } - ], - [ - new vscode.NotebookCellOutput([vscode.NotebookCellOutputItem.stdout(['Epoch 1/5\n', - '...\n', - 'Epoch 2/5\n', - '...\n', - 'Epoch 3/5\n', - '...\n', - 'Epoch 4/5\n', - '...\n', - 'Epoch 5/5\n', - '...\n'].join(''))], { - outputType: 'stream' - }) - ] - ); - }); - - test('Multi-line Stream output (last empty line should not be saved in ipynb)', () => { - validateCellOutputTranslation( - [ - { - name: 'stderr', - output_type: 'stream', - text: [ - 'Epoch 1/5\n', - '...\n', - 'Epoch 2/5\n', - '...\n', - 'Epoch 3/5\n', - '...\n', - 'Epoch 4/5\n', - '...\n', - 'Epoch 5/5\n', - '...\n' - ] - } - ], - [ - new vscode.NotebookCellOutput([vscode.NotebookCellOutputItem.stderr(['Epoch 1/5\n', - '...\n', - 'Epoch 2/5\n', - '...\n', - 'Epoch 3/5\n', - '...\n', - 'Epoch 4/5\n', - '...\n', - 'Epoch 5/5\n', - '...\n', - // This last empty line should not be saved in ipynb. - '\n'].join(''))], { - outputType: 'stream' - }) - ] - ); - }); test('Streamed text with Ansi characters', async () => { validateCellOutputTranslation( @@ -465,131 +391,4 @@ suite('ipynb serializer', () => { }); }); }); - - suite('Output Order', () => { - test('Verify order of outputs', async () => { - const dataAndExpectedOrder: { output: nbformat.IDisplayData; expectedMimeTypesOrder: string[] }[] = [ - { - output: { - data: { - 'application/vnd.vegalite.v4+json': 'some json', - 'text/html': 'Hello' - }, - metadata: {}, - output_type: 'display_data' - }, - expectedMimeTypesOrder: ['application/vnd.vegalite.v4+json', 'text/html'] - }, - { - output: { - data: { - 'application/vnd.vegalite.v4+json': 'some json', - 'application/javascript': 'some js', - 'text/plain': 'some text', - 'text/html': 'Hello' - }, - metadata: {}, - output_type: 'display_data' - }, - expectedMimeTypesOrder: [ - 'application/vnd.vegalite.v4+json', - 'text/html', - 'application/javascript', - 'text/plain' - ] - }, - { - output: { - data: { - 'application/vnd.vegalite.v4+json': '', // Empty, should give preference to other mimetypes. - 'application/javascript': 'some js', - 'text/plain': 'some text', - 'text/html': 'Hello' - }, - metadata: {}, - output_type: 'display_data' - }, - expectedMimeTypesOrder: [ - 'text/html', - 'application/javascript', - 'text/plain', - 'application/vnd.vegalite.v4+json' - ] - }, - { - output: { - data: { - 'text/plain': 'some text', - 'text/html': 'Hello' - }, - metadata: {}, - output_type: 'display_data' - }, - expectedMimeTypesOrder: ['text/html', 'text/plain'] - }, - { - output: { - data: { - 'application/javascript': 'some js', - 'text/plain': 'some text' - }, - metadata: {}, - output_type: 'display_data' - }, - expectedMimeTypesOrder: ['application/javascript', 'text/plain'] - }, - { - output: { - data: { - 'image/svg+xml': 'some svg', - 'text/plain': 'some text' - }, - metadata: {}, - output_type: 'display_data' - }, - expectedMimeTypesOrder: ['image/svg+xml', 'text/plain'] - }, - { - output: { - data: { - 'text/latex': 'some latex', - 'text/plain': 'some text' - }, - metadata: {}, - output_type: 'display_data' - }, - expectedMimeTypesOrder: ['text/latex', 'text/plain'] - }, - { - output: { - data: { - 'application/vnd.jupyter.widget-view+json': 'some widget', - 'text/plain': 'some text' - }, - metadata: {}, - output_type: 'display_data' - }, - expectedMimeTypesOrder: ['application/vnd.jupyter.widget-view+json', 'text/plain'] - }, - { - output: { - data: { - 'text/plain': 'some text', - 'image/svg+xml': 'some svg', - 'image/png': 'some png' - }, - metadata: {}, - output_type: 'display_data' - }, - expectedMimeTypesOrder: ['image/png', 'image/svg+xml', 'text/plain'] - } - ]; - - dataAndExpectedOrder.forEach(({ output, expectedMimeTypesOrder }) => { - const sortedOutputs = jupyterCellOutputToCellOutput(output); - const mimeTypes = sortedOutputs.items.map((item) => item.mime).join(','); - assert.equal(mimeTypes, expectedMimeTypesOrder.join(',')); - }); - }); - }) });