diff --git a/news/2 Fixes/8677.md b/news/2 Fixes/8677.md new file mode 100644 index 000000000000..62b81842a239 --- /dev/null +++ b/news/2 Fixes/8677.md @@ -0,0 +1 @@ +Converting to python script no longer working from a notebook. diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index db8c7c327b6b..83a98201383d 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -755,7 +755,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { tempFile = await this.fileSystem.createTemporaryFile('.ipynb'); // Translate the cells into a notebook - await this.fileSystem.writeFile(tempFile.filePath, this.generateNotebookContent(cells), { encoding: 'utf-8' }); + await this.fileSystem.writeFile(tempFile.filePath, await this.generateNotebookContent(cells), { encoding: 'utf-8' }); // Import this file and show it const contents = await this.importer.importFromFile(tempFile.filePath, this.file.fsPath); diff --git a/src/client/datascience/jupyter/jupyterImporter.ts b/src/client/datascience/jupyter/jupyterImporter.ts index c7db56d274bb..2c3b5921fa0b 100644 --- a/src/client/datascience/jupyter/jupyterImporter.ts +++ b/src/client/datascience/jupyter/jupyterImporter.ts @@ -1,14 +1,16 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. 'use strict'; +import '../../common/extensions'; + import { nbformat } from '@jupyterlab/coreutils'; import * as fs from 'fs-extra'; import { inject, injectable } from 'inversify'; import * as os from 'os'; import * as path from 'path'; -import '../../common/extensions'; import { IWorkspaceService } from '../../common/application/types'; +import { traceError } from '../../common/logger'; import { IFileSystem, IPlatformService } from '../../common/platform/types'; import { IConfigurationService, IDisposableRegistry } from '../../common/types'; import * as localize from '../../common/utils/localize'; @@ -138,35 +140,39 @@ export class JupyterImporter implements INotebookImporter { // When importing a file, calculate if we can create a %cd so that the relative paths work private async calculateDirectoryChange(notebookFile: string): Promise { let directoryChange: string | undefined; - // Make sure we don't already have an import/export comment in the file - const contents = await this.fileSystem.readFile(notebookFile); - const haveChangeAlready = contents.includes(CodeSnippits.ChangeDirectoryCommentIdentifier); - - if (!haveChangeAlready) { - const notebookFilePath = path.dirname(notebookFile); - // First see if we have a workspace open, this only works if we have a workspace root to be relative to - if (this.workspaceService.hasWorkspaceFolders) { - const workspacePath = this.workspaceService.workspaceFolders![0].uri.fsPath; - - // Make sure that we have everything that we need here - if (workspacePath && path.isAbsolute(workspacePath) && notebookFilePath && path.isAbsolute(notebookFilePath)) { - directoryChange = path.relative(workspacePath, notebookFilePath); + try { + // Make sure we don't already have an import/export comment in the file + const contents = await this.fileSystem.readFile(notebookFile); + const haveChangeAlready = contents.includes(CodeSnippits.ChangeDirectoryCommentIdentifier); + + if (!haveChangeAlready) { + const notebookFilePath = path.dirname(notebookFile); + // First see if we have a workspace open, this only works if we have a workspace root to be relative to + if (this.workspaceService.hasWorkspaceFolders) { + const workspacePath = this.workspaceService.workspaceFolders![0].uri.fsPath; + + // Make sure that we have everything that we need here + if (workspacePath && path.isAbsolute(workspacePath) && notebookFilePath && path.isAbsolute(notebookFilePath)) { + directoryChange = path.relative(workspacePath, notebookFilePath); + } } } - } - // If path.relative can't calculate a relative path, then it just returns the full second path - // so check here, we only want this if we were able to calculate a relative path, no network shares or drives - if (directoryChange && !path.isAbsolute(directoryChange)) { + // If path.relative can't calculate a relative path, then it just returns the full second path + // so check here, we only want this if we were able to calculate a relative path, no network shares or drives + if (directoryChange && !path.isAbsolute(directoryChange)) { - // Escape windows path chars so they end up in the source escaped - if (this.platform.isWindows) { - directoryChange = directoryChange.replace('\\', '\\\\'); - } + // Escape windows path chars so they end up in the source escaped + if (this.platform.isWindows) { + directoryChange = directoryChange.replace('\\', '\\\\'); + } - return directoryChange; - } else { - return undefined; + return directoryChange; + } else { + return undefined; + } + } catch (e) { + traceError(e); } } diff --git a/src/test/datascience/liveshare.functional.test.tsx b/src/test/datascience/liveshare.functional.test.tsx index db32b92f62f6..251b60c5a385 100644 --- a/src/test/datascience/liveshare.functional.test.tsx +++ b/src/test/datascience/liveshare.functional.test.tsx @@ -25,7 +25,7 @@ import { IJupyterExecution } from '../../client/datascience/types'; import { InteractivePanel } from '../../datascience-ui/history-react/interactivePanel'; -import { asyncDump } from '../common/asyncDump'; +//import { asyncDump } from '../common/asyncDump'; import { DataScienceIocContainer } from './dataScienceIocContainer'; import { createDocument } from './editor-integration/helpers'; import { waitForUpdate } from './reactHelpers'; @@ -62,7 +62,7 @@ suite('DataScience LiveShare tests', () => { }); suiteTeardown(() => { - asyncDump(); + //asyncDump(); }); function createContainer(role: vsls.Role): DataScienceIocContainer { diff --git a/src/test/datascience/mockJupyterManager.ts b/src/test/datascience/mockJupyterManager.ts index b7b3181c713a..973a71adbdf4 100644 --- a/src/test/datascience/mockJupyterManager.ts +++ b/src/test/datascience/mockJupyterManager.ts @@ -430,7 +430,7 @@ export class MockJupyterManager implements IJupyterSessionManager { this.setupPythonServiceExec(service, 'jupyter', ['nbconvert', '--version'], () => Promise.resolve({ stdout: '1.1.1.1' })); this.setupPythonServiceExec(service, 'jupyter', ['nbconvert', /.*/, '--to', 'python', '--stdout', '--template', /.*/], () => { return Promise.resolve({ - stdout: '#%%\r\nimport os\r\nos.chdir()' + stdout: '#%%\r\nimport os\r\nos.chdir()\r\n#%%\r\na=1' }); }); } diff --git a/src/test/datascience/nativeEditor.functional.test.tsx b/src/test/datascience/nativeEditor.functional.test.tsx index dc3b95b2a50e..a26e2435100d 100644 --- a/src/test/datascience/nativeEditor.functional.test.tsx +++ b/src/test/datascience/nativeEditor.functional.test.tsx @@ -226,17 +226,17 @@ for _ in range(50): assert.equal(afterDelete.length, 1, `Delete should NOT remove the last cell`); }, () => { return ioc; }); - runMountedTest('Export', async (wrapper) => { + runMountedTest('Convert to python', async (wrapper) => { // Export should cause the export dialog to come up. Remap appshell so we can check const dummyDisposable = { dispose: () => { return; } }; - let exportCalled = false; + let saveCalled = false; const appShell = TypeMoq.Mock.ofType(); appShell.setup(a => a.showErrorMessage(TypeMoq.It.isAnyString())).returns((e) => { throw e; }); appShell.setup(a => a.showInformationMessage(TypeMoq.It.isAny(), TypeMoq.It.isAny())).returns(() => Promise.resolve('')); appShell.setup(a => a.showSaveDialog(TypeMoq.It.isAny())).returns(() => { - exportCalled = true; + saveCalled = true; return Promise.resolve(undefined); }); appShell.setup(a => a.setStatusBarMessage(TypeMoq.It.isAny())).returns(() => dummyDisposable); @@ -247,9 +247,22 @@ for _ in range(50): await addCell(wrapper, ioc, 'a=1\na'); // Export should cause exportCalled to change to true - const exportButton = findButton(wrapper, NativeEditor, 8); + const saveButton = findButton(wrapper, NativeEditor, 8); + await waitForMessageResponse(ioc, () => saveButton!.simulate('click')); + assert.equal(saveCalled, true, 'Save should have been called'); + + // Click export and wait for a document to change + const documentChange = createDeferred(); + const docManager = ioc.get(IDocumentManager) as MockDocumentManager; + docManager.onDidChangeTextDocument(() => documentChange.resolve()); + const exportButton = findButton(wrapper, NativeEditor, 9); await waitForMessageResponse(ioc, () => exportButton!.simulate('click')); - assert.equal(exportCalled, true, 'Export should have been called'); + await waitForPromise(documentChange.promise, 3000); + // Verify the new document is valid python + const newDoc = docManager.activeTextEditor; + assert.ok(newDoc, 'New doc not created'); + assert.ok(newDoc!.document.getText().includes('a=1'), 'Export did not create a python file'); + }, () => { return ioc; }); runMountedTest('RunAllCells', async (wrapper) => {