Put cells in pending mode immediately when the user clicks run. Remove them from the pending state if the controller does not create an execution immediately inside the execute handler.

2026-08-19 14:22:19 +01:00 · 2022-02-11 15:00:46 -08:00
parent 6560fb7ea5
commit d8eb6006eb
12 changed files with 101 additions and 20 deletions
@@ -248,6 +248,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape
 	$createExecution(handle: number, controllerId: string, rawUri: UriComponents, cellHandle: number): void {
 		const uri = URI.revive(rawUri);
 		const execution = this._notebookExecutionStateService.createCellExecution(controllerId, uri, cellHandle);
+		execution.confirm();
 		this._executions.set(handle, execution);
 	}

@@ -341,7 +341,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape {
 		if (cell) {
 			this._onDidChangeCellExecutionState.fire({
 				cell: cell.apiCell,
-				state: state ?? ExtHostNotebookCellExecutionState.Idle
+				state: state ? extHostTypeConverters.NotebookCellExecutionState.to(state) : ExtHostNotebookCellExecutionState.Idle
 			});
 		}
 	}
@@ -1506,6 +1506,20 @@ export namespace NotebookCellExecutionSummary {
 	}
 }

+export namespace NotebookCellExecutionState {
+	export function to(state: notebooks.NotebookCellExecutionState): vscode.NotebookCellExecutionState {
+		if (state === notebooks.NotebookCellExecutionState.Executing) {
+			return types.NotebookCellExecutionState.Executing;
+		} else if (state === notebooks.NotebookCellExecutionState.Pending) {
+			return types.NotebookCellExecutionState.Pending;
+		} else if (state === notebooks.NotebookCellExecutionState.Unconfirmed) {
+			return types.NotebookCellExecutionState.Pending;
+		} else {
+			throw new Error(`Unknown state: ${state}`);
+		}
+	}
+}
+
 export namespace NotebookCellKind {
 	export function from(data: vscode.NotebookCellKind): notebooks.CellKind {
 		switch (data) {
@@ -162,7 +162,7 @@ class ExecutionStateCellStatusBarItem extends Disposable {
 				alignment: CellStatusbarAlignment.Left,
 				priority: Number.MAX_SAFE_INTEGER
 			};
-		} else if (state === NotebookCellExecutionState.Pending) {
+		} else if (state === NotebookCellExecutionState.Pending || state === NotebookCellExecutionState.Unconfirmed) {
 			return <INotebookCellStatusBarItem>{
 				text: `$(${pendingStateIcon.id})`,
 				tooltip: localize('notebook.cell.status.pending', "Pending"),
@@ -9,7 +9,7 @@ import { ILogService } from 'vs/platform/log/common/log';
 import { IWorkspaceTrustRequestService } from 'vs/platform/workspace/common/workspaceTrust';
 import { SELECT_KERNEL_ID } from 'vs/workbench/contrib/notebook/browser/controller/coreActions';
 import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel';
-import { CellKind, INotebookTextModel } from 'vs/workbench/contrib/notebook/common/notebookCommon';
+import { CellKind, INotebookTextModel, NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon';
 import { INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService';
 import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
 import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService';
@@ -45,7 +45,7 @@ export class NotebookExecutionService implements INotebookExecutionService {
 			return;
 		}

-		const cellHandles: number[] = [];
+		const executeCells: NotebookCellTextModel[] = [];
 		for (const cell of cellsArr) {
 			const cellExe = this._notebookExecutionStateService.getCellExecution(cell.uri);
 			if (cell.cellKind !== CellKind.Code || !!cellExe) {
@@ -54,12 +54,19 @@ export class NotebookExecutionService implements INotebookExecutionService {
 			if (!kernel.supportedLanguages.includes(cell.language)) {
 				continue;
 			}
-			cellHandles.push(cell.handle);
+			executeCells.push(cell);
 		}

-		if (cellHandles.length > 0) {
+		if (executeCells.length > 0) {
 			this._notebookKernelService.selectKernelForNotebook(kernel, notebook);
-			await kernel.executeNotebookCellsRequest(notebook.uri, cellHandles);
+
+			const exes = executeCells.map(c => this._notebookExecutionStateService.createCellExecution(kernel!.id, notebook.uri, c.handle));
+			await kernel.executeNotebookCellsRequest(notebook.uri, executeCells.map(c => c.handle));
+			const unconfirmed = exes.filter(exe => exe.state === NotebookCellExecutionState.Unconfirmed);
+			if (unconfirmed.length) {
+				this._logService.debug(`NotebookExecutionService#executeNotebookCells completing unconfirmed executions ${JSON.stringify(unconfirmed.map(exe => exe.cellHandle))}`);
+				unconfirmed.forEach(exe => exe.complete({}));
+			}
 		}
 	}

@@ -260,7 +260,7 @@ class CellExecution extends Disposable implements INotebookCellExecution {
 	private readonly _onDidComplete = this._register(new Emitter<void>());
 	readonly onDidComplete = this._onDidComplete.event;

-	private _state: NotebookCellExecutionState = NotebookCellExecutionState.Pending;
+	private _state: NotebookCellExecutionState = NotebookCellExecutionState.Unconfirmed;
 	get state() {
 		return this._state;
 	}
@@ -308,6 +308,12 @@ class CellExecution extends Disposable implements INotebookCellExecution {
 		this._logService.debug(`CellExecution#updateExecution ${this.getCellLog()}, [${updateTypes}]`);
 	}

+	confirm() {
+		this._logService.debug(`CellExecution#confirm ${this.getCellLog()}`);
+		this._state = NotebookCellExecutionState.Pending;
+		this._onDidUpdate.fire();
+	}
+
 	update(updates: ICellExecuteUpdate[]): void {
 		this.logUpdates(updates);
 		if (updates.some(u => u.editType === CellExecutionUpdateType.ExecutionState)) {
@@ -136,7 +136,7 @@ export class CellContextKeyManager extends Disposable {
 		} else if (exeState?.state === NotebookCellExecutionState.Executing) {
 			this.cellRunState.set('executing');
 			this.cellExecuting.set(true);
-		} else if (exeState?.state === NotebookCellExecutionState.Pending) {
+		} else if (exeState?.state === NotebookCellExecutionState.Pending || exeState?.state === NotebookCellExecutionState.Unconfirmed) {
 			this.cellRunState.set('pending');
 			this.cellExecuting.set(true);
 		} else if (internalMetadata.lastRunSuccess === true) {
@@ -73,7 +73,7 @@ export class CodeCellExecutionIcon extends Disposable {
 				text: `$(${errorStateIcon.id})`,
 				tooltip: localize('notebook.cell.status.failed', "Failed"),
 			};
-		} else if (state === NotebookCellExecutionState.Pending) {
+		} else if (state === NotebookCellExecutionState.Pending || state === NotebookCellExecutionState.Unconfirmed) {
 			return <IExecutionItem>{
 				text: `$(${pendingStateIcon.id})`,
 				tooltip: localize('notebook.cell.status.pending', "Pending"),
@@ -83,8 +83,8 @@ export enum NotebookRunState {

 export type NotebookDocumentMetadata = Record<string, unknown>;

-// Aligns with the vscode.d.ts version
 export enum NotebookCellExecutionState {
+	Unconfirmed = 1,
 	Pending = 2,
 	Executing = 3
 }
@@ -60,6 +60,7 @@ export interface INotebookCellExecution {
 	readonly didPause: boolean;
 	readonly isPaused: boolean;

+	confirm(): void;
 	update(updates: ICellExecuteUpdate[]): void;
 	complete(complete: ICellExecutionComplete): void;
 }
@@ -19,6 +19,7 @@ import { NotebookKernelService } from 'vs/workbench/contrib/notebook/browser/not
 import { NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl';
 import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel';
 import { CellKind, IOutputDto, NotebookCellMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon';
+import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
 import { INotebookKernel, INotebookKernelService, ISelectedNotebooksChangeEvent } from 'vs/workbench/contrib/notebook/common/notebookKernelService';
 import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
 import { setupInstantiationService, withTestNotebook as _withTestNotebook } from 'vs/workbench/contrib/notebook/test/browser/testNotebookEditor';
@@ -135,6 +136,34 @@ suite('NotebookExecutionService', () => {
 			assert.strictEqual(event.oldKernel, undefined);
 		});
 	});
+
+	test('Completes unconfirmed executions', async function () {
+
+		return withTestNotebook([], async viewModel => {
+			let didExecute = false;
+			const kernel = new class extends TestNotebookKernel {
+				constructor() {
+					super({ languages: ['javascript'] });
+					this.id = 'mySpecialId';
+				}
+
+				override async executeNotebookCellsRequest() {
+					didExecute = true;
+					return;
+				}
+			};
+
+			kernelService.registerKernel(kernel);
+			const executionService = instantiationService.createInstance(NotebookExecutionService);
+			const exeStateService = instantiationService.get(INotebookExecutionStateService);
+
+			const cell = insertCellAtIndex(viewModel, 0, 'var c = 3', 'javascript', CellKind.Code, {}, [], true, true);
+			await executionService.executeNotebookCells(viewModel.notebookDocument, [cell]);
+
+			assert.strictEqual(didExecute, true);
+			assert.strictEqual(exeStateService.getCellExecution(cell.uri), undefined);
+		});
+	});
 });

 class TestNotebookKernel implements INotebookKernel {
@@ -44,7 +44,7 @@ import { CellViewModel, NotebookViewModel } from 'vs/workbench/contrib/notebook/
 import { ViewContext } from 'vs/workbench/contrib/notebook/browser/viewModel/viewContext';
 import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel';
 import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel';
-import { CellKind, CellUri, INotebookDiffEditorModel, INotebookEditorModel, INotebookSearchOptions, IOutputDto, IResolvedNotebookEditorModel, NotebookCellMetadata, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon';
+import { CellKind, CellUri, INotebookDiffEditorModel, INotebookEditorModel, INotebookSearchOptions, IOutputDto, IResolvedNotebookEditorModel, NotebookCellExecutionState, NotebookCellMetadata, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon';
 import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionStateChangedEvent, INotebookCellExecution, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
 import { NotebookOptions } from 'vs/workbench/contrib/notebook/common/notebookOptions';
 import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange';
@@ -52,6 +52,7 @@ import { TextModelResolverService } from 'vs/workbench/services/textmodelResolve
 import { TestWorkspaceTrustRequestService } from 'vs/workbench/services/workspaces/test/common/testWorkspaceTrustService';
 import { TestClipboardService, TestLayoutService } from 'vs/workbench/test/browser/workbenchTestServices';
 import { TestStorageService } from 'vs/workbench/test/common/workbenchTestServices';
+import { ResourceMap } from 'vs/base/common/map';

 export class TestCell extends NotebookCellTextModel {
 	constructor(
@@ -378,9 +379,34 @@ export function valueBytesFromString(value: string): VSBuffer {
 	return VSBuffer.fromString(value);
 }

+class TestCellExecution implements INotebookCellExecution {
+	constructor(
+		readonly notebook: URI,
+		readonly cellHandle: number,
+		private onComplete: () => void,
+	) { }
+
+	readonly state: NotebookCellExecutionState = NotebookCellExecutionState.Unconfirmed;
+
+	readonly didPause: boolean = false;
+	readonly isPaused: boolean = false;
+
+	confirm(): void {
+	}
+
+	update(updates: ICellExecuteUpdate[]): void {
+	}
+
+	complete(complete: ICellExecutionComplete): void {
+		this.onComplete();
+	}
+}
+
 class TestNotebookExecutionStateService implements INotebookExecutionStateService {
 	_serviceBrand: undefined;

+	private _executions = new ResourceMap<INotebookCellExecution>();
+
 	onDidChangeCellExecution = new Emitter<ICellExecutionStateChangedEvent>().event;

 	forceCancelNotebookExecutions(notebookUri: URI): void {
@@ -391,16 +417,13 @@ class TestNotebookExecutionStateService implements INotebookExecutionStateServic
 	}

 	getCellExecution(cellUri: URI): INotebookCellExecution | undefined {
-		return undefined;
+		return this._executions.get(cellUri);
 	}

 	createCellExecution(controllerId: string, notebook: URI, cellHandle: number): INotebookCellExecution {
-		return undefined!;
-	}
-
-	updateNotebookCellExecution(notebook: URI, cellHandle: number, updates: ICellExecuteUpdate[]): void {
-	}
-
-	completeNotebookCellExecution(notebook: URI, cellHandle: number, complete: ICellExecutionComplete): void {
+		const onComplete = () => this._executions.delete(CellUri.generate(notebook, cellHandle));
+		const exe = new TestCellExecution(notebook, cellHandle, onComplete);
+		this._executions.set(CellUri.generate(notebook, cellHandle), exe);
+		return exe;
 	}
 }