From d4f5a63b4f9599c45aabd9c5bef9750b24b175c5 Mon Sep 17 00:00:00 2001
From: Ian Huff <ian.huff@gmail.com>
Date: Fri, 18 Jan 2019 13:54:54 -0800
Subject: [PATCH 1/5] tests and hygiene

---
 src/datascience-ui/history-react/cell.tsx     |  80 +++++++-----
 .../datascience/history.functional.test.tsx   | 121 ++++++++++++------
 src/test/datascience/mockJupyterManager.ts    |   7 +-
 3 files changed, 135 insertions(+), 73 deletions(-)

diff --git a/src/datascience-ui/history-react/cell.tsx b/src/datascience-ui/history-react/cell.tsx
index faa0ee5b6f3a..1ef6fa61a068 100644
--- a/src/datascience-ui/history-react/cell.tsx
+++ b/src/datascience-ui/history-react/cell.tsx
@@ -16,6 +16,7 @@ import { Identifiers } from '../../client/datascience/constants';
 import { CellState, ICell } from '../../client/datascience/types';
 import { noop } from '../../test/core';
 import { getLocString } from '../react-common/locReactSide';
+import { getSettings } from '../react-common/settingsReactSide';
 import { CellButton } from './cellButton';
 import { Code } from './code';
 import { CollapseButton } from './collapseButton';
@@ -97,34 +98,40 @@ export class Cell extends React.Component<ICellProps> {
     private renderNormalCell() {
         const busy = this.props.cellVM.cell.state === CellState.init || this.props.cellVM.cell.state === CellState.executing;
         const hasNoSource = this.props.cellVM.cell.file === Identifiers.EmptyFileName;
-        return (
-            <div className='cell-wrapper'>
-                <MenuBar baseTheme={this.props.baseTheme}>
-                    <CellButton baseTheme={this.props.baseTheme} onClick={this.props.delete} tooltip={this.getDeleteString()}>
-                        <Image baseTheme={this.props.baseTheme} class='cell-button-image' image={ImageName.Cancel}/>
-                    </CellButton>
-                    <CellButton baseTheme={this.props.baseTheme} onClick={this.props.gotoCode} tooltip={this.getGoToCodeString()} hidden={hasNoSource}>
-                        <Image baseTheme={this.props.baseTheme} class='cell-button-image' image={ImageName.GoToSourceCode}/>
-                    </CellButton>
-                </MenuBar>
-                <div className='cell-outer'>
-                    <div className='controls-div'>
-                        <div className='controls-flex'>
-                            <ExecutionCount isBusy={busy} count={this.props.cellVM.cell.data.execution_count.toString()} visible={this.isCodeCell()}/>
-                            <CollapseButton theme={this.props.baseTheme} hidden={this.props.cellVM.inputBlockCollapseNeeded && this.props.cellVM.inputBlockShow}
-                                open={this.props.cellVM.inputBlockOpen} onClick={this.toggleInputBlock}
-                                tooltip={getLocString('DataScience.collapseInputTooltip', 'Collapse input block')}/>
+        const results: JSX.Element[] = this.renderResults();
+        if (!getSettings().showCellInputCode && (!results || results.length === 0)) {
+            // null is a valid JSX.Element return to render nothing. Plain return doesn't work
+            return null;
+        } else {
+            return (
+                <div className='cell-wrapper'>
+                    <MenuBar baseTheme={this.props.baseTheme}>
+                        <CellButton baseTheme={this.props.baseTheme} onClick={this.props.delete} tooltip={this.getDeleteString()}>
+                            <Image baseTheme={this.props.baseTheme} class='cell-button-image' image={ImageName.Cancel}/>
+                        </CellButton>
+                        <CellButton baseTheme={this.props.baseTheme} onClick={this.props.gotoCode} tooltip={this.getGoToCodeString()} hidden={hasNoSource}>
+                            <Image baseTheme={this.props.baseTheme} class='cell-button-image' image={ImageName.GoToSourceCode}/>
+                        </CellButton>
+                    </MenuBar>
+                    <div className='cell-outer'>
+                        <div className='controls-div'>
+                            <div className='controls-flex'>
+                                <ExecutionCount isBusy={busy} count={this.props.cellVM.cell.data.execution_count ? this.props.cellVM.cell.data.execution_count.toString() : '0'} visible={this.isCodeCell()}/>
+                                <CollapseButton theme={this.props.baseTheme} hidden={this.props.cellVM.inputBlockCollapseNeeded && this.props.cellVM.inputBlockShow}
+                                    open={this.props.cellVM.inputBlockOpen} onClick={this.toggleInputBlock}
+                                    tooltip={getLocString('DataScience.collapseInputTooltip', 'Collapse input block')}/>
+                            </div>
                         </div>
-                    </div>
-                    <div className='content-div'>
-                        <div className='cell-result-container'>
-                            {this.renderInputs()}
-                            {this.renderResults()}
+                        <div className='content-div'>
+                            <div className='cell-result-container'>
+                                {this.renderInputs()}
+                                {this.renderResultsDiv(results)}
+                            </div>
                         </div>
                     </div>
                 </div>
-            </div>
-        );
+            );
+        }
     }
 
     private renderInputs = () => {
@@ -136,18 +143,20 @@ export class Cell extends React.Component<ICellProps> {
         }
     }
 
-    private renderResults = () => {
+    private renderResultsDiv = (results: JSX.Element[]) => {
         const outputClassNames = this.isCodeCell() ?
             `cell-output cell-output-${this.props.baseTheme}` :
             '';
 
+        // Then combine them inside a div
+        return <div className={outputClassNames}>{results}</div>;
+    }
+
+    private renderResults = (): JSX.Element[] => {
         // Results depend upon the type of cell
-        const results = this.isCodeCell() ?
+        return this.isCodeCell() ?
             this.renderCodeOutputs() :
             this.renderMarkdown(this.getMarkdownCell());
-
-        // Then combine them inside a div
-        return <div className={outputClassNames}>{results}</div>;
     }
     private renderCodeOutputs = () => {
         if (this.isCodeCell() && this.hasOutput()) {
@@ -155,8 +164,9 @@ export class Cell extends React.Component<ICellProps> {
             return this.getCodeCell().outputs.map((output: nbformat.IOutput, index: number) => {
                 return this.renderOutput(output, index);
             });
-
         }
+
+        return [];
     }
 
     private renderMarkdown = (markdown : nbformat.IMarkdownCell) => {
@@ -164,7 +174,7 @@ export class Cell extends React.Component<ICellProps> {
         const source = concatMultilineString(markdown.source);
         const Transform = transforms['text/markdown'];
 
-        return <Transform data={source}/>;
+        return [<Transform data={source}/>];
     }
 
     private renderWithTransform = (mimetype: string, output : nbformat.IOutput, index : number) => {
@@ -268,8 +278,12 @@ export class Cell extends React.Component<ICellProps> {
             return this.renderWithTransform(mimetype, copy, index);
         }
 
-        const keys = Object.keys(copy.data);
-        mimetype = keys.length > 0 ? keys[0] : 'unknown';
+        if (copy.data) {
+            const keys = Object.keys(copy.data);
+            mimetype = keys.length > 0 ? keys[0] : 'unknown';
+        } else {
+            mimetype = 'unknown';
+        }
         const str : string = this.getUnknownMimeTypeFormatString().format(mimetype);
         return <div key={index}>{str}</div>;
     }
diff --git a/src/test/datascience/history.functional.test.tsx b/src/test/datascience/history.functional.test.tsx
index a37d9340748a..60234d4f21ba 100644
--- a/src/test/datascience/history.functional.test.tsx
+++ b/src/test/datascience/history.functional.test.tsx
@@ -40,13 +40,18 @@ import { DataScienceIocContainer } from './dataScienceIocContainer';
 import { SupportedCommands } from './mockJupyterManager';
 import { waitForUpdate } from './reactHelpers';
 
-enum cellInputState {
+enum CellInputState {
     Hidden,
     Visible,
     Collapsed,
     Expanded
 }
 
+enum CellPosition {
+    First = 'first',
+    Last = 'last'
+}
+
 // tslint:disable-next-line:max-func-body-length no-any
 suite('History output tests', () => {
     const disposables: Disposable[] = [];
@@ -138,12 +143,16 @@ suite('History output tests', () => {
         delete global['ascquireVsCodeApi'];
     });
 
-    function addMockData(code: string, result: string | number, mimeType?: string, cellType?: string) {
+    function addMockData(code: string, result: string | number | undefined, mimeType?: string, cellType?: string) {
         if (ioc.mockJupyter) {
             if (cellType && cellType === 'error') {
-                ioc.mockJupyter.addError(code, result.toString());
+                ioc.mockJupyter.addError(code, result ? result.toString() : '');
             } else {
-                ioc.mockJupyter.addCell(code, result, mimeType);
+                if (result) {
+                    ioc.mockJupyter.addCell(code, result, mimeType);
+                } else {
+                    ioc.mockJupyter.addCell(code);
+                }
             }
         }
     }
@@ -174,23 +183,50 @@ suite('History output tests', () => {
         }).timeout(60000);
     }
 
-    function verifyHtmlOnLastCell(wrapper: ReactWrapper<any, Readonly<{}>, React.Component>, html: string) {
+    function verifyHtmlOnCell(wrapper: ReactWrapper<any, Readonly<{}>, React.Component>, html: string | undefined, cellIndex: number | CellPosition) {
         const foundResult = wrapper.find('Cell');
         assert.ok(foundResult.length >= 1, 'Didn\'t find any cells being rendered');
 
-        // Extract only the first 100 chars from the input string
-        const sliced = html.substr(0, min([html.length, 100]));
+        let targetCell: ReactWrapper;
+        // Get the correct result that we are dealing with
+        if (typeof cellIndex === 'number') {
+            if (cellIndex >= 0 && cellIndex <= (foundResult.length - 1)) {
+                targetCell = foundResult.at(cellIndex);
+            }
+        } else if (typeof cellIndex === 'string') {
+            switch (cellIndex) {
+                case CellPosition.First:
+                    targetCell = foundResult.first();
+                    break;
+
+                case CellPosition.Last:
+                    targetCell = foundResult.last();
+                    break;
+
+                default:
+                    // Fall through, targetCell check will fail out
+                    break;
+            }
+        }
 
-        // There should be some sort of span with 1 in it
-        const lastCell = foundResult.last();
-        assert.ok(lastCell, 'Last call doesn\'t exist');
-        const output = lastCell.find('div.cell-output');
-        assert.ok(output.length > 0, 'No output cell found');
-        const outHtml = output.html();
-        assert.ok(outHtml.includes(sliced), `${outHtml} does not contain ${sliced}`);
+        // ! is ok here to get rid of undefined type check as we want a fail here if we have not initialized targetCell
+        assert.ok(targetCell!, 'Target cell doesn\'t exist');
+
+        // If html is specified, check it
+        if (html) {
+            // Extract only the first 100 chars from the input string
+            const sliced = html.substr(0, min([html.length, 100]));
+            const output = targetCell!.find('div.cell-output');
+            assert.ok(output.length > 0, 'No output cell found');
+            const outHtml = output.html();
+            assert.ok(outHtml.includes(sliced), `${outHtml} does not contain ${sliced}`);
+        } else {
+            // html not specified, look for an empty render
+            assert.ok(targetCell!.isEmptyRender(), 'Target cell is not empty render');
+        }
     }
 
-    function verifyLastCellInputState(wrapper: ReactWrapper<any, Readonly<{}>, React.Component>, state: cellInputState) {
+    function verifyLastCellInputState(wrapper: ReactWrapper<any, Readonly<{}>, React.Component>, state: CellInputState) {
         const foundResult = wrapper.find('Cell');
         assert.ok(foundResult.length >= 1, 'Didn\'t find any cells being rendered');
 
@@ -201,19 +237,19 @@ suite('History output tests', () => {
         const toggleButton = lastCell.find('polygon.collapse-input-svg');
 
         switch (state) {
-            case cellInputState.Hidden:
+            case CellInputState.Hidden:
                 assert.ok(inputBlock.length === 0, 'Cell input not hidden');
                 break;
 
-            case cellInputState.Visible:
+            case CellInputState.Visible:
                 assert.ok(inputBlock.length === 1, 'Cell input not visible');
                 break;
 
-            case cellInputState.Expanded:
+            case CellInputState.Expanded:
                 assert.ok(toggleButton.html().includes('collapse-input-svg-rotate'), 'Cell input toggle not expanded');
                 break;
 
-            case cellInputState.Collapsed:
+            case CellInputState.Collapsed:
                 assert.ok(!toggleButton.html().includes('collapse-input-svg-rotate'), 'Cell input toggle not collapsed');
                 break;
 
@@ -261,7 +297,7 @@ suite('History output tests', () => {
     runMountedTest('Simple text', async (wrapper) => {
         await addCode(wrapper, 'a=1\na');
 
-        verifyHtmlOnLastCell(wrapper, '<span>1</span>');
+        verifyHtmlOnCell(wrapper, '<span>1</span>', CellPosition.Last);
     });
 
     runMountedTest('Hide inputs', async (wrapper) => {
@@ -269,7 +305,14 @@ suite('History output tests', () => {
 
         await addCode(wrapper, 'a=1\na');
 
-        verifyLastCellInputState(wrapper, cellInputState.Hidden);
+        verifyLastCellInputState(wrapper, CellInputState.Hidden);
+
+        // Add a cell without output, this cell should not show up at all
+        addMockData('a=1', undefined, 'text/plain');
+        await addCode(wrapper, 'a=1', 4);
+
+        verifyHtmlOnCell(wrapper, '<span>1</span>', CellPosition.First);
+        verifyHtmlOnCell(wrapper, undefined, CellPosition.Last);
     });
 
     runMountedTest('Show inputs', async (wrapper) => {
@@ -277,52 +320,52 @@ suite('History output tests', () => {
 
         await addCode(wrapper, 'a=1\na');
 
-        verifyLastCellInputState(wrapper, cellInputState.Visible);
-        verifyLastCellInputState(wrapper, cellInputState.Collapsed);
+        verifyLastCellInputState(wrapper, CellInputState.Visible);
+        verifyLastCellInputState(wrapper, CellInputState.Collapsed);
     });
 
     runMountedTest('Expand inputs', async (wrapper) => {
         initialDataScienceSettings({ ...defaultDataScienceSettings(), collapseCellInputCodeByDefault: false });
         await addCode(wrapper, 'a=1\na');
 
-        verifyLastCellInputState(wrapper, cellInputState.Expanded);
+        verifyLastCellInputState(wrapper, CellInputState.Expanded);
     });
 
     runMountedTest('Collapse / expand cell', async (wrapper) => {
         initialDataScienceSettings({ ...defaultDataScienceSettings() });
         await addCode(wrapper, 'a=1\na');
 
-        verifyLastCellInputState(wrapper, cellInputState.Visible);
-        verifyLastCellInputState(wrapper, cellInputState.Collapsed);
+        verifyLastCellInputState(wrapper, CellInputState.Visible);
+        verifyLastCellInputState(wrapper, CellInputState.Collapsed);
 
         toggleCellExpansion(wrapper);
 
-        verifyLastCellInputState(wrapper, cellInputState.Visible);
-        verifyLastCellInputState(wrapper, cellInputState.Expanded);
+        verifyLastCellInputState(wrapper, CellInputState.Visible);
+        verifyLastCellInputState(wrapper, CellInputState.Expanded);
 
         toggleCellExpansion(wrapper);
 
-        verifyLastCellInputState(wrapper, cellInputState.Visible);
-        verifyLastCellInputState(wrapper, cellInputState.Collapsed);
+        verifyLastCellInputState(wrapper, CellInputState.Visible);
+        verifyLastCellInputState(wrapper, CellInputState.Collapsed);
     });
 
     runMountedTest('Hide / show cell', async (wrapper) => {
         initialDataScienceSettings({ ...defaultDataScienceSettings() });
         await addCode(wrapper, 'a=1\na');
 
-        verifyLastCellInputState(wrapper, cellInputState.Visible);
-        verifyLastCellInputState(wrapper, cellInputState.Collapsed);
+        verifyLastCellInputState(wrapper, CellInputState.Visible);
+        verifyLastCellInputState(wrapper, CellInputState.Collapsed);
 
         // Hide the inputs and verify
         updateDataScienceSettings(wrapper, { ...defaultDataScienceSettings(), showCellInputCode: false });
 
-        verifyLastCellInputState(wrapper, cellInputState.Hidden);
+        verifyLastCellInputState(wrapper, CellInputState.Hidden);
 
         // Show the inputs and verify
         updateDataScienceSettings(wrapper, { ...defaultDataScienceSettings(), showCellInputCode: true });
 
-        verifyLastCellInputState(wrapper, cellInputState.Visible);
-        verifyLastCellInputState(wrapper, cellInputState.Collapsed);
+        verifyLastCellInputState(wrapper, CellInputState.Visible);
+        verifyLastCellInputState(wrapper, CellInputState.Collapsed);
     });
 
     // The default base set of data science settings to use
@@ -428,16 +471,16 @@ for _ in range(50):
         });
 
         await addCode(wrapper, badPanda, 4);
-        verifyHtmlOnLastCell(wrapper, `pd has no attribute 'read'`);
+        verifyHtmlOnCell(wrapper, `pd has no attribute 'read'`, CellPosition.Last);
 
         await addCode(wrapper, goodPanda);
-        verifyHtmlOnLastCell(wrapper, `<td>`);
+        verifyHtmlOnCell(wrapper, `<td>`, CellPosition.Last);
 
         await addCode(wrapper, matPlotLib);
-        verifyHtmlOnLastCell(wrapper, matPlotLibResults);
+        verifyHtmlOnCell(wrapper, matPlotLibResults, CellPosition.Last);
 
         await addCode(wrapper, spinningCursor, 4 + (cursors.length * 3));
-        verifyHtmlOnLastCell(wrapper, '<xmp>\\</xmp>');
+        verifyHtmlOnCell(wrapper, '<xmp>\\</xmp>', CellPosition.Last);
     });
 
     runMountedTest('Undo/redo commands', async (wrapper) => {
diff --git a/src/test/datascience/mockJupyterManager.ts b/src/test/datascience/mockJupyterManager.ts
index fe4db14d4325..694e0d788c0c 100644
--- a/src/test/datascience/mockJupyterManager.ts
+++ b/src/test/datascience/mockJupyterManager.ts
@@ -179,7 +179,12 @@ export class MockJupyterManager implements IJupyterSessionManager {
             if (c.data.cell_type === 'code') {
                 const massagedResult = this.massageCellResult(result, mimeType);
                 const data: nbformat.ICodeCell = c.data as nbformat.ICodeCell;
-                data.outputs = [...data.outputs, massagedResult];
+                if (result) {
+                    data.outputs = [...data.outputs, massagedResult];
+                } else {
+                    data.outputs = [];
+                }
+                //    data.outputs = [...data.outputs, massagedResult];
                 c.data = data;
             }
 

From 8e3bd6565cf6e34e0d6d597c3e08c5d93e7e8479 Mon Sep 17 00:00:00 2001
From: Ian Huff <ian.huff@gmail.com>
Date: Fri, 18 Jan 2019 13:56:03 -0800
Subject: [PATCH 2/5] new entry

---
 news/1 Enhancements/3981.md | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 news/1 Enhancements/3981.md

diff --git a/news/1 Enhancements/3981.md b/news/1 Enhancements/3981.md
new file mode 100644
index 000000000000..e5246be4df1d
--- /dev/null
+++ b/news/1 Enhancements/3981.md	
@@ -0,0 +1 @@
+When cell inputs to Python Interactive are hidden, don't show cells without any output
\ No newline at end of file

From b95e06d71a60bc72ce7cbaab04774cf9d25846a7 Mon Sep 17 00:00:00 2001
From: Ian Huff <ian.huff@gmail.com>
Date: Fri, 18 Jan 2019 14:01:17 -0800
Subject: [PATCH 3/5] remove comment

---
 src/test/datascience/mockJupyterManager.ts | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/test/datascience/mockJupyterManager.ts b/src/test/datascience/mockJupyterManager.ts
index 694e0d788c0c..177b842cebd3 100644
--- a/src/test/datascience/mockJupyterManager.ts
+++ b/src/test/datascience/mockJupyterManager.ts
@@ -182,9 +182,8 @@ export class MockJupyterManager implements IJupyterSessionManager {
                 if (result) {
                     data.outputs = [...data.outputs, massagedResult];
                 } else {
-                    data.outputs = [];
+                    data.outputs = [...data.outputs];
                 }
-                //    data.outputs = [...data.outputs, massagedResult];
                 c.data = data;
             }
 

From dfa122e7b1f8d6e612a14a9da00f74752194333d Mon Sep 17 00:00:00 2001
From: Ian Huff <ian.huff@gmail.com>
Date: Fri, 25 Jan 2019 14:30:52 -0800
Subject: [PATCH 4/5] add directory change

---
 src/client/datascience/historycommandlistener.ts | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/src/client/datascience/historycommandlistener.ts b/src/client/datascience/historycommandlistener.ts
index d7a3c4364851..9772cbe164e4 100644
--- a/src/client/datascience/historycommandlistener.ts
+++ b/src/client/datascience/historycommandlistener.ts
@@ -134,7 +134,13 @@ export class HistoryCommandListener implements IDataScienceCommandListener {
 
                     await this.waitForStatus(async () => {
                         if (uri) {
-                            const notebook = await this.jupyterExporter.translateToNotebook(cells);
+                            let directoryChange;
+                            const settings = this.configuration.getSettings();
+                            if (settings.datascience.changeDirOnImportExport) {
+                                directoryChange = uri.fsPath;
+                            }
+
+                            const notebook = await this.jupyterExporter.translateToNotebook(cells, directoryChange);
                             await this.fileSystem.writeFile(uri.fsPath, JSON.stringify(notebook));
                         }
                     }, localize.DataScience.exportingFormat(), file);
@@ -218,7 +224,12 @@ export class HistoryCommandListener implements IDataScienceCommandListener {
                 })));
 
             // Then save them to the file
-            const notebook = await this.jupyterExporter.translateToNotebook(cells);
+            let directoryChange;
+            if (settings.datascience.changeDirOnImportExport) {
+                directoryChange = file;
+            }
+
+            const notebook = await this.jupyterExporter.translateToNotebook(cells, directoryChange);
             await this.fileSystem.writeFile(file, JSON.stringify(notebook));
 
         } finally {

From 2a058deca871f36bbf460f7098ef447e63191ae5 Mon Sep 17 00:00:00 2001
From: Ian Huff <ian.huff@gmail.com>
Date: Fri, 25 Jan 2019 14:32:28 -0800
Subject: [PATCH 5/5] add news

---
 news/2 Fixes/4140.md | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 news/2 Fixes/4140.md

diff --git a/news/2 Fixes/4140.md b/news/2 Fixes/4140.md
new file mode 100644
index 000000000000..cc07247c8cd7
--- /dev/null
+++ b/news/2 Fixes/4140.md	
@@ -0,0 +1 @@
+Have the new export commands use our directory change code
\ No newline at end of file