Fixes to for escaping of output in native notebooks (#14159) (#156)

Author: DonJayamanne

src/client/datascience/jupyter/kernels/cellExecution.ts

@@ -38,8 +38,6 @@ import {
 import { IKernel } from './types';
 // tslint:disable-next-line: no-var-requires no-require-imports
 const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed');
-// tslint:disable-next-line: no-require-imports
-import escape = require('lodash/escape');
 
 export class CellExecutionFactory {
     constructor(
@@ -458,11 +456,6 @@ export class CellExecution {
     // See this for docs on the messages:
     // https://jupyter-client.readthedocs.io/en/latest/messaging.html#messaging-in-jupyter
     private async handleExecuteResult(msg: KernelMessage.IExecuteResultMsg, clearState: RefBool) {
-        // Escape text output
-        if (msg.content.data && msg.content.data.hasOwnProperty('text/plain')) {
-            msg.content.data['text/plain'] = escape(msg.content.data['text/plain'] as string);
-        }
-
         await this.addToCellData(
             {
                 output_type: 'execute_result',
@@ -487,7 +480,7 @@ export class CellExecution {
                                 // Mark as stream output so the text is formatted because it likely has ansi codes in it.
                                 output_type: 'stream',
                                 // tslint:disable-next-line: no-any
-                                text: escape((o.data as any)['text/plain'].toString()),
+                                text: (o.data as any)['text/plain'].toString(),
                                 name: 'stdout',
                                 metadata: {},
                                 execution_count: reply.execution_count
@@ -528,11 +521,11 @@ export class CellExecution {
             if (existing && 'text/plain' in existing.data) {
                 // tslint:disable-next-line:restrict-plus-operands
                 existing.data['text/plain'] = formatStreamText(
-                    concatMultilineString(`${existing.data['text/plain']}${escape(msg.content.text)}`)
+                    concatMultilineString(`${existing.data['text/plain']}${msg.content.text}`)
                 );
                 edit.replaceCellOutput(this.cellIndex, [...exitingCellOutput]); // This is necessary to get VS code to update (for now)
             } else {
-                const originalText = formatStreamText(concatMultilineString(escape(msg.content.text)));
+                const originalText = formatStreamText(concatMultilineString(msg.content.text));
                 // Create a new stream entry
                 const output: nbformat.IStream = {
                     output_type: 'stream',
@@ -545,11 +538,6 @@ export class CellExecution {
     }
 
     private async handleDisplayData(msg: KernelMessage.IDisplayDataMsg, clearState: RefBool) {
-        // Escape text output
-        if (msg.content.data && msg.content.data.hasOwnProperty('text/plain')) {
-            msg.content.data['text/plain'] = escape(msg.content.data['text/plain'] as string);
-        }
-
         const output: nbformat.IDisplayData = {
             output_type: 'display_data',
             data: msg.content.data,

src/test/datascience/notebook/executionService.vscode.test.ts

@@ -343,4 +343,52 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () {
             'Incorrect output'
         );
     });
+    test('Verify escaping of output', async () => {
+        await insertPythonCellAndWait('1');
+        await insertPythonCellAndWait(dedent`
+                                            a="<a href=f>"
+                                            a`);
+        await insertPythonCellAndWait(dedent`
+                                            a="<a href=f>"
+                                            print(a)`);
+        await insertPythonCellAndWait('raise Exception("<whatever>")');
+        const cells = vscodeNotebook.activeNotebookEditor?.document.cells!;
+
+        await executeActiveDocument();
+
+        // Wait till execution count changes and status is error.
+        await waitForCondition(
+            async () => assertHasExecutionCompletedWithErrors(cells[3]),
+            15_000,
+            'Cell did not get executed'
+        );
+
+        for (const cell of cells) {
+            assert.lengthOf(cell.outputs, 1, 'Incorrect output');
+        }
+        assert.equal(
+            cells[0].outputs[0].outputKind,
+            vscodeNotebookEnums.CellOutputKind.Rich,
+            'Incorrect output for first cell'
+        );
+        assert.equal(
+            cells[1].outputs[0].outputKind,
+            vscodeNotebookEnums.CellOutputKind.Rich,
+            'Incorrect output for first cell'
+        );
+        assert.equal(
+            cells[2].outputs[0].outputKind,
+            vscodeNotebookEnums.CellOutputKind.Rich,
+            'Incorrect output for first cell'
+        );
+        assertHasTextOutputInVSCode(cells[0], '1');
+        assertHasTextOutputInVSCode(cells[1], '<a href=f>', 0, false);
+        assertHasTextOutputInVSCode(cells[2], '<a href=f>', 0, false);
+        const errorOutput = cells[3].outputs[0] as CellErrorOutput;
+        assert.equal(errorOutput.outputKind, vscodeNotebookEnums.CellOutputKind.Error, 'Incorrect output');
+        assert.equal(errorOutput.ename, 'Exception', 'Incorrect ename'); // As status contains ename, we don't want this displayed again.
+        assert.equal(errorOutput.evalue, '<whatever>', 'Incorrect evalue'); // As status contains ename, we don't want this displayed again.
+        assert.isNotEmpty(errorOutput.traceback, 'Incorrect traceback');
+        assert.include(errorOutput.traceback.join(''), '<whatever>');
+    });
 });