Fix magics and comments parsing

news/2 Fixes/5537.md

@@ -0,0 +1 @@
+Fix magics running from a python file. 

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

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;
-}

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

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');
         });
+
 });

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;

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',