Skip to content

Commit

Permalink
Escaping fix broke a number of things (#14145) (#14154)
Browse files Browse the repository at this point in the history
* Fixes for escaping

* Push a comment ot start PR again

* Cache task is failing

* Remove cache task

* Not fixing so just put back cache task
  • Loading branch information
rchiodo authored Sep 29, 2020
1 parent 5065d31 commit f276e07
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 5 deletions.
6 changes: 5 additions & 1 deletion src/client/datascience/jupyter/jupyterDebugger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
'use strict';
import type { nbformat } from '@jupyterlab/coreutils';
import { inject, injectable, named } from 'inversify';
// tslint:disable-next-line: no-require-imports
import unescape = require('lodash/unescape');
import * as path from 'path';
import * as uuid from 'uuid/v4';
import { DebugConfiguration, Disposable } from 'vscode';
Expand Down Expand Up @@ -473,8 +475,10 @@ export class JupyterDebugger implements IJupyterDebugger, ICellHashListener {
if (outputs.length > 0) {
const data = outputs[0].data;
if (data && data.hasOwnProperty('text/plain')) {
// Plain text should be escaped by our execution engine. Unescape it so
// we can parse it.
// tslint:disable-next-line:no-any
return (data as any)['text/plain'];
return unescape((data as any)['text/plain']);
}
if (outputs[0].output_type === 'stream') {
const stream = outputs[0] as nbformat.IStream;
Expand Down
7 changes: 6 additions & 1 deletion src/datascience-ui/interactive-common/cellOutput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,12 @@ export class CellOutput extends React.Component<ICellOutputProps> {
input = JSON.stringify(output.data);
renderWithScrollbars = true;
isText = true;
} else if (output.output_type === 'execute_result' && input && input.hasOwnProperty('text/plain')) {
} else if (
output.output_type === 'execute_result' &&
input &&
input.hasOwnProperty('text/plain') &&
!input.hasOwnProperty('text/html')
) {
// Plain text should actually be shown as html so that escaped HTML shows up correctly
mimeType = 'text/html';
isText = true;
Expand Down
6 changes: 3 additions & 3 deletions src/test/datascience/notebook.functional.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ suite('DataScience notebook tests', () => {
const error = cell.outputs[0].evalue;
if (error) {
assert.ok(error, 'Error not found when expected');
assert.equal(error, errorString, 'Unexpected error found');
assert.ok(error.toString().includes(errorString), 'Unexpected error found');
}
}

Expand Down Expand Up @@ -757,7 +757,7 @@ suite('DataScience notebook tests', () => {
await server!.waitForIdle(10000);

console.log('Verifying restart');
await verifyError(server, 'a', `name 'a' is not defined`);
await verifyError(server, 'a', `is not defined`);
} catch (exc) {
assert.ok(
exc instanceof JupyterKernelPromiseFailedError,
Expand Down Expand Up @@ -1031,7 +1031,7 @@ a`,
mimeType: 'text/plain',
cellType: 'code',
result: `<a href=f>`,
verifyValue: (d) => assert.equal(d, escape(`<a href=f>`), 'XML not escaped')
verifyValue: (d) => assert.ok(d.includes(escape(`<a href=f>`)), 'XML not escaped')
},
{
markdownRegEx: undefined,
Expand Down

0 comments on commit f276e07

Please sign in to comment.