From eea761214f376f8f21eb457373e250f463489261 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 21 Aug 2023 02:27:13 +0100 Subject: [PATCH 1/4] Define cells to run as independent of selection --- packages/notebook-extension/src/index.ts | 49 ++++++--- packages/notebook/src/actions.tsx | 131 ++++++++++++++++------- 2 files changed, 124 insertions(+), 56 deletions(-) diff --git a/packages/notebook-extension/src/index.ts b/packages/notebook-extension/src/index.ts index aa055c003fff..c2cd8f7a2757 100644 --- a/packages/notebook-extension/src/index.ts +++ b/packages/notebook-extension/src/index.ts @@ -2399,18 +2399,24 @@ function addCommands( }); commands.addCommand(CommandIDs.restartAndRunToSelected, { label: trans.__('Restart Kernel and Run up to Selected Cell…'), - execute: async () => { - const restarted: boolean = await commands.execute(CommandIDs.restart, { - activate: false - }); + execute: async args => { + const current = getCurrent(tracker, shell, { activate: false, ...args }); + if (!current) { + return; + } + const { context, content } = current; + + const cells = content.widgets.slice(0, content.activeCellIndex + 1); + const restarted = await sessionDialogs.restart(current.sessionContext); + if (restarted) { - const executed: boolean = await commands.execute( - CommandIDs.runAllAbove, - { activate: false } + return NotebookActions.runCells( + content, + cells, + context.sessionContext, + sessionDialogs, + translator ); - if (executed) { - return commands.execute(CommandIDs.run); - } } }, isEnabled: isEnabledAndSingleSelected @@ -2418,12 +2424,25 @@ function addCommands( commands.addCommand(CommandIDs.restartRunAll, { label: trans.__('Restart Kernel and Run All Cells…'), caption: trans.__('Restart the kernel and run all cells'), - execute: async () => { - const restarted: boolean = await commands.execute(CommandIDs.restart, { - activate: false - }); + execute: async args => { + const current = getCurrent(tracker, shell, { activate: false, ...args }); + + if (!current) { + return; + } + const { context, content } = current; + + const cells = content.widgets; + const restarted = await sessionDialogs.restart(current.sessionContext); + if (restarted) { - await commands.execute(CommandIDs.runAll); + return NotebookActions.runCells( + content, + cells, + context.sessionContext, + sessionDialogs, + translator + ); } }, isEnabled: args => (args.toolbar ? true : isEnabled()), diff --git a/packages/notebook/src/actions.tsx b/packages/notebook/src/actions.tsx index 54e7ce4f4dc5..0ba677f0d80e 100644 --- a/packages/notebook/src/actions.tsx +++ b/packages/notebook/src/actions.tsx @@ -545,6 +545,42 @@ export namespace NotebookActions { return promise; } + /** + * Run specified cells. + * + * @param notebook - The target notebook widget. + * @param cells - The cells to run. + * @param sessionContext - The client session object. + * @param sessionDialogs - The session dialogs. + * @param translator - The application translator. + * + * #### Notes + * An execution error will prevent the remaining code cells from executing. + * All markdown cells will be rendered. + */ + export function runCells( + notebook: Notebook, + cells: readonly Cell[], + sessionContext?: ISessionContext, + sessionDialogs?: ISessionContextDialogs, + translator?: ITranslator + ): Promise { + if (!notebook.model) { + return Promise.resolve(false); + } + + const state = Private.getState(notebook); + const promise = Private.runCells( + notebook, + cells, + sessionContext, + sessionDialogs, + translator + ); + Private.handleRunState(notebook, state, false); + return promise; + } + /** * Run the selected cell(s) and advance to the next cell. * @@ -693,12 +729,9 @@ export namespace NotebookActions { const state = Private.getState(notebook); - notebook.widgets.forEach(child => { - notebook.select(child); - }); - - const promise = Private.runSelected( + const promise = Private.runCells( notebook, + notebook.widgets, sessionContext, sessionDialogs, translator @@ -759,20 +792,14 @@ export namespace NotebookActions { const state = Private.getState(notebook); - notebook.activeCellIndex--; - notebook.deselectAll(); - for (let i = 0; i < notebook.activeCellIndex; ++i) { - notebook.select(notebook.widgets[i]); - } - - const promise = Private.runSelected( + const promise = Private.runCells( notebook, + notebook.widgets.slice(0, notebook.activeCellIndex), sessionContext, sessionDialogs, translator ); - notebook.activeCellIndex++; Private.handleRunState(notebook, state, true); return promise; } @@ -803,13 +830,9 @@ export namespace NotebookActions { const state = Private.getState(notebook); - notebook.deselectAll(); - for (let i = notebook.activeCellIndex; i < notebook.widgets.length; ++i) { - notebook.select(notebook.widgets[i]); - } - - const promise = Private.runSelected( + const promise = Private.runCells( notebook, + notebook.widgets.slice(notebook.activeCellIndex, undefined), sessionContext, sessionDialogs, translator @@ -2187,35 +2210,22 @@ namespace Private { * Run the selected cells. * * @param notebook Notebook + * @param cells Cells to run * @param sessionContext Notebook session context * @param sessionDialogs Session dialogs * @param translator Application translator */ - export function runSelected( + export function runCells( notebook: Notebook, + cells: readonly Cell[], sessionContext?: ISessionContext, sessionDialogs?: ISessionContextDialogs, translator?: ITranslator ): Promise { - notebook.mode = 'command'; - - let lastIndex = notebook.activeCellIndex; - const selected = notebook.widgets.filter((child, index) => { - const active = notebook.isSelectedOrActive(child); - - if (active) { - lastIndex = index; - } - - return active; - }); - - notebook.activeCellIndex = lastIndex; - notebook.deselectAll(); - + const lastCell = cells[-1]; return Promise.all( - selected.map(child => - runCell(notebook, child, sessionContext, sessionDialogs, translator) + cells.map(cell => + runCell(notebook, cell, sessionContext, sessionDialogs, translator) ) ) .then(results => { @@ -2224,7 +2234,7 @@ namespace Private { } selectionExecuted.emit({ notebook, - lastCell: notebook.widgets[lastIndex] + lastCell }); // Post an update request. notebook.update(); @@ -2233,7 +2243,7 @@ namespace Private { }) .catch(reason => { if (reason.message.startsWith('KernelReplyNotOK')) { - selected.map(cell => { + cells.map(cell => { // Remove '*' prompt from cells that didn't execute if ( cell.model.type === 'code' && @@ -2248,7 +2258,7 @@ namespace Private { selectionExecuted.emit({ notebook, - lastCell: notebook.widgets[lastIndex] + lastCell }); notebook.update(); @@ -2257,6 +2267,45 @@ namespace Private { }); } + /** + * Run the selected cells. + * + * @param notebook Notebook + * @param sessionContext Notebook session context + * @param sessionDialogs Session dialogs + * @param translator Application translator + */ + export function runSelected( + notebook: Notebook, + sessionContext?: ISessionContext, + sessionDialogs?: ISessionContextDialogs, + translator?: ITranslator + ): Promise { + notebook.mode = 'command'; + + let lastIndex = notebook.activeCellIndex; + const selected = notebook.widgets.filter((child, index) => { + const active = notebook.isSelectedOrActive(child); + + if (active) { + lastIndex = index; + } + + return active; + }); + + notebook.activeCellIndex = lastIndex; + notebook.deselectAll(); + + return runCells( + notebook, + selected, + sessionContext, + sessionDialogs, + translator + ); + } + /** * Run a cell. */ From 62bd31fe6d8e9f9408fcef30fd29830cb1f31feb Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Mon, 21 Aug 2023 10:37:13 +0100 Subject: [PATCH 2/4] Restore side effects of `runAll()` --- packages/notebook/src/actions.tsx | 10 +++++++++ packages/notebook/test/actions.spec.ts | 30 ++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/packages/notebook/src/actions.tsx b/packages/notebook/src/actions.tsx index 0ba677f0d80e..a3108458dcbb 100644 --- a/packages/notebook/src/actions.tsx +++ b/packages/notebook/src/actions.tsx @@ -555,6 +555,8 @@ export namespace NotebookActions { * @param translator - The application translator. * * #### Notes + * The existing selection will be preserved. + * The mode will be changed to command. * An execution error will prevent the remaining code cells from executing. * All markdown cells will be rendered. */ @@ -728,6 +730,7 @@ export namespace NotebookActions { } const state = Private.getState(notebook); + const lastIndex = notebook.widgets.length; const promise = Private.runCells( notebook, @@ -737,6 +740,9 @@ export namespace NotebookActions { translator ); + notebook.activeCellIndex = lastIndex; + notebook.deselectAll(); + Private.handleRunState(notebook, state, true); return promise; } @@ -800,6 +806,8 @@ export namespace NotebookActions { translator ); + notebook.deselectAll(); + Private.handleRunState(notebook, state, true); return promise; } @@ -2223,6 +2231,8 @@ namespace Private { translator?: ITranslator ): Promise { const lastCell = cells[-1]; + notebook.mode = 'command'; + return Promise.all( cells.map(cell => runCell(notebook, cell, sessionContext, sessionDialogs, translator) diff --git a/packages/notebook/test/actions.spec.ts b/packages/notebook/test/actions.spec.ts index c7f0e8d29ddf..b91ac65391c7 100644 --- a/packages/notebook/test/actions.spec.ts +++ b/packages/notebook/test/actions.spec.ts @@ -1000,6 +1000,36 @@ describe('@jupyterlab/notebook', () => { }); }); + describe('#runCells()', () => { + beforeEach(() => { + // Make sure all cells have valid code. + widget.widgets[2].model.sharedModel.setSource('a = 1'); + }); + + it('should change to command mode', async () => { + widget.mode = 'edit'; + const result = await NotebookActions.runCells( + widget, + [widget.widgets[2]], + sessionContext + ); + expect(result).toBe(true); + expect(widget.mode).toBe('command'); + }); + + it('should preserve the existing selection', async () => { + const next = widget.widgets[2]; + widget.select(next); + const result = await NotebookActions.runCells( + widget, + [widget.widgets[1]], + sessionContext + ); + expect(result).toBe(true); + expect(widget.isSelected(widget.widgets[2])).toBe(true); + }); + }); + describe('#runAll()', () => { beforeEach(() => { // Make sure all cells have valid code. From 32f1977047b46a46bb38e667867edc565e761878 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Krassowski?= <5832902+krassowski@users.noreply.github.com> Date: Sun, 8 Oct 2023 18:28:50 +0100 Subject: [PATCH 3/4] Skip optional `slice` argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Frédéric Collonval --- packages/notebook/src/actions.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/notebook/src/actions.tsx b/packages/notebook/src/actions.tsx index a3108458dcbb..9e06bcd5059d 100644 --- a/packages/notebook/src/actions.tsx +++ b/packages/notebook/src/actions.tsx @@ -840,7 +840,7 @@ export namespace NotebookActions { const promise = Private.runCells( notebook, - notebook.widgets.slice(notebook.activeCellIndex, undefined), + notebook.widgets.slice(notebook.activeCellIndex), sessionContext, sessionDialogs, translator From 202e958e967955d7a3d94a1d666dd7661a77a644 Mon Sep 17 00:00:00 2001 From: krassowski <5832902+krassowski@users.noreply.github.com> Date: Sun, 8 Oct 2023 19:03:37 +0100 Subject: [PATCH 4/4] Restore previous behaviour of `runAllBelow()` by making last cell active. --- packages/notebook/src/actions.tsx | 4 ++++ packages/notebook/test/actions.spec.ts | 21 +++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/packages/notebook/src/actions.tsx b/packages/notebook/src/actions.tsx index 9e06bcd5059d..ff6998e44152 100644 --- a/packages/notebook/src/actions.tsx +++ b/packages/notebook/src/actions.tsx @@ -837,6 +837,7 @@ export namespace NotebookActions { } const state = Private.getState(notebook); + const lastIndex = notebook.widgets.length; const promise = Private.runCells( notebook, @@ -846,6 +847,9 @@ export namespace NotebookActions { translator ); + notebook.activeCellIndex = lastIndex; + notebook.deselectAll(); + Private.handleRunState(notebook, state, true); return promise; } diff --git a/packages/notebook/test/actions.spec.ts b/packages/notebook/test/actions.spec.ts index b91ac65391c7..f53914f29f94 100644 --- a/packages/notebook/test/actions.spec.ts +++ b/packages/notebook/test/actions.spec.ts @@ -1100,6 +1100,27 @@ describe('@jupyterlab/notebook', () => { }); }); + describe('#runAllBelow()', () => { + it('should run all selected cell and all below', async () => { + const next = widget.widgets[1] as MarkdownCell; + const cell = widget.activeCell as CodeCell; + cell.model.outputs.clear(); + next.rendered = false; + const result = await NotebookActions.runAllBelow( + widget, + sessionContext + ); + expect(result).toBe(true); + expect(cell.model.outputs.length).toBeGreaterThan(0); + expect(next.rendered).toBe(true); + }); + + it('should activate the last cell', async () => { + await NotebookActions.runAllBelow(widget, sessionContext); + expect(widget.activeCellIndex).toBe(widget.widgets.length - 1); + }); + }); + describe('#selectAbove()', () => { it('should select the cell above the active cell', () => { widget.activeCellIndex = 1;