From 483a8b5d828db96f61cef6adcad055aed134525a Mon Sep 17 00:00:00 2001 From: Jake Donham Date: Tue, 23 Jul 2024 09:40:40 -0700 Subject: [PATCH] don't execute paused cells when initiated from client fixes #48. also clarified usages of `force`. this is a little bit hairy because execution can be initiated from either client (user runs a cell, switches cell focus, or clicks Run Dirty) or server (external change triggers execution of dirty dependencies), and the server doesn't know if a cell is paused; so we need to check in a few places. --- packages/server/src/server.ts | 6 +++--- packages/vscode/src/controller.ts | 18 +++++++++++------- packages/vscode/src/extension.ts | 1 + .../handleDidChangeNotebookEditorSelection.ts | 3 +++ 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/packages/server/src/server.ts b/packages/server/src/server.ts index 5f6014c..bb1d3ae 100644 --- a/packages/server/src/server.ts +++ b/packages/server/src/server.ts @@ -175,9 +175,9 @@ class VitaleDevServer { } private invalidateModuleAndDirty(id: string) { - const cells: { path: string; cellId: string; ext: string }[] = []; - this.runtime.invalidateRuntimeModule(id, cells); - this.rpc.markCellsDirty(cells); + const dirtyCells: { path: string; cellId: string; ext: string }[] = []; + this.runtime.invalidateRuntimeModule(id, dirtyCells); + this.rpc.markCellsDirty(dirtyCells); } listen() { diff --git a/packages/vscode/src/controller.ts b/packages/vscode/src/controller.ts index d87d1ac..9c47ac1 100644 --- a/packages/vscode/src/controller.ts +++ b/packages/vscode/src/controller.ts @@ -349,6 +349,7 @@ export class NotebookController { notebookCells.map((cell) => this.setCellDirty(cell, true)) ); if (getRerunCellsWhenDirty()) { + // don't force paused cells, user didn't request execution this.executeCells(notebookCells, false); } } @@ -443,6 +444,7 @@ export class NotebookController { } private executeHandler(notebookCells: vscode.NotebookCell[]) { + // force paused cells, user explictly requested execution this.executeCells(notebookCells, true); } @@ -453,17 +455,19 @@ export class NotebookController { if (notebookCells.length === 0) { return; } - const cells = notebookCells.map((cell) => ({ - path: cell.notebook.uri.fsPath, - cellId: cell.metadata.id, - language: cell.document.languageId, - code: cell.document.getText(), - })); - await Promise.all( notebookCells.map((cell) => this.setCellDocDirty(cell, false)) ); + const cells = notebookCells + .filter((cell) => !cell.metadata.paused || force) + .map((cell) => ({ + path: cell.notebook.uri.fsPath, + cellId: cell.metadata.id, + language: cell.document.languageId, + code: cell.document.getText(), + })); + const client = await this.getClient(); client.executeCells(cells, force, getRerunCellsWhenDirty()); } diff --git a/packages/vscode/src/extension.ts b/packages/vscode/src/extension.ts index 515be99..94bd951 100644 --- a/packages/vscode/src/extension.ts +++ b/packages/vscode/src/extension.ts @@ -27,6 +27,7 @@ export function activate(context: vscode.ExtensionContext) { vscode.commands.registerCommand( "vitale.runDirty", (ctx: { notebookEditor: { notebookUri: string } }) => { + // force paused cells, user explicitly requested execution controller.runDirty(ctx.notebookEditor.notebookUri, true); } ), diff --git a/packages/vscode/src/handleDidChangeNotebookEditorSelection.ts b/packages/vscode/src/handleDidChangeNotebookEditorSelection.ts index 0279331..318f3ef 100644 --- a/packages/vscode/src/handleDidChangeNotebookEditorSelection.ts +++ b/packages/vscode/src/handleDidChangeNotebookEditorSelection.ts @@ -1,11 +1,13 @@ import type { NotebookEditorSelectionChangeEvent } from "vscode"; import { getRerunCellsWhenDirty } from "./controller"; import type { NotebookController } from "./controller"; +import { log } from "./log"; export function makeHandleDidChangeNotebookEditorSelection( controller: NotebookController ) { return (e: NotebookEditorSelectionChangeEvent) => { + log.info("selection changed"); // what we really want here is to run when an edited cell loses focus // but it doesn't seem to be possible in the VS Code API // so instead we run when the notebook selection changes @@ -18,6 +20,7 @@ export function makeHandleDidChangeNotebookEditorSelection( // TODO(jaked) ugh setTimeout(() => { if (getRerunCellsWhenDirty()) { + // don't force paused cells, user didn't explicitly request execution controller.runDirty(e.notebookEditor.notebook.uri.toString(), false); } }, 100);