diff --git a/package.nls.json b/package.nls.json index b8941cffecd4..42737e8a84d1 100644 --- a/package.nls.json +++ b/package.nls.json @@ -23,7 +23,7 @@ "DataScience.checkingIfImportIsSupported": "Checking if import is supported", "DataScience.installingMissingDependencies": "Installing missing dependencies", "DataScience.exportNotebookToPython": "Exporting Notebook to Python", - "DataScience.performingExport": "Performing export", + "DataScience.performingExport": "Performing Export", "DataScience.convertingToPDF": "Converting to PDF", "DataScience.exportHTMLQuickPickLabel": "HTML", "DataScience.exportPDFQuickPickLabel": "PDF", diff --git a/src/client/common/utils/localize.ts b/src/client/common/utils/localize.ts index 0f7629b79ed6..c6ce5bb19a1f 100644 --- a/src/client/common/utils/localize.ts +++ b/src/client/common/utils/localize.ts @@ -331,7 +331,7 @@ export namespace DataScience { 'DataScience.installingMissingDependencies', 'Installing missing dependencies' ); - export const performingExport = localize('DataScience.performingExport', 'Performing export'); + export const performingExport = localize('DataScience.performingExport', 'Performing Export'); export const convertingToPDF = localize('DataScience.convertingToPDF', 'Converting to PDF'); export const exportNotebookToPython = localize( 'DataScience.exportNotebookToPython', diff --git a/src/client/datascience/export/exportBase.ts b/src/client/datascience/export/exportBase.ts index a5ed31449b51..2614433f68e6 100644 --- a/src/client/datascience/export/exportBase.ts +++ b/src/client/datascience/export/exportBase.ts @@ -1,11 +1,12 @@ import { inject, injectable } from 'inversify'; -import { Uri } from 'vscode'; +import * as path from 'path'; +import { CancellationToken, Uri } from 'vscode'; import { IFileSystem } from '../../common/platform/types'; import { IPythonExecutionFactory, IPythonExecutionService } from '../../common/process/types'; import { reportAction } from '../progress/decorator'; import { ReportableAction } from '../progress/types'; import { IJupyterSubCommandExecutionService, INotebookImporter } from '../types'; -import { IExport } from './types'; +import { ExportFormat, IExport } from './types'; @injectable() export class ExportBase implements IExport { @@ -18,37 +19,55 @@ export class ExportBase implements IExport { ) {} // tslint:disable-next-line: no-empty - public async export(_source: Uri, _target: Uri): Promise {} + public async export(_source: Uri, _target: Uri, _token: CancellationToken): Promise {} @reportAction(ReportableAction.PerformingExport) - public async executeCommand(source: Uri, target: Uri, args: string[]): Promise { + public async executeCommand( + source: Uri, + target: Uri, + format: ExportFormat, + token: CancellationToken + ): Promise { + if (token.isCancellationRequested) { + return; + } + const service = await this.getExecutionService(source); if (!service) { return; } - const oldFileExists = await this.fileSystem.fileExists(target.fsPath); - let oldFileTime; - if (oldFileExists) { - oldFileTime = (await this.fileSystem.stat(target.fsPath)).mtime; + if (token.isCancellationRequested) { + return; } + const tempTarget = await this.fileSystem.createTemporaryFile(path.extname(target.fsPath)); + const args = [ + source.fsPath, + '--to', + format, + '--output', + path.basename(tempTarget.filePath), + '--output-dir', + path.dirname(tempTarget.filePath) + ]; const result = await service.execModule('jupyter', ['nbconvert'].concat(args), { throwOnStdErr: false, - encoding: 'utf8' + encoding: 'utf8', + token: token }); - // Need to check if export failed, since throwOnStdErr is not an - // indicator of a failed export. - if (!(await this.fileSystem.fileExists(target.fsPath))) { + if (token.isCancellationRequested) { + tempTarget.dispose(); + return; + } + + try { + await this.fileSystem.copyFile(tempTarget.filePath, target.fsPath); + } catch { throw new Error(result.stderr); - } else if (oldFileExists) { - // If we exported to a file that already exists we need to check that - // this file was actually overridden during export - const newFileTime = (await this.fileSystem.stat(target.fsPath)).mtime; - if (newFileTime === oldFileTime) { - throw new Error(result.stderr); - } + } finally { + tempTarget.dispose(); } } diff --git a/src/client/datascience/export/exportManager.ts b/src/client/datascience/export/exportManager.ts index dcfb47f80183..d58367975728 100644 --- a/src/client/datascience/export/exportManager.ts +++ b/src/client/datascience/export/exportManager.ts @@ -51,27 +51,31 @@ export class ExportManager implements IExportManager { await this.exportUtil.removeSvgs(source); } - const reporter = this.progressReporter.createProgressIndicator(`Exporting to ${format}`); + const reporter = this.progressReporter.createProgressIndicator(`Exporting to ${format}`, true); try { switch (format) { case ExportFormat.python: - await this.exportToPython.export(source, target); + await this.exportToPython.export(source, target, reporter.token); break; case ExportFormat.pdf: - await this.exportToPDF.export(source, target); + await this.exportToPDF.export(source, target, reporter.token); break; case ExportFormat.html: - await this.exportToHTML.export(source, target); + await this.exportToHTML.export(source, target, reporter.token); break; default: break; } } finally { - reporter.dispose(); tempDir.dispose(); + reporter.dispose(); + } + + if (reporter.token.isCancellationRequested) { + return; } return target; diff --git a/src/client/datascience/export/exportToHTML.ts b/src/client/datascience/export/exportToHTML.ts index 005addcc4cfc..d12c92502402 100644 --- a/src/client/datascience/export/exportToHTML.ts +++ b/src/client/datascience/export/exportToHTML.ts @@ -1,20 +1,11 @@ import { injectable } from 'inversify'; -import * as path from 'path'; -import { Uri } from 'vscode'; +import { CancellationToken, Uri } from 'vscode'; import { ExportBase } from './exportBase'; +import { ExportFormat } from './types'; @injectable() export class ExportToHTML extends ExportBase { - public async export(source: Uri, target: Uri): Promise { - const args = [ - source.fsPath, - '--to', - 'html', - '--output', - path.basename(target.fsPath), - '--output-dir', - path.dirname(target.fsPath) - ]; - await this.executeCommand(source, target, args); + public async export(source: Uri, target: Uri, token: CancellationToken): Promise { + await this.executeCommand(source, target, ExportFormat.html, token); } } diff --git a/src/client/datascience/export/exportToPDF.ts b/src/client/datascience/export/exportToPDF.ts index 550e4b1c9e67..0b4b6afba335 100644 --- a/src/client/datascience/export/exportToPDF.ts +++ b/src/client/datascience/export/exportToPDF.ts @@ -1,33 +1,11 @@ -import { inject, injectable } from 'inversify'; -import * as path from 'path'; -import { Uri } from 'vscode'; -import { IFileSystem } from '../../common/platform/types'; -import { IPythonExecutionFactory } from '../../common/process/types'; -import { IJupyterSubCommandExecutionService, INotebookImporter } from '../types'; +import { injectable } from 'inversify'; +import { CancellationToken, Uri } from 'vscode'; import { ExportBase } from './exportBase'; +import { ExportFormat } from './types'; @injectable() export class ExportToPDF extends ExportBase { - constructor( - @inject(IPythonExecutionFactory) protected readonly pythonExecutionFactory: IPythonExecutionFactory, - @inject(IJupyterSubCommandExecutionService) - protected jupyterService: IJupyterSubCommandExecutionService, - @inject(IFileSystem) protected readonly fileSystem: IFileSystem, - @inject(INotebookImporter) protected readonly importer: INotebookImporter - ) { - super(pythonExecutionFactory, jupyterService, fileSystem, importer); - } - - public async export(source: Uri, target: Uri): Promise { - const args = [ - source.fsPath, - '--to', - 'pdf', - '--output', - path.basename(target.fsPath), - '--output-dir', - path.dirname(target.fsPath) - ]; - await this.executeCommand(source, target, args); + public async export(source: Uri, target: Uri, token: CancellationToken): Promise { + await this.executeCommand(source, target, ExportFormat.pdf, token); } } diff --git a/src/client/datascience/export/exportToPython.ts b/src/client/datascience/export/exportToPython.ts index 607026982710..58e010ead4f9 100644 --- a/src/client/datascience/export/exportToPython.ts +++ b/src/client/datascience/export/exportToPython.ts @@ -1,10 +1,13 @@ import { injectable } from 'inversify'; -import { Uri } from 'vscode'; +import { CancellationToken, Uri } from 'vscode'; import { ExportBase } from './exportBase'; @injectable() export class ExportToPython extends ExportBase { - public async export(source: Uri, target: Uri): Promise { + public async export(source: Uri, target: Uri, token: CancellationToken): Promise { + if (token.isCancellationRequested) { + return; + } const contents = await this.importer.importFromFile(source.fsPath); await this.fileSystem.writeFile(target.fsPath, contents); } diff --git a/src/client/datascience/export/types.ts b/src/client/datascience/export/types.ts index cb632e57ebc1..07ff4eee7f61 100644 --- a/src/client/datascience/export/types.ts +++ b/src/client/datascience/export/types.ts @@ -1,4 +1,4 @@ -import { Uri } from 'vscode'; +import { CancellationToken, Uri } from 'vscode'; import { INotebookModel } from '../types'; export enum ExportFormat { @@ -14,7 +14,7 @@ export interface IExportManager { export const IExport = Symbol('IExport'); export interface IExport { - export(source: Uri, target: Uri): Promise; + export(source: Uri, target: Uri, token: CancellationToken): Promise; } export const IExportManagerFilePicker = Symbol('IExportManagerFilePicker'); diff --git a/src/client/datascience/progress/progressReporter.ts b/src/client/datascience/progress/progressReporter.ts index 45098c9a0ff8..87ccadf9f450 100644 --- a/src/client/datascience/progress/progressReporter.ts +++ b/src/client/datascience/progress/progressReporter.ts @@ -33,15 +33,15 @@ export class ProgressReporter implements IProgressReporter { * @returns {(IDisposable & { token: CancellationToken })} * @memberof ProgressReporter */ - public createProgressIndicator(message: string): IDisposable & { token: CancellationToken } { + public createProgressIndicator(message: string, cancellable = false): IDisposable & { token: CancellationToken } { const cancellation = new CancellationTokenSource(); const deferred = createDeferred(); - const options = { location: ProgressLocation.Notification, cancellable: false, title: message }; + const options = { location: ProgressLocation.Notification, cancellable: cancellable, title: message }; this.appShell .withProgress(options, async (progress, cancelToken) => { cancelToken.onCancellationRequested(() => { - if (!cancelToken.isCancellationRequested) { + if (cancelToken.isCancellationRequested) { cancellation.cancel(); } deferred.resolve(); diff --git a/src/test/datascience/export/exportManager.test.ts b/src/test/datascience/export/exportManager.test.ts index 359fa59d6ba5..189b823e973d 100644 --- a/src/test/datascience/export/exportManager.test.ts +++ b/src/test/datascience/export/exportManager.test.ts @@ -40,7 +40,7 @@ suite('Data Science - Export Manager', () => { when(exportUtil.generateTempDir()).thenResolve({ path: 'test', dispose: () => {} }); when(exportUtil.makeFileInDirectory(anything(), anything(), anything())).thenResolve('foo'); when(fileSystem.createTemporaryFile(anything())).thenResolve(instance(tempFile)); - when(exportPdf.export(anything(), anything())).thenResolve(); + when(exportPdf.export(anything(), anything(), anything())).thenResolve(); when(filePicker.getExportFileLocation(anything(), anything())).thenResolve(Uri.file('foo')); // tslint:disable-next-line: no-any when(reporter.createProgressIndicator(anything())).thenReturn(instance(mock()) as any); diff --git a/src/test/datascience/export/exportToHTML.test.ts b/src/test/datascience/export/exportToHTML.test.ts index ec1b0b225543..4412881dea0e 100644 --- a/src/test/datascience/export/exportToHTML.test.ts +++ b/src/test/datascience/export/exportToHTML.test.ts @@ -4,7 +4,7 @@ // tslint:disable: no-var-requires no-require-imports no-invalid-this no-any import { assert } from 'chai'; import * as path from 'path'; -import { Uri } from 'vscode'; +import { CancellationTokenSource, Uri } from 'vscode'; import { IFileSystem } from '../../../client/common/platform/types'; import { ExportFormat, IExport } from '../../../client/datascience/export/types'; import { IExtensionTestApi } from '../../common'; @@ -34,9 +34,11 @@ suite('DataScience - Export HTML', () => { const file = await fileSystem.createTemporaryFile('.html'); const target = Uri.file(file.filePath); await file.dispose(); + const token = new CancellationTokenSource(); await exportToHTML.export( Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'test', 'datascience', 'export', 'test.ipynb')), - target + target, + token.token ); assert.equal(await fileSystem.fileExists(target.fsPath), true); diff --git a/src/test/datascience/export/exportToPython.test.ts b/src/test/datascience/export/exportToPython.test.ts index 0bcf74ab912e..d7d130f141e5 100644 --- a/src/test/datascience/export/exportToPython.test.ts +++ b/src/test/datascience/export/exportToPython.test.ts @@ -4,7 +4,7 @@ // tslint:disable: no-var-requires no-require-imports no-invalid-this no-any import { assert } from 'chai'; import * as path from 'path'; -import { Uri } from 'vscode'; +import { CancellationTokenSource, Uri } from 'vscode'; import { IDocumentManager } from '../../../client/common/application/types'; import { IFileSystem } from '../../../client/common/platform/types'; import { ExportFormat, IExport } from '../../../client/datascience/export/types'; @@ -33,9 +33,11 @@ suite('DataScience - Export Python', () => { const fileSystem = api.serviceContainer.get(IFileSystem); const exportToPython = api.serviceContainer.get(IExport, ExportFormat.python); const target = Uri.file((await fileSystem.createTemporaryFile('.py')).filePath); + const token = new CancellationTokenSource(); await exportToPython.export( Uri.file(path.join(EXTENSION_ROOT_DIR_FOR_TESTS, 'src', 'test', 'datascience', 'export', 'test.ipynb')), - target + target, + token.token ); const documentManager = api.serviceContainer.get(IDocumentManager);