Fix multiline and shell magic commands (microsoft#4143)

IanMatthewHuff · rchiodo · commit 5f8afe80b2cb · 2019-01-24T16:16:26.000-08:00
* tests and hygiene

* new entry

* remove comment

* strip comments before sending code to jupyter

* add news
diff --git a/news/1 Enhancements/4064.md b/news/1 Enhancements/4064.md
@@ -0,0 +1 @@
+Strip comments before sending so shell command and multiline jupyter magics work correctly.
diff --git a/src/client/datascience/common.ts b/src/client/datascience/common.ts
@@ -20,6 +20,21 @@ export function concatMultilineString(str : nbformat.MultilineString) : string {
     return str.toString().trim();
 }
 
+// Strip out comment lines from code
+export function stripComments(str : nbformat.MultilineString): nbformat.MultilineString {
+    if (Array.isArray(str)) {
+        return str.filter((value: string) => {
+            if (value.trim().startsWith('#')) {
+                return false;
+            }
+
+            return true;
+        });
+    } else {
+        return str;
+    }
+}
+
 export function formatStreamText(str: string) : string {
     // Go through the string, looking for \r's that are not followed by \n. This is
     // a special case that means replace the string before. This is necessary to
diff --git a/src/client/datascience/jupyter/jupyterServer.ts b/src/client/datascience/jupyter/jupyterServer.ts
@@ -20,7 +20,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 { concatMultilineString } from '../common';
+import { concatMultilineString, stripComments } from '../common';
 import {
     CellState,
     ICell,
@@ -480,7 +480,7 @@ export class JupyterServer implements INotebookServer, IAsyncDisposable {
         // Generate a new request if we still can
         if (subscriber.isValid(this.sessionStartTime)) {
 
-            const request = this.generateRequest(concatMultilineString(subscriber.cell.data.source), false);
+            const request = this.generateRequest(concatMultilineString(stripComments(subscriber.cell.data.source)), false);
 
             // tslint:disable-next-line:no-require-imports
             const jupyterLab = require('@jupyterlab/services') as typeof import('@jupyterlab/services');
diff --git a/src/test/datascience/mockJupyterManager.ts b/src/test/datascience/mockJupyterManager.ts
@@ -16,7 +16,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 { concatMultilineString } from '../../client/datascience/common';
+import { concatMultilineString, stripComments } from '../../client/datascience/common';
 import {
     ICell,
     IConnection,
@@ -183,7 +183,7 @@ 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(code, 'foo.py', 1, true);
         cells.forEach(c => {
-            const key = concatMultilineString(c.data.source).replace(LineFeedRegEx, '');
+            const key = concatMultilineString(stripComments(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
@@ -654,6 +654,26 @@ df.head()`,
                 result: `<td>A table</td>`,
                 verifyValue: (d) => assert.ok(d.toString().includes('</td>'), 'Table not found')
             },
+            {
+                // Important to test as multiline cell magics only work if they are the first item in the cell
+                code:
+                    `#%% Cell Comment
+%%bash
+echo 'hello'`,
+                mimeType: 'text/plain',
+                cellType: 'code',
+                result: 'hello',
+                verifyValue: (d) => assert.equal(d, 'hello', 'Multiline cell magic incorrect')
+            },
+            {
+                // Test shell command should work on PC / Mac / Linux
+                code:
+                    `!echo world`,
+                mimeType: 'text/plain',
+                cellType: 'code',
+                result: 'world',
+                verifyValue: (d) => assert.equal(d, 'world', 'Cell command incorrect')
+            },
             {
                 // Plotly
                 code:

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Strip comments before sending so shell command and multiline jupyter magics work correctly.`