Skip to content

Commit 50c3fd3

Browse files
DonJayamanneDonJayamanne
authored
Fixes to for escaping of output in native notebooks (#14159)
1 parent 8e0e622 commit 50c3fd3

3 files changed

Lines changed: 52 additions & 16 deletions

File tree

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

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,6 @@ import {
3838
import { IKernel } from './types';
3939
// tslint:disable-next-line: no-var-requires no-require-imports
4040
const vscodeNotebookEnums = require('vscode') as typeof import('vscode-proposed');
41-
// tslint:disable-next-line: no-require-imports
42-
import escape = require('lodash/escape');
4341

4442
export class CellExecutionFactory {
4543
constructor(
@@ -471,11 +469,6 @@ export class CellExecution {
471469
// See this for docs on the messages:
472470
// https://jupyter-client.readthedocs.io/en/latest/messaging.html#messaging-in-jupyter
473471
private async handleExecuteResult(msg: KernelMessage.IExecuteResultMsg, clearState: RefBool) {
474-
// Escape text output
475-
if (msg.content.data && msg.content.data.hasOwnProperty('text/plain')) {
476-
msg.content.data['text/plain'] = escape(msg.content.data['text/plain'] as string);
477-
}
478-
479472
await this.addToCellData(
480473
{
481474
output_type: 'execute_result',
@@ -500,7 +493,7 @@ export class CellExecution {
500493
// Mark as stream output so the text is formatted because it likely has ansi codes in it.
501494
output_type: 'stream',
502495
// tslint:disable-next-line: no-any
503-
text: escape((o.data as any)['text/plain'].toString()),
496+
text: (o.data as any)['text/plain'].toString(),
504497
metadata: {},
505498
execution_count: reply.execution_count
506499
},
@@ -544,7 +537,7 @@ export class CellExecution {
544537
);
545538
edit.replaceCellOutput(this.cellIndex, [...exitingCellOutput]); // This is necessary to get VS code to update (for now)
546539
} else {
547-
const originalText = formatStreamText(concatMultilineString(escape(msg.content.text)));
540+
const originalText = formatStreamText(concatMultilineString(msg.content.text));
548541
// Create a new stream entry
549542
const output: nbformat.IStream = {
550543
output_type: 'stream',
@@ -557,11 +550,6 @@ export class CellExecution {
557550
}
558551

559552
private async handleDisplayData(msg: KernelMessage.IDisplayDataMsg, clearState: RefBool) {
560-
// Escape text output
561-
if (msg.content.data && msg.content.data.hasOwnProperty('text/plain')) {
562-
msg.content.data['text/plain'] = escape(msg.content.data['text/plain'] as string);
563-
}
564-
565553
const output: nbformat.IDisplayData = {
566554
output_type: 'display_data',
567555
data: msg.content.data,

‎src/test/datascience/notebook/executionService.ds.test.ts‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -343,4 +343,52 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () {
343343
'Incorrect output'
344344
);
345345
});
346+
test('Verify escaping of output', async () => {
347+
await insertPythonCellAndWait('1');
348+
await insertPythonCellAndWait(dedent`
349+
a="<a href=f>"
350+
a`);
351+
await insertPythonCellAndWait(dedent`
352+
a="<a href=f>"
353+
print(a)`);
354+
await insertPythonCellAndWait('raise Exception("<whatever>")');
355+
const cells = vscodeNotebook.activeNotebookEditor?.document.cells!;
356+
357+
await executeActiveDocument();
358+
359+
// Wait till execution count changes and status is error.
360+
await waitForCondition(
361+
async () => assertHasExecutionCompletedWithErrors(cells[3]),
362+
15_000,
363+
'Cell did not get executed'
364+
);
365+
366+
for (const cell of cells) {
367+
assert.lengthOf(cell.outputs, 1, 'Incorrect output');
368+
}
369+
assert.equal(
370+
cells[0].outputs[0].outputKind,
371+
vscodeNotebookEnums.CellOutputKind.Rich,
372+
'Incorrect output for first cell'
373+
);
374+
assert.equal(
375+
cells[1].outputs[0].outputKind,
376+
vscodeNotebookEnums.CellOutputKind.Rich,
377+
'Incorrect output for first cell'
378+
);
379+
assert.equal(
380+
cells[2].outputs[0].outputKind,
381+
vscodeNotebookEnums.CellOutputKind.Rich,
382+
'Incorrect output for first cell'
383+
);
384+
assertHasTextOutputInVSCode(cells[0], '1');
385+
assertHasTextOutputInVSCode(cells[1], '<a href=f>', 0, false);
386+
assertHasTextOutputInVSCode(cells[2], '<a href=f>', 0, false);
387+
const errorOutput = cells[3].outputs[0] as CellErrorOutput;
388+
assert.equal(errorOutput.outputKind, vscodeNotebookEnums.CellOutputKind.Error, 'Incorrect output');
389+
assert.equal(errorOutput.ename, 'Exception', 'Incorrect ename'); // As status contains ename, we don't want this displayed again.
390+
assert.equal(errorOutput.evalue, '<whatever>', 'Incorrect evalue'); // As status contains ename, we don't want this displayed again.
391+
assert.isNotEmpty(errorOutput.traceback, 'Incorrect traceback');
392+
assert.include(errorOutput.traceback.join(''), '<whatever>');
393+
});
346394
});

‎src/test/datascience/notebook/interrupRestart.ds.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors (slow)',
8181
await closeNotebooksAndCleanUpAfterTests(disposables.concat(suiteDisposables));
8282
});
8383

84-
test('Cancelling token will cancel cell executionxxx', async () => {
84+
test('Cancelling token will cancel cell execution', async () => {
8585
await insertPythonCellAndWait('import time\nfor i in range(10000):\n print(i)\n time.sleep(0.1)', 0);
8686
const cell = vscEditor.document.cells[0];
8787
const appShell = api.serviceContainer.get<IApplicationShell>(IApplicationShell);
@@ -115,7 +115,7 @@ suite('DataScience - VSCode Notebook - Restart/Interrupt/Cancel/Errors (slow)',
115115
assertVSCCellHasErrors(cell);
116116
}
117117
});
118-
test('Restarting kernel will cancel cell execution & we can re-run a cellxxx', async () => {
118+
test('Restarting kernel will cancel cell execution & we can re-run a cell', async () => {
119119
await insertPythonCellAndWait('import time\nfor i in range(10000):\n print(i)\n time.sleep(0.1)', 0);
120120
const cell = vscEditor.document.cells[0];
121121

0 commit comments

Comments
 (0)