From 4a849834de7c81aa0e14f511267664cf343c394b Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Mon, 14 Oct 2024 16:51:03 -0300 Subject: [PATCH 1/4] Improve restarting behavior of the reticulate session. --- .../positron-reticulate/src/extension.ts | 45 ++++++++++++++----- 1 file changed, 35 insertions(+), 10 deletions(-) diff --git a/extensions/positron-reticulate/src/extension.ts b/extensions/positron-reticulate/src/extension.ts index 22b7745323d..f2ce011ec65 100644 --- a/extensions/positron-reticulate/src/extension.ts +++ b/extensions/positron-reticulate/src/extension.ts @@ -484,20 +484,45 @@ class ReticulateRuntimeSession implements positron.LanguageRuntimeSession { // tied to the R session. // We have to send a restart to the R session, and send a reticulate::repl_python() // command to it. - this.pythonSession.shutdown(positron.RuntimeExitReason.Restart); - await this.rSession.restart(); - const rSession = await getRSession(); - rSession.execute( - 'reticulate::repl_python()', - 'start-reticulate', - positron.RuntimeCodeExecutionMode.Interactive, - positron.RuntimeErrorBehavior.Continue + const restart = await positron.window.showSimpleModalDialogPrompt( + 'Restarting reticulate', + 'This is will also restart the parent R session. Are you sure you want to continue?', + 'Yes', + 'No' ); + + if (!restart) { + return; + } + + // The events below will make sure that things occure in the right order: + // 1. shutdown the current reticulate session + // 2. restart the attached R session + // 3. start a new reticulate session. + this.pythonSession.onDidEndSession((sess) => { + this.rSession.restart(); + }); + + const disposeListener = this.rSession.onDidChangeRuntimeState(async (e) => { + if (e === positron.RuntimeState.Ready) { + this.rSession.execute( + 'reticulate::repl_python()', + 'start-reticulate', + positron.RuntimeCodeExecutionMode.Interactive, + positron.RuntimeErrorBehavior.Continue + ); + // This should only happen once, so we dispose of this event as soon + // as we have started reticulate. + disposeListener.dispose(); + } + }); + + await this.shutdown(positron.RuntimeExitReason.SwitchRuntime); return; } - public async shutdown() { - await this.pythonSession.shutdown(positron.RuntimeExitReason.Shutdown); + public async shutdown(exitReason: positron.RuntimeExitReason) { + await this.pythonSession.shutdown(exitReason); // Tell Positron that the kernel has exit. When launching IPykernel from a standalone // process, when the kernel exits, then all of it's threads, specially the IOPub thread // holding the ZeroMQ sockets will cease to exist, and thus Positron identifies that the From 6b9a4646f9edca6b37f7612ed9e14d81f130f754 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 15 Oct 2024 17:36:08 -0300 Subject: [PATCH 2/4] Address code review --- extensions/positron-reticulate/src/extension.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/extensions/positron-reticulate/src/extension.ts b/extensions/positron-reticulate/src/extension.ts index f2ce011ec65..84037614baa 100644 --- a/extensions/positron-reticulate/src/extension.ts +++ b/extensions/positron-reticulate/src/extension.ts @@ -485,14 +485,14 @@ class ReticulateRuntimeSession implements positron.LanguageRuntimeSession { // We have to send a restart to the R session, and send a reticulate::repl_python() // command to it. const restart = await positron.window.showSimpleModalDialogPrompt( - 'Restarting reticulate', - 'This is will also restart the parent R session. Are you sure you want to continue?', - 'Yes', - 'No' + vscode.l10n.t('Restarting reticulate'), + vscode.l10n.t('This is will also restart the parent R session. Are you sure you want to continue?'), + vscode.l10n.t('Yes'), + vscode.l10n.t('No') ); if (!restart) { - return; + throw new Error('Restart cancelled.'); } // The events below will make sure that things occure in the right order: @@ -517,7 +517,7 @@ class ReticulateRuntimeSession implements positron.LanguageRuntimeSession { } }); - await this.shutdown(positron.RuntimeExitReason.SwitchRuntime); + await this.shutdown(positron.RuntimeExitReason.Restart); return; } From aa5667a071af03df28376edc4b193e5f0e3e85e2 Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Tue, 15 Oct 2024 17:36:47 -0300 Subject: [PATCH 3/4] Typo Co-authored-by: Jonathan Signed-off-by: Daniel Falbel --- extensions/positron-reticulate/src/extension.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/positron-reticulate/src/extension.ts b/extensions/positron-reticulate/src/extension.ts index 84037614baa..a0f1829a4d4 100644 --- a/extensions/positron-reticulate/src/extension.ts +++ b/extensions/positron-reticulate/src/extension.ts @@ -495,7 +495,7 @@ class ReticulateRuntimeSession implements positron.LanguageRuntimeSession { throw new Error('Restart cancelled.'); } - // The events below will make sure that things occure in the right order: + // The events below will make sure that things occur in the right order: // 1. shutdown the current reticulate session // 2. restart the attached R session // 3. start a new reticulate session. From eed2263465742d1bd287870831c6484c4752400a Mon Sep 17 00:00:00 2001 From: Daniel Falbel Date: Wed, 16 Oct 2024 10:11:56 -0300 Subject: [PATCH 4/4] Use shutdown instead --- extensions/positron-reticulate/src/extension.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/positron-reticulate/src/extension.ts b/extensions/positron-reticulate/src/extension.ts index a0f1829a4d4..a56c643c72b 100644 --- a/extensions/positron-reticulate/src/extension.ts +++ b/extensions/positron-reticulate/src/extension.ts @@ -517,7 +517,7 @@ class ReticulateRuntimeSession implements positron.LanguageRuntimeSession { } }); - await this.shutdown(positron.RuntimeExitReason.Restart); + await this.shutdown(positron.RuntimeExitReason.Shutdown); return; }