diff --git a/news/2 Fixes/5537.md b/news/2 Fixes/5537.md new file mode 100644 index 000000000000..8a515c152c40 --- /dev/null +++ b/news/2 Fixes/5537.md @@ -0,0 +1 @@ +Fix magics running from a python file. \ No newline at end of file diff --git a/package-lock.json b/package-lock.json index 1bdda9f42adb..a3ecd5d820e6 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6728,7 +6728,7 @@ }, "event-stream": { "version": "3.3.4", - "resolved": "https://registry.npmjs.org/event-stream/-/event-stream-3.3.4.tgz", + "resolved": "http://registry.npmjs.org/event-stream/-/event-stream-3.3.4.tgz", "integrity": "sha1-SrTJoPWlTbkzi0w02Gv86PSzVXE=", "dev": true, "requires": { @@ -12347,7 +12347,7 @@ "dependencies": { "convert-source-map": { "version": "1.6.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/convert-source-map/-/convert-source-map-1.6.0.tgz", "integrity": "sha512-eFu7XigvxdZ1ETfbgPBohgyQ/Z++C0eEhTor0qRwBw9unw+L0/6V8wkSuGgzdThkiS5lSpdptOQPD8Ak40a+7A==", "dev": true, "requires": { @@ -12356,7 +12356,7 @@ }, "execa": { "version": "1.0.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/execa/-/execa-1.0.0.tgz", "integrity": "sha512-adbxcyWV46qiHyvSp50TKt05tB4tK3HcmF7/nxfAdhnox83seTDbwnaqKO4sXRy7roHAIFqJP/Rw/AuEbX61LA==", "dev": true, "requires": { @@ -12382,7 +12382,7 @@ }, "find-up": { "version": "3.0.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/find-up/-/find-up-3.0.0.tgz", "integrity": "sha512-1yD6RmLI1XBfxugvORwlck6f75tYL+iR0jqwsOrOxMZyGYqUuDhJ0l4AXdO1iX/FTs9cBAMEk1gWSEx1kSbylg==", "dev": true, "requires": { @@ -12397,7 +12397,7 @@ }, "get-stream": { "version": "4.1.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-4.1.0.tgz", "integrity": "sha512-GMat4EJ5161kIy2HevLlr4luNjBgvmj413KaQA7jt4V8B4RDsfpHk7WQ9GVqfYyyx8OS/L66Kox+rJRNklLK7w==", "dev": true, "requires": { @@ -12406,7 +12406,7 @@ }, "glob": { "version": "7.1.3", - "resolved": false, + "resolved": "https://registry.npmjs.org/glob/-/glob-7.1.3.tgz", "integrity": "sha512-vcfuiIxogLV4DlGBHIUOwI0IbrJ8HWPc4MU7HzviGeNho/UJDfi6B5p3sHeWIQ0KGIU0Jpxi5ZHxemQfLkkAwQ==", "dev": true, "requires": { @@ -12420,7 +12420,7 @@ }, "locate-path": { "version": "3.0.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/locate-path/-/locate-path-3.0.0.tgz", "integrity": "sha512-7AO748wWnIhNqAuaty2ZWHkQHRSNfPVIsPIfwEOWO22AmaoVrWavlOcMR5nzTLNYvp36X220/maaRsrec1G65A==", "dev": true, "requires": { @@ -12440,7 +12440,7 @@ }, "os-locale": { "version": "3.1.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/os-locale/-/os-locale-3.1.0.tgz", "integrity": "sha512-Z8l3R4wYWM40/52Z+S265okfFj8Kt2cC2MKY+xNi3kFs+XGI7WXu/I309QQQYbRW4ijiZ+yxs9pqEhJh0DqW3Q==", "dev": true, "requires": { @@ -12451,7 +12451,7 @@ }, "path-exists": { "version": "3.0.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/path-exists/-/path-exists-3.0.0.tgz", "integrity": "sha1-zg6+ql94yxiSXqfYENe1mwEP1RU=", "dev": true }, @@ -12463,7 +12463,7 @@ }, "pkg-dir": { "version": "3.0.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/pkg-dir/-/pkg-dir-3.0.0.tgz", "integrity": "sha512-/E57AYkoeQ25qkxMj5PBOVgF8Kiu/h7cYS30Z5+R7WaiCCBfLq58ZI/dSeaEKb9WVJV5n/03QwrN3IeWIFllvw==", "dev": true, "requires": { @@ -12472,7 +12472,7 @@ }, "pump": { "version": "3.0.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/pump/-/pump-3.0.0.tgz", "integrity": "sha512-LwZy+p3SFs1Pytd/jYct4wpv49HiYCqd9Rlc5ZVdk0V+8Yzv6jR5Blk3TRmPL1ft69TxP0IMZGJ+WPFU2BFhww==", "dev": true, "requires": { @@ -12488,13 +12488,13 @@ }, "resolve-from": { "version": "4.0.0", - "resolved": false, + "resolved": "https://registry.npmjs.org/resolve-from/-/resolve-from-4.0.0.tgz", "integrity": "sha512-pb/MYmXstAkysRFx8piNI1tGFNQIFA3vkE3Gq4EuA1dF6gHp/+vgZqsCGJapvy8N3Q+4o7FwvquPJcnZ7RYy4g==", "dev": true }, "rimraf": { "version": "2.6.3", - "resolved": false, + "resolved": "https://registry.npmjs.org/rimraf/-/rimraf-2.6.3.tgz", "integrity": "sha512-mwqeW5XsA2qAejG46gYdENaxXjx9onRNCfn7L0duuP4hCuTIi/QO7PDK07KJfp1d+izWPrzEJDcSqBa0OZQriA==", "dev": true, "requires": { @@ -13681,7 +13681,7 @@ }, "queue": { "version": "3.1.0", - "resolved": "https://registry.npmjs.org/queue/-/queue-3.1.0.tgz", + "resolved": "http://registry.npmjs.org/queue/-/queue-3.1.0.tgz", "integrity": "sha1-bEnQHwCeIlZ4h4nyv/rGuLmZBYU=", "dev": true, "requires": { diff --git a/src/client/datascience/cellMatcher.ts b/src/client/datascience/cellMatcher.ts index 7892a662269c..fdf8999f140e 100644 --- a/src/client/datascience/cellMatcher.ts +++ b/src/client/datascience/cellMatcher.ts @@ -32,6 +32,11 @@ export class CellMatcher { return this.codeMatchRegEx.test(code); } + public stripMarkers(code: string) : string { + const lines = code.splitLines({trim: false, removeEmptyEntries: false}); + return lines.filter(l => !this.isCode(l) && !this.isMarkdown(l)).join('\n'); + } + public exec(code: string) : string | undefined { let result: RegExpExecArray | null = null; if (this.codeMatchRegEx.test(code)) { diff --git a/src/client/datascience/common.ts b/src/client/datascience/common.ts index cd19b5f13ff7..b60024563045 100644 --- a/src/client/datascience/common.ts +++ b/src/client/datascience/common.ts @@ -24,12 +24,13 @@ export function concatMultilineString(str: nbformat.MultilineString): string { } // Strip out comment lines from code -export function stripComments(str: nbformat.MultilineString): nbformat.MultilineString { - if (Array.isArray(str)) { - return extractNonComments(str); - } else { - return extractNonComments([str]); - } +export function stripComments(str: string): string { + let result: string = ''; + parseForComments( + str.splitLines({trim: false, removeEmptyEntries: false}), + (_s) => noop, + (s) => result = result.concat(`${s}\n`)); + return result; } export function formatStreamText(str: string): string { @@ -77,6 +78,7 @@ export function generateMarkdownFromCodeLines(lines: string[]) { return appendLineFeed(extractComments(lines.slice(1))); } +// tslint:disable-next-line: cyclomatic-complexity export function parseForComments( lines: string[], foundCommentLine: (s: string, i: number) => void, @@ -109,15 +111,20 @@ export function parseForComments( } // Not inside either, see if starting a quote } else if (isMultilineQuote && !isMultilineComment) { - insideMultilineQuote = isMultilineQuote; + // Make sure doesn't begin and end on the same line. + const beginQuote = trim.indexOf(isMultilineQuote); + const endQuote = trim.lastIndexOf(isMultilineQuote); + insideMultilineQuote = endQuote !== beginQuote ? undefined : isMultilineQuote; foundNonCommentLine(l, pos); // Not starting a quote, might be starting a comment } else if (isMultilineComment) { - insideMultilineComment = isMultilineComment; + // See if this line ends the comment too or not + const endIndex = trim.indexOf(isMultilineComment, 3); + insideMultilineComment = endIndex >= 0 ? undefined : isMultilineComment; // Might end with text too if (trim.length > 3) { - foundCommentLine(trim.slice(3), pos); + foundCommentLine(trim.slice(3, endIndex >= 0 ? endIndex : undefined), pos); } } else { // Normal line @@ -136,9 +143,3 @@ function extractComments(lines: string[]): string[] { parseForComments(lines, (s) => result.push(s), (_s) => noop()); return result; } - -function extractNonComments(lines: string[]): string[] { - const result: string[] = []; - parseForComments(lines, (_s) => noop, (s) => result.push(s)); - return result; -} diff --git a/src/client/datascience/jupyter/jupyterServer.ts b/src/client/datascience/jupyter/jupyterServer.ts index 2df5c5f16a87..f08f81b872b6 100644 --- a/src/client/datascience/jupyter/jupyterServer.ts +++ b/src/client/datascience/jupyter/jupyterServer.ts @@ -21,6 +21,7 @@ import { createDeferred, Deferred, sleep } from '../../common/utils/async'; import * as localize from '../../common/utils/localize'; import { noop } from '../../common/utils/misc'; import { generateCells } from '../cellFactory'; +import { CellMatcher } from '../cellMatcher'; import { concatMultilineString } from '../common'; import { Identifiers } from '../constants'; import { @@ -514,10 +515,11 @@ export class JupyterServerBase implements INotebookServer { private generateRequest = (code: string, silent?: boolean): Kernel.IFuture | undefined => { //this.logger.logInformation(`Executing code in jupyter : ${code}`) try { + const cellMatcher = new CellMatcher(this.configService.getSettings().datascience); return this.session ? this.session.requestExecute( { - // Replace windows line endings with unix line endings. - code: code.replace(/\r\n/g, '\n'), + // Remove the cell marker if we have one. + code: cellMatcher.stripMarkers(code), stop_on_error: false, allow_stdin: false, store_history: !silent // Silent actually means don't output anything. Store_history is what affects execution_count diff --git a/src/test/datascience/datascience.unit.test.ts b/src/test/datascience/datascience.unit.test.ts index a34457e37e00..07bfb0e849cf 100644 --- a/src/test/datascience/datascience.unit.test.ts +++ b/src/test/datascience/datascience.unit.test.ts @@ -4,7 +4,7 @@ import { assert } from 'chai'; import { generateCells } from '../../client/datascience/cellFactory'; -import { formatStreamText } from '../../client/datascience/common'; +import { formatStreamText, stripComments } from '../../client/datascience/common'; import { InputHistory } from '../../datascience-ui/history-react/inputHistory'; // tslint:disable: max-func-body-length @@ -219,5 +219,14 @@ class Pizza(object): assert.equal(cells[1].data.cell_type, 'code', 'code cell not generated'); assert.equal(cells[1].data.source.length, 7, 'Lines for code not emitted'); assert.equal(cells[1].data.source[3], ' self.toppings = toppings\n', 'Lines for cell not emitted'); + + // Non comments tests + let nonComments = stripComments(multilineCode); + assert.ok(nonComments.startsWith('myvar = """ # Lorem Ipsum'), 'Variable set to multiline string not working'); + nonComments = stripComments(multilineTwo); + assert.equal(nonComments, '', 'Multline comment is not being stripped'); + nonComments = stripComments(multilineQuoteInFunc); + assert.equal(nonComments.splitLines().length, 6, 'Splitting quote in func wrong number of lines'); }); + }); diff --git a/src/test/datascience/mockJupyterManager.ts b/src/test/datascience/mockJupyterManager.ts index b089ddd32ef5..014287f9fd4c 100644 --- a/src/test/datascience/mockJupyterManager.ts +++ b/src/test/datascience/mockJupyterManager.ts @@ -17,6 +17,7 @@ import { ExecutionResult, IProcessServiceFactory, IPythonExecutionFactory, Outpu import { IAsyncDisposableRegistry, IConfigurationService } from '../../client/common/types'; import { EXTENSION_ROOT_DIR } from '../../client/constants'; import { generateCells } from '../../client/datascience/cellFactory'; +import { CellMatcher } from '../../client/datascience/cellMatcher'; import { concatMultilineString } from '../../client/datascience/common'; import { Identifiers } from '../../client/datascience/constants'; import { @@ -192,7 +193,8 @@ export class MockJupyterManager implements IJupyterSessionManager { public addCell(code: string, result?: undefined | string | number | nbformat.IUnrecognizedOutput | nbformat.IExecuteResult | nbformat.IDisplayData | nbformat.IStream | nbformat.IError, mimeType?: string) { const cells = generateCells(undefined, code, 'foo.py', 1, true, uuid()); cells.forEach(c => { - const key = concatMultilineString(c.data.source).replace(LineFeedRegEx, ''); + const cellMatcher = new CellMatcher(); + const key = cellMatcher.stripMarkers(concatMultilineString(c.data.source)).replace(LineFeedRegEx, ''); if (c.data.cell_type === 'code') { const massagedResult = this.massageCellResult(result, mimeType); const data: nbformat.ICodeCell = c.data as nbformat.ICodeCell; diff --git a/src/test/datascience/notebook.functional.test.ts b/src/test/datascience/notebook.functional.test.ts index 8e8e7e874bcd..97c3710999b2 100644 --- a/src/test/datascience/notebook.functional.test.ts +++ b/src/test/datascience/notebook.functional.test.ts @@ -661,10 +661,10 @@ df.head()`, }, { // Important to test as multiline cell magics only work if they are the first item in the cell - // Doesn't work with a comment though. markdownRegEx: undefined, code: - `%%bash + `#%% +%%bash echo 'hello'`, mimeType: 'text/plain', cellType: 'code',