Skip to content

Commit 4f3d5d7

Browse files
authored
Fix reexecution of cells on second open of an ipynb file (microsoft#7482)
* Fix reexecution of cells on second open of an ipynb file * Fix tooltips on buttons
1 parent 6647615 commit 4f3d5d7

File tree

4 files changed

+67
-54
lines changed

4 files changed

+67
-54
lines changed

news/2 Fixes/7417.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Allow cells to be reexecuted on second open of an ipynb file.

package.nls.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,7 @@
340340
"DataScience.instructionComments": "# To add a new cell, type '#%%'\n# To add a new markdown cell, type '#%% [markdown]'\n",
341341
"DataScience.scrollToCellTitleFormatMessage": "Go to [{0}]",
342342
"DataScience.remoteDebuggerNotSupported": "Debugging while attached to a remote server is not currently supported.",
343-
"DataScience.save" : "Save file",
343+
"DataScience.save" : "Save notebook",
344344
"DataScience.invalidNotebookFileError" : "{0} is not a valid notebook file. Check the file for correct json.",
345345
"DataScience.nativeEditorTitle" : "Notebook Editor",
346346
"DataScience.untitledNotebookFileName" : "Untitled",
@@ -354,13 +354,13 @@
354354
"DataScience.exportAsPythonFileTooltip" : "Convert and save to a python script",
355355
"DataScience.exportAsPythonFileTitle" : "Save as Python File",
356356
"DataScience.runCell" : "Run cell",
357-
"DataScience.deleteCell" : "Move selected cell down",
357+
"DataScience.deleteCell" : "Delete cell",
358358
"DataScience.moveCellUp" : "Move cell up",
359359
"DataScience.moveCellDown" : "Move cell down",
360360
"DataScience.moveSelectedCellUp" : "Move selected cell up",
361361
"DataScience.insertBelow" : "Insert cell below",
362362
"DataScience.insertAbove" : "Insert cell above",
363363
"DataScience.addCell" : "Add cell",
364-
"DataScience.runAll" : "Insert cell",
364+
"DataScience.runAll" : "Run all cells",
365365
"DataScience.convertingToPythonFile" : "Converting ipynb to python file"
366366
}

src/client/datascience/interactive-common/interactiveBase.ts

Lines changed: 52 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -421,7 +421,10 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
421421

422422
protected abstract closeBecauseOfFailure(exc: Error): Promise<void>;
423423

424-
protected clearResult(id: string): void {
424+
protected async clearResult(id: string): Promise<void> {
425+
// This will get called during an execution, so we need to have
426+
// a notebook ready.
427+
await this.ensureServerActive();
425428
if (this.notebook) {
426429
this.notebook.clear(id);
427430
}
@@ -460,18 +463,8 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
460463
};
461464

462465
try {
463-
464466
// Make sure we're loaded first.
465-
try {
466-
traceInfo('Waiting for jupyter server and web panel ...');
467-
await this.startServer();
468-
} catch (exc) {
469-
// We should dispose ourselves if the load fails. Othewise the user
470-
// updates their install and we just fail again because the load promise is the same.
471-
await this.closeBecauseOfFailure(exc);
472-
473-
throw exc;
474-
}
467+
await this.ensureServerActive();
475468

476469
// Then show our webpanel
477470
await this.show();
@@ -661,6 +654,53 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
661654
return result;
662655
}
663656

657+
protected async addSysInfo(reason: SysInfoReason): Promise<void> {
658+
if (!this.addSysInfoPromise || reason !== SysInfoReason.Start) {
659+
traceInfo(`Adding sys info for ${this.id} ${reason}`);
660+
const deferred = createDeferred<boolean>();
661+
this.addSysInfoPromise = deferred;
662+
663+
// Generate a new sys info cell and send it to the web panel.
664+
const sysInfo = await this.generateSysInfoCell(reason);
665+
if (sysInfo) {
666+
this.sendCellsToWebView([sysInfo]);
667+
}
668+
669+
// For anything but start, tell the other sides of a live share session
670+
if (reason !== SysInfoReason.Start && sysInfo) {
671+
this.shareMessage(InteractiveWindowMessages.AddedSysInfo, { type: reason, sysInfoCell: sysInfo, id: this.id });
672+
}
673+
674+
// For a restart, tell our window to reset
675+
if (reason === SysInfoReason.Restart || reason === SysInfoReason.New) {
676+
this.postMessage(InteractiveWindowMessages.RestartKernel).ignoreErrors();
677+
if (this.notebook) {
678+
this.jupyterDebugger.onRestart(this.notebook);
679+
}
680+
}
681+
682+
traceInfo(`Sys info for ${this.id} ${reason} complete`);
683+
deferred.resolve(true);
684+
} else if (this.addSysInfoPromise) {
685+
traceInfo(`Wait for sys info for ${this.id} ${reason}`);
686+
await this.addSysInfoPromise.promise;
687+
}
688+
}
689+
690+
private async ensureServerActive(): Promise<void> {
691+
// Make sure we're loaded first.
692+
try {
693+
traceInfo('Waiting for jupyter server and web panel ...');
694+
await this.startServer();
695+
} catch (exc) {
696+
// We should dispose ourselves if the load fails. Othewise the user
697+
// updates their install and we just fail again because the load promise is the same.
698+
await this.closeBecauseOfFailure(exc);
699+
700+
throw exc;
701+
}
702+
}
703+
664704
private async startServerImpl(): Promise<void> {
665705
// Status depends upon if we're about to connect to existing server or not.
666706
const status = (await this.jupyterExecution.getServer(await this.getNotebookOptions())) ?
@@ -1077,39 +1117,6 @@ export abstract class InteractiveBase extends WebViewHost<IInteractiveWindowMapp
10771117
return `${localize.DataScience.sysInfoURILabel()}${urlString}`;
10781118
}
10791119

1080-
private addSysInfo = async (reason: SysInfoReason): Promise<void> => {
1081-
if (!this.addSysInfoPromise || reason !== SysInfoReason.Start) {
1082-
traceInfo(`Adding sys info for ${this.id} ${reason}`);
1083-
const deferred = createDeferred<boolean>();
1084-
this.addSysInfoPromise = deferred;
1085-
1086-
// Generate a new sys info cell and send it to the web panel.
1087-
const sysInfo = await this.generateSysInfoCell(reason);
1088-
if (sysInfo) {
1089-
this.sendCellsToWebView([sysInfo]);
1090-
}
1091-
1092-
// For anything but start, tell the other sides of a live share session
1093-
if (reason !== SysInfoReason.Start && sysInfo) {
1094-
this.shareMessage(InteractiveWindowMessages.AddedSysInfo, { type: reason, sysInfoCell: sysInfo, id: this.id });
1095-
}
1096-
1097-
// For a restart, tell our window to reset
1098-
if (reason === SysInfoReason.Restart || reason === SysInfoReason.New) {
1099-
this.postMessage(InteractiveWindowMessages.RestartKernel).ignoreErrors();
1100-
if (this.notebook) {
1101-
this.jupyterDebugger.onRestart(this.notebook);
1102-
}
1103-
}
1104-
1105-
traceInfo(`Sys info for ${this.id} ${reason} complete`);
1106-
deferred.resolve(true);
1107-
} else if (this.addSysInfoPromise) {
1108-
traceInfo(`Wait for sys info for ${this.id} ${reason}`);
1109-
await this.addSysInfoPromise.promise;
1110-
}
1111-
}
1112-
11131120
private async checkUsable(): Promise<boolean> {
11141121
let activeInterpreter: PythonInterpreter | undefined;
11151122
try {

src/client/datascience/interactive-ipynb/nativeEditor.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ import {
4040
INativeCommand,
4141
InteractiveWindowMessages,
4242
ISaveAll,
43-
ISubmitNewCell
43+
ISubmitNewCell,
44+
SysInfoReason
4445
} from '../interactive-common/interactiveWindowTypes';
4546
import {
4647
ICell,
@@ -299,22 +300,21 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
299300

300301
@captureTelemetry(Telemetry.ExecuteNativeCell, undefined, false)
301302
// tslint:disable-next-line:no-any
302-
protected reexecuteCell(info: ISubmitNewCell) {
303+
protected async reexecuteCell(info: ISubmitNewCell): Promise<void> {
303304
// If there's any payload, it has the code and the id
304305
if (info && info.code && info.id) {
305306
// Update dirtiness
306307
this.setDirty();
307308

308309
// Clear the result if we've run before
309-
this.clearResult(info.id);
310+
await this.clearResult(info.id);
310311

311312
// Send to ourselves.
312313
this.submitCode(info.code, Identifiers.EmptyFileName, 0, info.id).ignoreErrors();
313314

314315
// Activate the other side, and send as if came from a file
315-
this.ipynbProvider.show(this.file).then(_v => {
316-
this.shareMessage(InteractiveWindowMessages.RemoteReexecuteCode, { code: info.code, file: Identifiers.EmptyFileName, line: 0, id: info.id, originator: this.id, debug: false });
317-
}).ignoreErrors();
316+
await this.ipynbProvider.show(this.file);
317+
this.shareMessage(InteractiveWindowMessages.RemoteReexecuteCode, { code: info.code, file: Identifiers.EmptyFileName, line: 0, id: info.id, originator: this.id, debug: false });
318318
}
319319
}
320320

@@ -365,6 +365,11 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
365365
// Actually don't close, just let the error bubble out
366366
}
367367

368+
protected addSysInfo(_reason: SysInfoReason): Promise<void> {
369+
// No sys info in the native editor
370+
return Promise.resolve();
371+
}
372+
368373
private editCell(request: IEditCell) {
369374
// Apply the changes to the visible cell list. We won't get an update until
370375
// submission otherwise

0 commit comments

Comments
 (0)