Skip to content

Commit 2eaf364

Browse files
techwithtimtechwithtim
authored
Made exporting notebooks cancellable (microsoft#12863)
* made export cancellable * some small fixes * removed monaco editor import * changed deleting method * made export to temp file first * removed unused action * removed unused types
1 parent 85ee01c commit 2eaf364

12 files changed

Lines changed: 77 additions & 78 deletions

File tree

‎package.nls.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
"DataScience.checkingIfImportIsSupported": "Checking if import is supported",
2424
"DataScience.installingMissingDependencies": "Installing missing dependencies",
2525
"DataScience.exportNotebookToPython": "Exporting Notebook to Python",
26-
"DataScience.performingExport": "Performing export",
26+
"DataScience.performingExport": "Performing Export",
2727
"DataScience.convertingToPDF": "Converting to PDF",
2828
"DataScience.exportHTMLQuickPickLabel": "HTML",
2929
"DataScience.exportPDFQuickPickLabel": "PDF",

‎src/client/common/utils/localize.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,7 @@ export namespace DataScience {
331331
'DataScience.installingMissingDependencies',
332332
'Installing missing dependencies'
333333
);
334-
export const performingExport = localize('DataScience.performingExport', 'Performing export');
334+
export const performingExport = localize('DataScience.performingExport', 'Performing Export');
335335
export const convertingToPDF = localize('DataScience.convertingToPDF', 'Converting to PDF');
336336
export const exportNotebookToPython = localize(
337337
'DataScience.exportNotebookToPython',

‎src/client/datascience/export/exportBase.ts‎

Lines changed: 38 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import { inject, injectable } from 'inversify';
2-
import { Uri } from 'vscode';
2+
import * as path from 'path';
3+
import { CancellationToken, Uri } from 'vscode';
34
import { IFileSystem } from '../../common/platform/types';
45
import { IPythonExecutionFactory, IPythonExecutionService } from '../../common/process/types';
56
import { reportAction } from '../progress/decorator';
67
import { ReportableAction } from '../progress/types';
78
import { IJupyterSubCommandExecutionService, INotebookImporter } from '../types';
8-
import { IExport } from './types';
9+
import { ExportFormat, IExport } from './types';
910

1011
@injectable()
1112
export class ExportBase implements IExport {
@@ -18,37 +19,55 @@ export class ExportBase implements IExport {
1819
) {}
1920

2021
// tslint:disable-next-line: no-empty
21-
public async export(_source: Uri, _target: Uri): Promise<void> {}
22+
public async export(_source: Uri, _target: Uri, _token: CancellationToken): Promise<void> {}
2223

2324
@reportAction(ReportableAction.PerformingExport)
24-
public async executeCommand(source: Uri, target: Uri, args: string[]): Promise<void> {
25+
public async executeCommand(
26+
source: Uri,
27+
target: Uri,
28+
format: ExportFormat,
29+
token: CancellationToken
30+
): Promise<void> {
31+
if (token.isCancellationRequested) {
32+
return;
33+
}
34+
2535
const service = await this.getExecutionService(source);
2636
if (!service) {
2737
return;
2838
}
2939

30-
const oldFileExists = await this.fileSystem.fileExists(target.fsPath);
31-
let oldFileTime;
32-
if (oldFileExists) {
33-
oldFileTime = (await this.fileSystem.stat(target.fsPath)).mtime;
40+
if (token.isCancellationRequested) {
41+
return;
3442
}
3543

44+
const tempTarget = await this.fileSystem.createTemporaryFile(path.extname(target.fsPath));
45+
const args = [
46+
source.fsPath,
47+
'--to',
48+
format,
49+
'--output',
50+
path.basename(tempTarget.filePath),
51+
'--output-dir',
52+
path.dirname(tempTarget.filePath)
53+
];
3654
const result = await service.execModule('jupyter', ['nbconvert'].concat(args), {
3755
throwOnStdErr: false,
38-
encoding: 'utf8'
56+
encoding: 'utf8',
57+
token: token
3958
});
4059

41-
// Need to check if export failed, since throwOnStdErr is not an
42-
// indicator of a failed export.
43-
if (!(await this.fileSystem.fileExists(target.fsPath))) {
60+
if (token.isCancellationRequested) {
61+
tempTarget.dispose();
62+
return;
63+
}
64+
65+
try {
66+
await this.fileSystem.copyFile(tempTarget.filePath, target.fsPath);
67+
} catch {
4468
throw new Error(result.stderr);
45-
} else if (oldFileExists) {
46-
// If we exported to a file that already exists we need to check that
47-
// this file was actually overridden during export
48-
const newFileTime = (await this.fileSystem.stat(target.fsPath)).mtime;
49-
if (newFileTime === oldFileTime) {
50-
throw new Error(result.stderr);
51-
}
69+
} finally {
70+
tempTarget.dispose();
5271
}
5372
}
5473

‎src/client/datascience/export/exportManager.ts‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,27 +51,31 @@ export class ExportManager implements IExportManager {
5151
await this.exportUtil.removeSvgs(source);
5252
}
5353

54-
const reporter = this.progressReporter.createProgressIndicator(`Exporting to ${format}`);
54+
const reporter = this.progressReporter.createProgressIndicator(`Exporting to ${format}`, true);
5555
try {
5656
switch (format) {
5757
case ExportFormat.python:
58-
await this.exportToPython.export(source, target);
58+
await this.exportToPython.export(source, target, reporter.token);
5959
break;
6060

6161
case ExportFormat.pdf:
62-
await this.exportToPDF.export(source, target);
62+
await this.exportToPDF.export(source, target, reporter.token);
6363
break;
6464

6565
case ExportFormat.html:
66-
await this.exportToHTML.export(source, target);
66+
await this.exportToHTML.export(source, target, reporter.token);
6767
break;
6868

6969
default:
7070
break;
7171
}
7272
} finally {
73-
reporter.dispose();
7473
tempDir.dispose();
74+
reporter.dispose();
75+
}
76+
77+
if (reporter.token.isCancellationRequested) {
78+
return;
7579
}
7680

7781
return target;

‎src/client/datascience/export/exportToHTML.ts‎

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,11 @@
11
import { injectable } from 'inversify';
2-
import * as path from 'path';
3-
import { Uri } from 'vscode';
2+
import { CancellationToken, Uri } from 'vscode';
43
import { ExportBase } from './exportBase';
4+
import { ExportFormat } from './types';
55

66
@injectable()
77
export class ExportToHTML extends ExportBase {
8-
public async export(source: Uri, target: Uri): Promise<void> {
9-
const args = [
10-
source.fsPath,
11-
'--to',
12-
'html',
13-
'--output',
14-
path.basename(target.fsPath),
15-
'--output-dir',
16-
path.dirname(target.fsPath)
17-
];
18-
await this.executeCommand(source, target, args);
8+
public async export(source: Uri, target: Uri, token: CancellationToken): Promise<void> {
9+
await this.executeCommand(source, target, ExportFormat.html, token);
1910
}
2011
}

‎src/client/datascience/export/exportToPDF.ts‎

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,11 @@
1-
import { inject, injectable } from 'inversify';
2-
import * as path from 'path';
3-
import { Uri } from 'vscode';
4-
import { IFileSystem } from '../../common/platform/types';
5-
import { IPythonExecutionFactory } from '../../common/process/types';
6-
import { IJupyterSubCommandExecutionService, INotebookImporter } from '../types';
1+
import { injectable } from 'inversify';
2+
import { CancellationToken, Uri } from 'vscode';
73
import { ExportBase } from './exportBase';
4+
import { ExportFormat } from './types';
85

96
@injectable()
107
export class ExportToPDF extends ExportBase {
11-
constructor(
12-
@inject(IPythonExecutionFactory) protected readonly pythonExecutionFactory: IPythonExecutionFactory,
13-
@inject(IJupyterSubCommandExecutionService)
14-
protected jupyterService: IJupyterSubCommandExecutionService,
15-
@inject(IFileSystem) protected readonly fileSystem: IFileSystem,
16-
@inject(INotebookImporter) protected readonly importer: INotebookImporter
17-
) {
18-
super(pythonExecutionFactory, jupyterService, fileSystem, importer);
19-
}
20-
21-
public async export(source: Uri, target: Uri): Promise<void> {
22-
const args = [
23-
source.fsPath,
24-
'--to',
25-
'pdf',
26-
'--output',
27-
path.basename(target.fsPath),
28-
'--output-dir',
29-
path.dirname(target.fsPath)
30-
];
31-
await this.executeCommand(source, target, args);
8+
public async export(source: Uri, target: Uri, token: CancellationToken): Promise<void> {
9+
await this.executeCommand(source, target, ExportFormat.pdf, token);
3210
}
3311
}

‎src/client/datascience/export/exportToPython.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
import { injectable } from 'inversify';
2-
import { Uri } from 'vscode';
2+
import { CancellationToken, Uri } from 'vscode';
33
import { ExportBase } from './exportBase';
44

55
@injectable()
66
export class ExportToPython extends ExportBase {
7-
public async export(source: Uri, target: Uri): Promise<void> {
7+
public async export(source: Uri, target: Uri, token: CancellationToken): Promise<void> {
8+
if (token.isCancellationRequested) {
9+
return;
10+
}
811
const contents = await this.importer.importFromFile(source.fsPath);
912
await this.fileSystem.writeFile(target.fsPath, contents);
1013
}

‎src/client/datascience/export/types.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Uri } from 'vscode';
1+
import { CancellationToken, Uri } from 'vscode';
22
import { INotebookModel } from '../types';
33

44
export enum ExportFormat {
@@ -14,7 +14,7 @@ export interface IExportManager {
1414

1515
export const IExport = Symbol('IExport');
1616
export interface IExport {
17-
export(source: Uri, target: Uri): Promise<void>;
17+
export(source: Uri, target: Uri, token: CancellationToken): Promise<void>;
1818
}
1919

2020
export const IExportManagerFilePicker = Symbol('IExportManagerFilePicker');

‎src/client/datascience/progress/progressReporter.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,15 @@ export class ProgressReporter implements IProgressReporter {
3333
* @returns {(IDisposable & { token: CancellationToken })}
3434
* @memberof ProgressReporter
3535
*/
36-
public createProgressIndicator(message: string): IDisposable & { token: CancellationToken } {
36+
public createProgressIndicator(message: string, cancellable = false): IDisposable & { token: CancellationToken } {
3737
const cancellation = new CancellationTokenSource();
3838
const deferred = createDeferred();
39-
const options = { location: ProgressLocation.Notification, cancellable: false, title: message };
39+
const options = { location: ProgressLocation.Notification, cancellable: cancellable, title: message };
4040

4141
this.appShell
4242
.withProgress(options, async (progress, cancelToken) => {
4343
cancelToken.onCancellationRequested(() => {
44-
if (!cancelToken.isCancellationRequested) {
44+
if (cancelToken.isCancellationRequested) {
4545
cancellation.cancel();
4646
}
4747
deferred.resolve();

‎src/test/datascience/export/exportManager.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ suite('Data Science - Export Manager', () => {
4040
when(exportUtil.generateTempDir()).thenResolve({ path: 'test', dispose: () => {} });
4141
when(exportUtil.makeFileInDirectory(anything(), anything(), anything())).thenResolve('foo');
4242
when(fileSystem.createTemporaryFile(anything())).thenResolve(instance(tempFile));
43-
when(exportPdf.export(anything(), anything())).thenResolve();
43+
when(exportPdf.export(anything(), anything(), anything())).thenResolve();
4444
when(filePicker.getExportFileLocation(anything(), anything())).thenResolve(Uri.file('foo'));
4545
// tslint:disable-next-line: no-any
4646
when(reporter.createProgressIndicator(anything())).thenReturn(instance(mock<IDisposable>()) as any);

0 commit comments

Comments
 (0)