Skip to content

Commit 44555b7

Browse files
DonJayamanneDonJayamanne
committed
Fixes to for escaping of output
1 parent 218dd8d commit 44555b7

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
},
@@ -540,7 +533,7 @@ export class CellExecution {
540533
existing.text = formatStreamText(concatMultilineString(existing.text + escape(msg.content.text)));
541534
edit.replaceCellOutput(this.cellIndex, [...exitingCellOutput]); // This is necessary to get VS code to update (for now)
542535
} else {
543-
const originalText = formatStreamText(concatMultilineString(escape(msg.content.text)));
536+
const originalText = formatStreamText(concatMultilineString(msg.content.text));
544537
// Create a new stream entry
545538
const output: nbformat.IStream = {
546539
output_type: 'stream',
@@ -553,11 +546,6 @@ export class CellExecution {
553546
}
554547

555548
private async handleDisplayData(msg: KernelMessage.IDisplayDataMsg, clearState: RefBool) {
556-
// Escape text output
557-
if (msg.content.data && msg.content.data.hasOwnProperty('text/plain')) {
558-
msg.content.data['text/plain'] = escape(msg.content.data['text/plain'] as string);
559-
}
560-
561549
const output: nbformat.IDisplayData = {
562550
output_type: 'display_data',
563551
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
@@ -264,4 +264,52 @@ suite('DataScience - VSCode Notebook - (Execution) (slow)', function () {
264264
// Verify that it hasn't got added (even after interrupting).
265265
assertNotHasTextOutputInVSCode(cell, 'Start', 0, false);
266266
});
267+
test('Verify escaping of output', async () => {
268+
await insertPythonCellAndWait('1');
269+
await insertPythonCellAndWait(dedent`
270+
a="<a href=f>"
271+
a`);
272+
await insertPythonCellAndWait(dedent`
273+
a="<a href=f>"
274+
print(a)`);
275+
await insertPythonCellAndWait('raise Exception("<whatever>")');
276+
const cells = vscodeNotebook.activeNotebookEditor?.document.cells!;
277+
278+
await executeActiveDocument();
279+
280+
// Wait till execution count changes and status is error.
281+
await waitForCondition(
282+
async () => assertHasExecutionCompletedWithErrors(cells[3]),
283+
15_000,
284+
'Cell did not get executed'
285+
);
286+
287+
for (const cell of cells) {
288+
assert.lengthOf(cell.outputs, 1, 'Incorrect output');
289+
}
290+
assert.equal(
291+
cells[0].outputs[0].outputKind,
292+
vscodeNotebookEnums.CellOutputKind.Rich,
293+
'Incorrect output for first cell'
294+
);
295+
assert.equal(
296+
cells[1].outputs[0].outputKind,
297+
vscodeNotebookEnums.CellOutputKind.Rich,
298+
'Incorrect output for first cell'
299+
);
300+
assert.equal(
301+
cells[2].outputs[0].outputKind,
302+
vscodeNotebookEnums.CellOutputKind.Rich,
303+
'Incorrect output for first cell'
304+
);
305+
assertHasTextOutputInVSCode(cells[0], '1');
306+
assertHasTextOutputInVSCode(cells[1], '<a href=f>', 0, false);
307+
assertHasTextOutputInVSCode(cells[2], '<a href=f>', 0, false);
308+
const errorOutput = cells[3].outputs[0] as CellErrorOutput;
309+
assert.equal(errorOutput.outputKind, vscodeNotebookEnums.CellOutputKind.Error, 'Incorrect output');
310+
assert.equal(errorOutput.ename, 'Exception', 'Incorrect ename'); // As status contains ename, we don't want this displayed again.
311+
assert.equal(errorOutput.evalue, '<whatever>', 'Incorrect evalue'); // As status contains ename, we don't want this displayed again.
312+
assert.isNotEmpty(errorOutput.traceback, 'Incorrect traceback');
313+
assert.include(errorOutput.traceback.join(''), '<whatever>');
314+
});
267315
});

‎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)