From 42cb1c68b2ab81e5e677f117c8b82247d39218b2 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 5 Apr 2023 14:43:03 +1000 Subject: [PATCH 1/8] Support for arbitrary notebook kernel execution --- .../api/browser/mainThreadNotebookKernels.ts | 44 +++++- .../workbench/api/common/extHost.protocol.ts | 4 + .../api/common/extHostNotebookKernels.ts | 109 +++++++++++++- .../browser/controller/executeActions.ts | 16 +-- .../notebookExecutionStateServiceImpl.ts | 135 ++++++++++++++++-- .../notebookEditorWidgetContextKeys.ts | 21 ++- .../contrib/notebook/common/notebookCommon.ts | 5 + .../notebook/common/notebookContextKeys.ts | 1 + .../common/notebookExecutionStateService.ts | 18 ++- .../notebookExecutionStateService.test.ts | 18 +-- .../test/browser/testNotebookEditor.ts | 9 +- .../common/extensionsApiProposals.ts | 2 + .../vscode.proposed.notebookExecution.d.ts | 48 +++++++ 13 files changed, 394 insertions(+), 36 deletions(-) create mode 100644 src/vscode-dts/vscode.proposed.notebookExecution.d.ts diff --git a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts index 80671ccc3862c..2af1912229380 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts @@ -15,7 +15,7 @@ import { NotebookDto } from 'vs/workbench/api/browser/mainThreadNotebookDto'; import { extHostNamedCustomer, IExtHostContext } from 'vs/workbench/services/extensions/common/extHostCustomers'; import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/services/notebookEditorService'; -import { INotebookCellExecution, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { INotebookCellExecution, INotebookExecution, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { IKernelSourceActionProvider, INotebookKernel, INotebookKernelChangeEvent, INotebookKernelDetectionTask, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { SerializableObjectWithBuffers } from 'vs/workbench/services/extensions/common/proxyIdentifier'; import { ExtHostContext, ExtHostNotebookKernelsShape, ICellExecuteUpdateDto, ICellExecutionCompleteDto, INotebookKernelDto2, MainContext, MainThreadNotebookKernelsShape } from '../common/extHost.protocol'; @@ -114,6 +114,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape private readonly _proxy: ExtHostNotebookKernelsShape; private readonly _executions = new Map(); + private readonly _notebookExecutions = new Map(); constructor( extHostContext: IExtHostContext, @@ -134,6 +135,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape this._executions.forEach(e => { e.complete({}); }); + this._notebookExecutions.forEach(e => e.complete()); })); this._disposables.add(this._notebookExecutionStateService.onDidChangeCellExecution(e => { @@ -257,7 +259,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape } } - // --- execution + // --- Cell execution $createExecution(handle: number, controllerId: string, rawUri: UriComponents, cellHandle: number): void { const uri = URI.revive(rawUri); @@ -296,6 +298,44 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape } } + // --- Notebook execution + + $createNotebookExecution(handle: number, controllerId: string, rawUri: UriComponents): void { + const uri = URI.revive(rawUri); + const notebook = this._notebookService.getNotebookTextModel(uri); + if (!notebook) { + throw new Error(`Notebook not found: ${uri.toString()}`); + } + + const kernel = this._notebookKernelService.getMatchingKernel(notebook); + if (!kernel.selected || kernel.selected.id !== controllerId) { + throw new Error(`Kernel is not selected: ${kernel.selected?.id} !== ${controllerId}`); + } + const execution = this._notebookExecutionStateService.createExecution(uri); + execution.confirm(); + this._notebookExecutions.set(handle, execution); + } + + $beginNotebookExecution(handle: number): void { + try { + const execution = this._notebookExecutions.get(handle); + execution?.begin(); + } catch (e) { + onUnexpectedError(e); + } + } + + $completeNotebookExecution(handle: number): void { + try { + const execution = this._notebookExecutions.get(handle); + execution?.complete(); + } catch (e) { + onUnexpectedError(e); + } finally { + this._notebookExecutions.delete(handle); + } + } + // --- notebook kernel detection task async $addKernelDetectionTask(handle: number, notebookType: string): Promise { const kernelDetectionTask = new MainThreadKernelDetectionTask(notebookType); diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 18534ef8acae8..944f5d5c92258 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -1056,6 +1056,10 @@ export interface MainThreadNotebookKernelsShape extends IDisposable { $updateExecution(handle: number, data: SerializableObjectWithBuffers): void; $completeExecution(handle: number, data: SerializableObjectWithBuffers): void; + $createNotebookExecution(handle: number, controllerId: string, uri: UriComponents): void; + $beginNotebookExecution(handle: number,): void; + $completeNotebookExecution(handle: number): void; + $addKernelDetectionTask(handle: number, notebookType: string): Promise; $removeKernelDetectionTask(handle: number): void; diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index 014c2782bfdf6..a35109582e980 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -17,7 +17,7 @@ import { ExtHostNotebookKernelsShape, ICellExecuteUpdateDto, IMainContext, INote import { ApiCommand, ApiCommandArgument, ApiCommandResult, ExtHostCommands } from 'vs/workbench/api/common/extHostCommands'; import { IExtHostInitDataService } from 'vs/workbench/api/common/extHostInitDataService'; import { ExtHostNotebookController } from 'vs/workbench/api/common/extHostNotebook'; -import { ExtHostCell } from 'vs/workbench/api/common/extHostNotebookDocument'; +import { ExtHostCell, ExtHostNotebookDocument } from 'vs/workbench/api/common/extHostNotebookDocument'; import * as extHostTypeConverters from 'vs/workbench/api/common/extHostTypeConverters'; import { NotebookCellExecutionState as ExtHostNotebookCellExecutionState, NotebookCellOutput, NotebookControllerAffinity2 } from 'vs/workbench/api/common/extHostTypes'; import { asWebviewUri } from 'vs/workbench/contrib/webview/common/webview'; @@ -44,6 +44,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { private readonly _proxy: MainThreadNotebookKernelsShape; private readonly _activeExecutions = new ResourceMap(); + private readonly _activeNotebookExecutions = new ResourceMap<[NotebookExecutionTask, IDisposable]>(); private _kernelDetectionTask = new Map(); private _kernelDetectionTaskHandlePool: number = 0; @@ -219,6 +220,16 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { } return that._createNotebookCellExecution(cell, createKernelId(extension.identifier, this.id)); }, + createNotebookExecution(notebook) { + if (isDisposed) { + throw new Error('notebook controller is DISPOSED'); + } + if (!associatedNotebooks.has(notebook.uri)) { + that._logService.trace(`NotebookController[${handle}] NOT associated to notebook, associated to THESE notebooks:`, Array.from(associatedNotebooks.keys()).map(u => u.toString())); + throw new Error(`notebook controller is NOT associated to notebook: ${notebook.uri.toString()}`); + } + return that._createNotebookExecution(notebook, createKernelId(extension.identifier, this.id)); + }, dispose: () => { if (!isDisposed) { this._logService.trace(`NotebookController[${handle}], DISPOSED`); @@ -387,6 +398,12 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { } } } + + // If we're interrupting all cells, we also need to cancel the notebook level execution. + const items = this._activeNotebookExecutions.get(document.uri); + if (handles.length && Array.isArray(items) && items.length) { + items.forEach(d => d.dispose()); + } } $acceptKernelMessageFromRenderer(handle: number, editorId: string, message: any): void { @@ -439,6 +456,32 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { }); return execution.asApiObject(); } + + // --- + + _createNotebookExecution(nb: vscode.NotebookDocument, controllerId: string): vscode.NotebookExecution { + const notebook = this._extHostNotebook.getNotebookDocument(nb.uri); + const runningCell = nb.getCells().find(cell => { + const apiCell = notebook.getCellFromApiCell(cell); + return apiCell && this._activeExecutions.has(apiCell.uri); + }); + if (runningCell) { + throw new Error(`duplicate cell execution for ${runningCell.document.uri}`); + } + if (this._activeNotebookExecutions.has(notebook.uri)) { + throw new Error(`duplicate notebook execution for ${notebook.uri}`); + } + const execution = new NotebookExecutionTask(controllerId, notebook, this._proxy); + const listener = execution.onDidChangeState(() => { + if (execution.state === NotebookExecutionTaskState.Resolved) { + execution.dispose(); + listener.dispose(); + this._activeNotebookExecutions.delete(notebook.uri); + } + }); + this._activeNotebookExecutions.set(notebook.uri, [execution, listener]); + return execution.asApiObject(); + } } @@ -622,6 +665,70 @@ class NotebookCellExecutionTask extends Disposable { } } + +enum NotebookExecutionTaskState { + Init, + Started, + Resolved +} + + +class NotebookExecutionTask extends Disposable { + private static HANDLE = 0; + private _handle = NotebookExecutionTask.HANDLE++; + + private _onDidChangeState = new Emitter(); + readonly onDidChangeState = this._onDidChangeState.event; + + private _state = NotebookExecutionTaskState.Init; + get state(): NotebookExecutionTaskState { return this._state; } + + private readonly _tokenSource = this._register(new CancellationTokenSource()); + + constructor( + controllerId: string, + private readonly _notebook: ExtHostNotebookDocument, + private readonly _proxy: MainThreadNotebookKernelsShape + ) { + super(); + + this._proxy.$createNotebookExecution(this._handle, controllerId, this._notebook.uri); + } + + cancel(): void { + this._tokenSource.cancel(); + } + asApiObject(): vscode.NotebookExecution { + const token = this._tokenSource.token; + const result: vscode.NotebookExecution = { + get token() { return token; }, + start: () => { + if (this._state === NotebookExecutionTaskState.Resolved || this._state === NotebookExecutionTaskState.Started) { + throw new Error('Cannot call start again'); + } + + this._state = NotebookExecutionTaskState.Started; + this._onDidChangeState.fire(); + + this._proxy.$beginNotebookExecution(this._handle); + }, + + end: () => { + if (this._state === NotebookExecutionTaskState.Resolved) { + throw new Error('Cannot call resolve twice'); + } + + this._state = NotebookExecutionTaskState.Resolved; + this._onDidChangeState.fire(); + + this._proxy.$completeNotebookExecution(this._handle); + }, + + }; + return Object.freeze(result); + } +} + class TimeoutBasedCollector { private batch: T[] = []; private startedTimer = Date.now(); diff --git a/src/vs/workbench/contrib/notebook/browser/controller/executeActions.ts b/src/vs/workbench/contrib/notebook/browser/controller/executeActions.ts index 2713b867d93cc..da85c5e975a88 100644 --- a/src/vs/workbench/contrib/notebook/browser/controller/executeActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/controller/executeActions.ts @@ -15,7 +15,7 @@ import { ThemeIcon } from 'vs/base/common/themables'; import { EditorsOrder } from 'vs/workbench/common/editor'; import { insertCell } from 'vs/workbench/contrib/notebook/browser/controller/cellOperations'; import { cellExecutionArgs, CellToolbarOrder, CELL_TITLE_CELL_GROUP_ID, executeNotebookCondition, getContextFromActiveEditor, getContextFromUri, INotebookActionContext, INotebookCellActionContext, INotebookCellToolbarActionContext, INotebookCommandContext, NotebookAction, NotebookCellAction, NotebookMultiCellAction, NOTEBOOK_EDITOR_WIDGET_ACTION_WEIGHT, parseMultiCellExecutionArgs } from 'vs/workbench/contrib/notebook/browser/controller/coreActions'; -import { NOTEBOOK_CELL_EXECUTING, NOTEBOOK_CELL_EXECUTION_STATE, NOTEBOOK_CELL_LIST_FOCUSED, NOTEBOOK_CELL_TYPE, NOTEBOOK_HAS_RUNNING_CELL, NOTEBOOK_INTERRUPTIBLE_KERNEL, NOTEBOOK_IS_ACTIVE_EDITOR, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_KERNEL_SOURCE_COUNT, NOTEBOOK_LAST_CELL_FAILED, NOTEBOOK_MISSING_KERNEL_EXTENSION } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; +import { NOTEBOOK_CELL_EXECUTING, NOTEBOOK_CELL_EXECUTION_STATE, NOTEBOOK_CELL_LIST_FOCUSED, NOTEBOOK_CELL_TYPE, NOTEBOOK_HAS_RUNNING_CELL, NOTEBOOK_HAS_SOMETHING_RUNNING, NOTEBOOK_INTERRUPTIBLE_KERNEL, NOTEBOOK_IS_ACTIVE_EDITOR, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_KERNEL_SOURCE_COUNT, NOTEBOOK_LAST_CELL_FAILED, NOTEBOOK_MISSING_KERNEL_EXTENSION } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; import { CellEditState, CellFocusMode, EXECUTE_CELL_COMMAND_ID } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import * as icons from 'vs/workbench/contrib/notebook/browser/notebookIcons'; import { CellKind, CellUri, NotebookSetting } from 'vs/workbench/contrib/notebook/common/notebookCommon'; @@ -124,7 +124,7 @@ registerAction2(class ExecuteNotebookAction extends NotebookAction { when: ContextKeyExpr.and( NOTEBOOK_IS_ACTIVE_EDITOR, executeNotebookCondition, - ContextKeyExpr.or(NOTEBOOK_INTERRUPTIBLE_KERNEL.toNegated(), NOTEBOOK_HAS_RUNNING_CELL.toNegated()), + ContextKeyExpr.or(NOTEBOOK_INTERRUPTIBLE_KERNEL.toNegated(), NOTEBOOK_HAS_SOMETHING_RUNNING.toNegated()), ContextKeyExpr.notEquals('config.notebook.globalToolbar', true) ) }, @@ -134,7 +134,7 @@ registerAction2(class ExecuteNotebookAction extends NotebookAction { group: 'navigation/execute', when: ContextKeyExpr.and( executeNotebookCondition, - ContextKeyExpr.or(NOTEBOOK_INTERRUPTIBLE_KERNEL.toNegated(), NOTEBOOK_HAS_RUNNING_CELL.toNegated()), + ContextKeyExpr.or(NOTEBOOK_INTERRUPTIBLE_KERNEL.toNegated(), NOTEBOOK_HAS_SOMETHING_RUNNING.toNegated()), ContextKeyExpr.equals('config.notebook.globalToolbar', true) ) } @@ -516,7 +516,7 @@ registerAction2(class CancelAllNotebook extends CancelNotebook { group: 'navigation', when: ContextKeyExpr.and( NOTEBOOK_IS_ACTIVE_EDITOR, - NOTEBOOK_HAS_RUNNING_CELL, + NOTEBOOK_HAS_SOMETHING_RUNNING, NOTEBOOK_INTERRUPTIBLE_KERNEL.toNegated(), ContextKeyExpr.notEquals('config.notebook.globalToolbar', true) ) @@ -526,7 +526,7 @@ registerAction2(class CancelAllNotebook extends CancelNotebook { order: -1, group: 'navigation/execute', when: ContextKeyExpr.and( - NOTEBOOK_HAS_RUNNING_CELL, + NOTEBOOK_HAS_SOMETHING_RUNNING, NOTEBOOK_INTERRUPTIBLE_KERNEL.toNegated(), ContextKeyExpr.equals('config.notebook.globalToolbar', true) ) @@ -552,7 +552,7 @@ registerAction2(class InterruptNotebook extends CancelNotebook { group: 'navigation', when: ContextKeyExpr.and( NOTEBOOK_IS_ACTIVE_EDITOR, - NOTEBOOK_HAS_RUNNING_CELL, + NOTEBOOK_HAS_SOMETHING_RUNNING, NOTEBOOK_INTERRUPTIBLE_KERNEL, ContextKeyExpr.notEquals('config.notebook.globalToolbar', true) ) @@ -562,7 +562,7 @@ registerAction2(class InterruptNotebook extends CancelNotebook { order: -1, group: 'navigation/execute', when: ContextKeyExpr.and( - NOTEBOOK_HAS_RUNNING_CELL, + NOTEBOOK_HAS_SOMETHING_RUNNING, NOTEBOOK_INTERRUPTIBLE_KERNEL, ContextKeyExpr.equals('config.notebook.globalToolbar', true) ) @@ -570,7 +570,7 @@ registerAction2(class InterruptNotebook extends CancelNotebook { { id: MenuId.InteractiveToolbar, when: ContextKeyExpr.and( - NOTEBOOK_HAS_RUNNING_CELL, + NOTEBOOK_HAS_SOMETHING_RUNNING, NOTEBOOK_INTERRUPTIBLE_KERNEL, ContextKeyExpr.equals('resourceScheme', Schemas.vscodeInteractive) ), diff --git a/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts index a8f305f63b33d..b5c039dcd34ad 100644 --- a/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts @@ -14,9 +14,9 @@ import { AudioCue, IAudioCueService } from 'vs/platform/audioCues/browser/audioC import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { ILogService } from 'vs/platform/log/common/log'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; -import { CellEditType, CellUri, ICellEditOperation, NotebookCellExecutionState, NotebookCellInternalMetadata, NotebookTextModelWillAddRemoveEvent } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellEditType, CellUri, ICellEditOperation, NotebookCellExecutionState, NotebookCellInternalMetadata, NotebookExecutionState, NotebookTextModelWillAddRemoveEvent } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { CellExecutionUpdateType, INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; -import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionStateChangedEvent, ICellExecutionStateUpdate, IFailedCellInfo, INotebookCellExecution, INotebookExecutionStateService, INotebookFailStateChangedEvent } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionStateChangedEvent, ICellExecutionStateUpdate, IExecutionStateChangedEvent, IFailedCellInfo, INotebookCellExecution, INotebookExecution, INotebookExecutionStateService, INotebookFailStateChangedEvent } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; @@ -24,6 +24,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo declare _serviceBrand: undefined; private readonly _executions = new ResourceMap>(); + private readonly _notebookExecutions = new ResourceMap<[NotebookExecution, IDisposable]>(); private readonly _notebookListeners = new ResourceMap(); private readonly _cellListeners = new ResourceMap(); private readonly _lastFailedCells = new ResourceMap(); @@ -31,6 +32,9 @@ export class NotebookExecutionStateService extends Disposable implements INotebo private readonly _onDidChangeCellExecution = this._register(new Emitter()); onDidChangeCellExecution = this._onDidChangeCellExecution.event; + private readonly _onDidChangeExecution = this._register(new Emitter()); + onDidChangeExecution = this._onDidChangeExecution.event; + private readonly _onDidChangeLastRunFailState = this._register(new Emitter()); onDidChangeLastRunFailState = this._onDidChangeLastRunFailState.event; @@ -49,13 +53,14 @@ export class NotebookExecutionStateService extends Disposable implements INotebo } forceCancelNotebookExecutions(notebookUri: URI): void { - const notebookExecutions = this._executions.get(notebookUri); - if (!notebookExecutions) { - return; + const notebookCellExecutions = this._executions.get(notebookUri); + if (notebookCellExecutions) { + for (const exe of notebookCellExecutions.values()) { + this._onCellExecutionDidComplete(notebookUri, exe.cellHandle, exe); + } } - - for (const exe of notebookExecutions.values()) { - this._onCellExecutionDidComplete(notebookUri, exe.cellHandle, exe); + if (this._notebookExecutions.has(notebookUri)) { + this._onExecutionDidComplete(notebookUri); } } @@ -72,6 +77,9 @@ export class NotebookExecutionStateService extends Disposable implements INotebo return undefined; } + getExecution(notebook: URI): INotebookExecution | undefined { + return this._notebookExecutions.get(notebook)?.[0]; + } getCellExecutionsForNotebook(notebook: URI): INotebookCellExecution[] { const exeMap = this._executions.get(notebook); @@ -84,7 +92,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo } private _onCellExecutionDidChange(notebookUri: URI, cellHandle: number, exe: CellExecution): void { - this._onDidChangeCellExecution.fire(new NotebookExecutionEvent(notebookUri, cellHandle, exe)); + this._onDidChangeCellExecution.fire(new NotebookCellExecutionEvent(notebookUri, cellHandle, exe)); } private _onCellExecutionDidComplete(notebookUri: URI, cellHandle: number, exe: CellExecution, lastRunSuccess?: boolean): void { @@ -117,7 +125,23 @@ export class NotebookExecutionStateService extends Disposable implements INotebo } } - this._onDidChangeCellExecution.fire(new NotebookExecutionEvent(notebookUri, cellHandle)); + this._onDidChangeCellExecution.fire(new NotebookCellExecutionEvent(notebookUri, cellHandle)); + } + + private _onExecutionDidChange(notebookUri: URI, exe: NotebookExecution): void { + this._onDidChangeExecution.fire(new NotebookExecutionEvent(notebookUri, exe)); + } + + private _onExecutionDidComplete(notebookUri: URI): void { + const disposables = this._notebookExecutions.get(notebookUri); + if (!Array.isArray(disposables)) { + this._logService.debug(`NotebookExecutionStateService#_onCellExecutionDidComplete - unknown notebook ${notebookUri.toString()}`); + return; + } + + disposables.forEach(d => d.dispose()); + this._notebookExecutions.delete(notebookUri); + this._onDidChangeExecution.fire(new NotebookExecutionEvent(notebookUri)); } createCellExecution(notebookUri: URI, cellHandle: number): INotebookCellExecution { @@ -140,11 +164,32 @@ export class NotebookExecutionStateService extends Disposable implements INotebo exe = this._createNotebookCellExecution(notebook, cellHandle); notebookExecutionMap.set(cellHandle, exe); exe.initialize(); - this._onDidChangeCellExecution.fire(new NotebookExecutionEvent(notebookUri, cellHandle, exe)); + this._onDidChangeCellExecution.fire(new NotebookCellExecutionEvent(notebookUri, cellHandle, exe)); } return exe; } + createExecution(notebookUri: URI): INotebookExecution { + const notebook = this._notebookService.getNotebookTextModel(notebookUri); + if (!notebook) { + throw new Error(`Notebook not found: ${notebookUri.toString()}`); + } + + if (!this._notebookListeners.has(notebookUri)) { + const listeners = this._instantiationService.createInstance(NotebookExecutionListeners, notebookUri); + this._notebookListeners.set(notebookUri, listeners); + } + + let info = this._notebookExecutions.get(notebookUri); + if (!info) { + info = this._createNotebookExecution(notebook); + this._notebookExecutions.set(notebookUri, info); + // exe.initialize(); + this._onDidChangeExecution.fire(new NotebookExecutionEvent(notebookUri, info[0])); + } + + return info[0]; + } private _createNotebookCellExecution(notebook: NotebookTextModel, cellHandle: number): CellExecution { const notebookUri = notebook.uri; @@ -157,6 +202,15 @@ export class NotebookExecutionStateService extends Disposable implements INotebo return exe; } + private _createNotebookExecution(notebook: NotebookTextModel): [NotebookExecution, IDisposable] { + const notebookUri = notebook.uri; + const exe: NotebookExecution = this._instantiationService.createInstance(NotebookExecution, notebook); + const disposable = combinedDisposable( + exe.onDidUpdate(() => this._onExecutionDidChange(notebookUri, exe)), + exe.onDidComplete(() => this._onExecutionDidComplete(notebookUri))); + return [exe, disposable]; + } + private _setLastFailedCell(notebookURI: URI, cellHandle: number): void { const prevLastFailedCellInfo = this._lastFailedCells.get(notebookURI); const notebook = this._notebookService.getNotebookTextModel(notebookURI); @@ -235,7 +289,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo } } -class NotebookExecutionEvent implements ICellExecutionStateChangedEvent { +class NotebookCellExecutionEvent implements ICellExecutionStateChangedEvent { constructor( readonly notebook: URI, readonly cellHandle: number, @@ -252,6 +306,17 @@ class NotebookExecutionEvent implements ICellExecutionStateChangedEvent { } } +class NotebookExecutionEvent implements IExecutionStateChangedEvent { + constructor( + readonly notebook: URI, + readonly changed?: NotebookExecution + ) { } + + affectsNotebook(notebook: URI): boolean { + return isEqual(this.notebook, notebook); + } +} + class NotebookExecutionListeners extends Disposable { private readonly _notebookModel: NotebookTextModel; @@ -473,3 +538,49 @@ class CellExecution extends Disposable implements INotebookCellExecution { this._notebookModel.applyEdits(edits, true, undefined, () => undefined, undefined, false); } } + +class NotebookExecution extends Disposable implements INotebookExecution { + private readonly _onDidUpdate = this._register(new Emitter()); + readonly onDidUpdate = this._onDidUpdate.event; + + private readonly _onDidComplete = this._register(new Emitter()); + readonly onDidComplete = this._onDidComplete.event; + + private _state: NotebookExecutionState = NotebookExecutionState.Unconfirmed; + get state() { + return this._state; + } + + get notebook(): URI { + return this._notebookModel.uri; + } + + constructor( + private readonly _notebookModel: NotebookTextModel, + @ILogService private readonly _logService: ILogService, + ) { + super(); + this._logService.debug(`NotebookExecution#ctor`); + } + private debug(message: string) { + this._logService.debug(`${message} ${this._notebookModel.uri.toString()}`); + } + + confirm() { + this.debug(`Execution#confirm`); + this._state = NotebookExecutionState.Pending; + this._onDidUpdate.fire(); + } + + begin(): void { + this.debug(`Execution#begin`); + this._state = NotebookExecutionState.Executing; + this._onDidUpdate.fire(); + } + + complete(): void { + this.debug(`Execution#begin`); + this._state = NotebookExecutionState.Unconfirmed; + this._onDidComplete.fire(); + } +} diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts index 7ee5fba6af64d..88c7861f25027 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts @@ -7,7 +7,7 @@ import * as DOM from 'vs/base/browser/dom'; import { DisposableStore, dispose, IDisposable } from 'vs/base/common/lifecycle'; import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { ICellViewModel, INotebookEditorDelegate, KERNEL_EXTENSIONS } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { NOTEBOOK_CELL_TOOLBAR_LOCATION, NOTEBOOK_HAS_OUTPUTS, NOTEBOOK_HAS_RUNNING_CELL, NOTEBOOK_INTERRUPTIBLE_KERNEL, NOTEBOOK_KERNEL, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_KERNEL_SELECTED, NOTEBOOK_KERNEL_SOURCE_COUNT, NOTEBOOK_LAST_CELL_FAILED, NOTEBOOK_MISSING_KERNEL_EXTENSION, NOTEBOOK_USE_CONSOLIDATED_OUTPUT_BUTTON, NOTEBOOK_VIEW_TYPE } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; +import { NOTEBOOK_CELL_TOOLBAR_LOCATION, NOTEBOOK_HAS_OUTPUTS, NOTEBOOK_HAS_RUNNING_CELL, NOTEBOOK_HAS_SOMETHING_RUNNING, NOTEBOOK_INTERRUPTIBLE_KERNEL, NOTEBOOK_KERNEL, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_KERNEL_SELECTED, NOTEBOOK_KERNEL_SOURCE_COUNT, NOTEBOOK_LAST_CELL_FAILED, NOTEBOOK_MISSING_KERNEL_EXTENSION, NOTEBOOK_USE_CONSOLIDATED_OUTPUT_BUTTON, NOTEBOOK_VIEW_TYPE } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; import { INotebookExecutionStateService, INotebookFailStateChangedEvent } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; @@ -20,6 +20,7 @@ export class NotebookEditorContextKeys { private readonly _notebookKernelSelected: IContextKey; private readonly _interruptibleKernel: IContextKey; private readonly _someCellRunning: IContextKey; + private readonly _kernelRunning: IContextKey; private readonly _hasOutputs: IContextKey; private readonly _useConsolidatedOutputButton: IContextKey; private readonly _viewType!: IContextKey; @@ -44,6 +45,7 @@ export class NotebookEditorContextKeys { this._notebookKernelSelected = NOTEBOOK_KERNEL_SELECTED.bindTo(contextKeyService); this._interruptibleKernel = NOTEBOOK_INTERRUPTIBLE_KERNEL.bindTo(contextKeyService); this._someCellRunning = NOTEBOOK_HAS_RUNNING_CELL.bindTo(contextKeyService); + this._kernelRunning = NOTEBOOK_HAS_SOMETHING_RUNNING.bindTo(contextKeyService); this._useConsolidatedOutputButton = NOTEBOOK_USE_CONSOLIDATED_OUTPUT_BUTTON.bindTo(contextKeyService); this._hasOutputs = NOTEBOOK_HAS_OUTPUTS.bindTo(contextKeyService); this._viewType = NOTEBOOK_VIEW_TYPE.bindTo(contextKeyService); @@ -62,6 +64,7 @@ export class NotebookEditorContextKeys { this._disposables.add(_editor.notebookOptions.onDidChangeOptions(this._updateForNotebookOptions, this)); this._disposables.add(_extensionService.onDidChangeExtensions(this._updateForInstalledExtension, this)); this._disposables.add(_notebookExecutionStateService.onDidChangeCellExecution(this._updateForCellExecution, this)); + this._disposables.add(_notebookExecutionStateService.onDidChangeExecution(this._updateForExecution, this)); this._disposables.add(_notebookExecutionStateService.onDidChangeLastRunFailState(this._updateForLastRunFailState, this)); } @@ -72,6 +75,7 @@ export class NotebookEditorContextKeys { this._notebookKernelSourceCount.reset(); this._interruptibleKernel.reset(); this._someCellRunning.reset(); + this._kernelRunning.reset(); this._viewType.reset(); dispose(this._cellOutputsListeners); this._cellOutputsListeners.length = 0; @@ -136,10 +140,21 @@ export class NotebookEditorContextKeys { private _updateForCellExecution(): void { if (this._editor.textModel) { - const notebookExe = this._notebookExecutionStateService.getCellExecutionsForNotebook(this._editor.textModel.uri); - this._someCellRunning.set(notebookExe.length > 0); + const notebookCellExe = this._notebookExecutionStateService.getCellExecutionsForNotebook(this._editor.textModel.uri); + const notebookExe = this._notebookExecutionStateService.getExecution(this._editor.textModel.uri); + this._someCellRunning.set(notebookCellExe.length > 0 || !!notebookExe); } else { this._someCellRunning.set(false); + this._kernelRunning.set(false); + } + } + + private _updateForExecution(): void { + if (this._editor.textModel) { + const notebookExe = this._notebookExecutionStateService.getExecution(this._editor.textModel.uri); + this._kernelRunning.set(!!notebookExe); + } else { + this._kernelRunning.set(false); } } diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 2c7728eddb977..66cf285b0a41d 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -91,6 +91,11 @@ export enum NotebookCellExecutionState { Pending = 2, Executing = 3 } +export enum NotebookExecutionState { + Unconfirmed = 1, + Pending = 2, + Executing = 3 +} export interface INotebookCellPreviousExecutionResult { executionOrder?: number; diff --git a/src/vs/workbench/contrib/notebook/common/notebookContextKeys.ts b/src/vs/workbench/contrib/notebook/common/notebookContextKeys.ts index 9709d55099ca9..4906dff78ed69 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookContextKeys.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookContextKeys.ts @@ -22,6 +22,7 @@ export const NOTEBOOK_CELL_LIST_FOCUSED = new RawContextKey('notebookCe export const NOTEBOOK_OUTPUT_FOCUSED = new RawContextKey('notebookOutputFocused', false); export const NOTEBOOK_EDITOR_EDITABLE = new RawContextKey('notebookEditable', true); export const NOTEBOOK_HAS_RUNNING_CELL = new RawContextKey('notebookHasRunningCell', false); +export const NOTEBOOK_HAS_SOMETHING_RUNNING = new RawContextKey('notebookHasSomethingRunning', false); export const NOTEBOOK_USE_CONSOLIDATED_OUTPUT_BUTTON = new RawContextKey('notebookUseConsolidatedOutputButton', false); export const NOTEBOOK_BREAKPOINT_MARGIN_ACTIVE = new RawContextKey('notebookBreakpointMargin', false); export const NOTEBOOK_CELL_TOOLBAR_LOCATION = new RawContextKey<'left' | 'right' | 'hidden'>('notebookCellToolbarLocation', 'left'); diff --git a/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts b/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts index 5d6e2b5134ee9..0f4b5d4ccdf60 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts @@ -7,7 +7,7 @@ import { Event } from 'vs/base/common/event'; import { IDisposable } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; -import { NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { NotebookCellExecutionState, NotebookExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { CellExecutionUpdateType, ICellExecuteOutputEdit, ICellExecuteOutputItemEdit } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; export type ICellExecuteUpdate = ICellExecuteOutputEdit | ICellExecuteOutputItemEdit | ICellExecutionStateUpdate; @@ -32,6 +32,11 @@ export interface ICellExecutionStateChangedEvent { affectsCell(cell: URI): boolean; affectsNotebook(notebook: URI): boolean; } +export interface IExecutionStateChangedEvent { + notebook: URI; + changed?: INotebookExecution; // undefined -> execution was completed + affectsNotebook(notebook: URI): boolean; +} export interface INotebookFailStateChangedEvent { visible: boolean; notebook: URI; @@ -48,6 +53,7 @@ export const INotebookExecutionStateService = createDecorator; onDidChangeCellExecution: Event; onDidChangeLastRunFailState: Event; @@ -56,6 +62,8 @@ export interface INotebookExecutionStateService { getCellExecutionsByHandleForNotebook(notebook: URI): Map | undefined; getCellExecution(cellUri: URI): INotebookCellExecution | undefined; createCellExecution(notebook: URI, cellHandle: number): INotebookCellExecution; + getExecution(notebook: URI): INotebookExecution | undefined; + createExecution(notebook: URI): INotebookExecution; getLastFailedCellForNotebook(notebook: URI): number | undefined; } @@ -70,3 +78,11 @@ export interface INotebookCellExecution { update(updates: ICellExecuteUpdate[]): void; complete(complete: ICellExecutionComplete): void; } +export interface INotebookExecution { + readonly notebook: URI; + readonly state: NotebookExecutionState; + + confirm(): void; + begin(): void; + complete(): void; +} diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts index dd524a0eda05b..7c38132870e0c 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts @@ -188,16 +188,18 @@ suite('NotebookExecutionStateService', () => { const deferred = new DeferredPromise(); disposables.add(executionStateService.onDidChangeCellExecution(e => { - const cellUri = CellUri.generate(e.notebook, e.cellHandle); - const exe = executionStateService.getCellExecution(cellUri); - assert.ok(exe); - assert.strictEqual(e.notebook.toString(), exe.notebook.toString()); - assert.strictEqual(e.cellHandle, exe.cellHandle); + if (typeof e.cellHandle === 'number') { + const cellUri = CellUri.generate(e.notebook, e.cellHandle); + const exe = executionStateService.getCellExecution(cellUri); + assert.ok(exe); + assert.strictEqual(e.notebook.toString(), exe.notebook.toString()); + assert.strictEqual(e.cellHandle, exe.cellHandle); - assert.strictEqual(exe.notebook.toString(), e.changed?.notebook.toString()); - assert.strictEqual(exe.cellHandle, e.changed?.cellHandle); + assert.strictEqual(exe.notebook.toString(), e.changed?.notebook.toString()); + assert.strictEqual(exe.cellHandle, e.changed?.cellHandle); - deferred.complete(); + deferred.complete(); + } })); executionStateService.createCellExecution(viewModel.uri, cell.handle); diff --git a/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts b/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts index 620df78ee00d5..f2e33d6c5c950 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts @@ -53,7 +53,7 @@ import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/mode import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { INotebookCellStatusBarService } from 'vs/workbench/contrib/notebook/common/notebookCellStatusBarService'; import { CellKind, CellUri, ICellDto2, INotebookDiffEditorModel, INotebookEditorModel, INotebookSearchOptions, IOutputDto, IResolvedNotebookEditorModel, NotebookCellExecutionState, NotebookCellMetadata, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionStateChangedEvent, INotebookCellExecution, INotebookExecutionStateService, INotebookFailStateChangedEvent } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionStateChangedEvent, IExecutionStateChangedEvent, INotebookCellExecution, INotebookExecution, INotebookExecutionStateService, INotebookFailStateChangedEvent } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { NotebookOptions } from 'vs/workbench/contrib/notebook/browser/notebookOptions'; import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange'; import { TextModelResolverService } from 'vs/workbench/services/textmodelResolver/common/textModelResolverService'; @@ -420,6 +420,7 @@ class TestNotebookExecutionStateService implements INotebookExecutionStateServic private _executions = new ResourceMap(); onDidChangeCellExecution = new Emitter().event; + onDidChangeExecution = new Emitter().event; onDidChangeLastRunFailState = new Emitter().event; forceCancelNotebookExecutions(notebookUri: URI): void { @@ -447,4 +448,10 @@ class TestNotebookExecutionStateService implements INotebookExecutionStateServic getLastFailedCellForNotebook(notebook: URI): number | undefined { return; } + getExecution(notebook: URI): INotebookExecution | undefined { + return; + } + createExecution(notebook: URI): INotebookExecution { + throw new Error('Method not implemented.'); + } } diff --git a/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts b/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts index 5860ada41b230..c2f7ff775cf20 100644 --- a/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts +++ b/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts @@ -48,6 +48,8 @@ export const allApiProposals = Object.freeze({ notebookCellExecutionState: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookCellExecutionState.d.ts', notebookControllerAffinityHidden: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookControllerAffinityHidden.d.ts', notebookDeprecated: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookDeprecated.d.ts', + notebookDocumentWillSave: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookDocumentWillSave.d.ts', + notebookExecution: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookExecution.d.ts', notebookKernelSource: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookKernelSource.d.ts', notebookLiveShare: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookLiveShare.d.ts', notebookMessaging: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookMessaging.d.ts', diff --git a/src/vscode-dts/vscode.proposed.notebookExecution.d.ts b/src/vscode-dts/vscode.proposed.notebookExecution.d.ts new file mode 100644 index 0000000000000..70322756d2624 --- /dev/null +++ b/src/vscode-dts/vscode.proposed.notebookExecution.d.ts @@ -0,0 +1,48 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +declare module 'vscode' { + + /** + * A NotebookExecution is how {@link NotebookController notebook controller} can indicate whether the notebook controller is busy or not. + * + * When {@linkcode NotebookExecution.start start()} is called on the execution task, it causes the Notebook to enter a executing state . + * When {@linkcode NotebookExecution.end end()} is called, it enters the idle state. + */ + export interface NotebookExecution { + /** + * A cancellation token which will be triggered when the execution is canceled + * from the UI. + * + * _Note_ that the cancellation token will not be triggered when the {@link NotebookController controller} + * that created this execution uses an {@link NotebookController.interruptHandler interrupt-handler}. + */ + readonly token: CancellationToken; + + /** + * Signal that the execution has begun. + */ + start(): void; + + /** + * Signal that execution has ended. + */ + end(): void; + } + + export interface NotebookController { + /** + * Create an execution task. + * + * _Note_ that there can only be one execution per Notebook, that also includes NotebookCellExecutions and t an error is thrown if + * a cell execution or another NotebookExecution is created while another is still active. + * + * This should be used to indicate the {@link NotebookController notebook controller} is busy even though user may not have executed any cell though the UI. + * @param {NotebookDocument} notebook + * @returns A notebook execution. + */ + createNotebookExecution(notebook: NotebookDocument): NotebookExecution; + } +} From 3ffa5a52e77a47f333bcfbe61a702de902361ebe Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 5 Apr 2023 22:51:04 +1000 Subject: [PATCH 2/8] oops --- .../notebookExecutionStateService.test.ts | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts index 7c38132870e0c..dd524a0eda05b 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts @@ -188,18 +188,16 @@ suite('NotebookExecutionStateService', () => { const deferred = new DeferredPromise(); disposables.add(executionStateService.onDidChangeCellExecution(e => { - if (typeof e.cellHandle === 'number') { - const cellUri = CellUri.generate(e.notebook, e.cellHandle); - const exe = executionStateService.getCellExecution(cellUri); - assert.ok(exe); - assert.strictEqual(e.notebook.toString(), exe.notebook.toString()); - assert.strictEqual(e.cellHandle, exe.cellHandle); + const cellUri = CellUri.generate(e.notebook, e.cellHandle); + const exe = executionStateService.getCellExecution(cellUri); + assert.ok(exe); + assert.strictEqual(e.notebook.toString(), exe.notebook.toString()); + assert.strictEqual(e.cellHandle, exe.cellHandle); - assert.strictEqual(exe.notebook.toString(), e.changed?.notebook.toString()); - assert.strictEqual(exe.cellHandle, e.changed?.cellHandle); + assert.strictEqual(exe.notebook.toString(), e.changed?.notebook.toString()); + assert.strictEqual(exe.cellHandle, e.changed?.cellHandle); - deferred.complete(); - } + deferred.complete(); })); executionStateService.createCellExecution(viewModel.uri, cell.handle); From 385d7cae483b5eb2d533f21f5544fb49c3dfe2f2 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 5 Apr 2023 22:52:34 +1000 Subject: [PATCH 3/8] Misc --- src/vs/workbench/api/common/extHostNotebookKernels.ts | 2 -- src/vscode-dts/vscode.proposed.notebookExecution.d.ts | 9 --------- 2 files changed, 11 deletions(-) diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index a35109582e980..f705e114dd389 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -699,9 +699,7 @@ class NotebookExecutionTask extends Disposable { this._tokenSource.cancel(); } asApiObject(): vscode.NotebookExecution { - const token = this._tokenSource.token; const result: vscode.NotebookExecution = { - get token() { return token; }, start: () => { if (this._state === NotebookExecutionTaskState.Resolved || this._state === NotebookExecutionTaskState.Started) { throw new Error('Cannot call start again'); diff --git a/src/vscode-dts/vscode.proposed.notebookExecution.d.ts b/src/vscode-dts/vscode.proposed.notebookExecution.d.ts index 70322756d2624..59f9174768fa2 100644 --- a/src/vscode-dts/vscode.proposed.notebookExecution.d.ts +++ b/src/vscode-dts/vscode.proposed.notebookExecution.d.ts @@ -12,15 +12,6 @@ declare module 'vscode' { * When {@linkcode NotebookExecution.end end()} is called, it enters the idle state. */ export interface NotebookExecution { - /** - * A cancellation token which will be triggered when the execution is canceled - * from the UI. - * - * _Note_ that the cancellation token will not be triggered when the {@link NotebookController controller} - * that created this execution uses an {@link NotebookController.interruptHandler interrupt-handler}. - */ - readonly token: CancellationToken; - /** * Signal that the execution has begun. */ From f8a20e7c36c1b7aaf541f4822fa5d7d9ad93ab92 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 6 Apr 2023 05:34:23 +1000 Subject: [PATCH 4/8] Unify events into one --- .../api/browser/mainThreadNotebookKernels.ts | 6 ++-- .../interactive/browser/interactiveEditor.ts | 4 +-- .../executionStatusBarItemController.ts | 4 +-- .../contrib/debug/notebookDebugDecorations.ts | 4 +-- .../execute/executionEditorProgress.ts | 4 +-- .../contrib/outline/notebookOutline.ts | 4 +-- .../notebookExecutionStateServiceImpl.ts | 13 ++++--- .../browser/view/cellParts/cellContextKeys.ts | 4 +-- .../view/cellParts/codeCellExecutionIcon.ts | 4 +-- .../viewModel/notebookViewModelImpl.ts | 5 ++- .../notebookEditorWidgetContextKeys.ts | 24 +++++-------- .../common/notebookExecutionStateService.ts | 5 +-- .../notebookExecutionStateService.test.ts | 36 +++++++++++-------- .../test/browser/testNotebookEditor.ts | 3 +- 14 files changed, 62 insertions(+), 58 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts index 2af1912229380..65b1087b0208c 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts @@ -138,8 +138,10 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape this._notebookExecutions.forEach(e => e.complete()); })); - this._disposables.add(this._notebookExecutionStateService.onDidChangeCellExecution(e => { - this._proxy.$cellExecutionChanged(e.notebook, e.cellHandle, e.changed?.state); + this._disposables.add(this._notebookExecutionStateService.onDidChangeExecution(e => { + if (e.type === 'cell') { + this._proxy.$cellExecutionChanged(e.notebook, e.cellHandle, e.changed?.state); + } })); } diff --git a/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts b/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts index aa34619c6fc7b..1620b31ab6088 100644 --- a/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts +++ b/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts @@ -153,8 +153,8 @@ export class InteractiveEditor extends EditorPane { codeEditorService.registerDecorationType('interactive-decoration', DECORATION_KEY, {}); this._register(this.#keybindingService.onDidUpdateKeybindings(this.#updateInputDecoration, this)); - this._register(this.#notebookExecutionStateService.onDidChangeCellExecution((e) => { - if (isEqual(e.notebook, this.#notebookWidget.value?.viewModel?.notebookDocument.uri)) { + this._register(this.#notebookExecutionStateService.onDidChangeExecution((e) => { + if (e.type === 'cell' && isEqual(e.notebook, this.#notebookWidget.value?.viewModel?.notebookDocument.uri)) { const cell = this.#notebookWidget.value?.getCellByHandle(e.cellHandle); if (cell && e.changed?.state) { this.#scrollIfNecessary(cell); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts b/src/vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts index 342d96f17dd2a..804b9942bac8e 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts @@ -112,8 +112,8 @@ class ExecutionStateCellStatusBarItem extends Disposable { super(); this._update(); - this._register(this._executionStateService.onDidChangeCellExecution(e => { - if (e.affectsCell(this._cell.uri)) { + this._register(this._executionStateService.onDidChangeExecution(e => { + if (e.type === 'cell' && e.affectsCell(this._cell.uri)) { this._update(); } })); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/debug/notebookDebugDecorations.ts b/src/vs/workbench/contrib/notebook/browser/contrib/debug/notebookDebugDecorations.ts index 7676fe8766d7b..931430ca6065f 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/debug/notebookDebugDecorations.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/debug/notebookDebugDecorations.ts @@ -36,8 +36,8 @@ export class PausedCellDecorationContribution extends Disposable implements INot this._register(_debugService.getModel().onDidChangeCallStack(() => this.updateExecutionDecorations())); this._register(_debugService.getViewModel().onDidFocusStackFrame(() => this.updateExecutionDecorations())); - this._register(_notebookExecutionStateService.onDidChangeCellExecution(e => { - if (this._notebookEditor.textModel && e.affectsNotebook(this._notebookEditor.textModel.uri)) { + this._register(_notebookExecutionStateService.onDidChangeExecution(e => { + if (e.type === 'cell' && this._notebookEditor.textModel && e.affectsNotebook(this._notebookEditor.textModel.uri)) { this.updateExecutionDecorations(); } })); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/execute/executionEditorProgress.ts b/src/vs/workbench/contrib/notebook/browser/contrib/execute/executionEditorProgress.ts index 7479267b464f8..6d23520e2efda 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/execute/executionEditorProgress.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/execute/executionEditorProgress.ts @@ -21,8 +21,8 @@ export class ExecutionEditorProgressController extends Disposable implements INo this._register(_notebookEditor.onDidScroll(() => this._update())); - this._register(_notebookExecutionStateService.onDidChangeCellExecution(e => { - if (e.notebook.toString() !== this._notebookEditor.textModel?.uri.toString()) { + this._register(_notebookExecutionStateService.onDidChangeExecution(e => { + if (e.type === 'cell' && e.notebook.toString() !== this._notebookEditor.textModel?.uri.toString()) { return; } diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts b/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts index 6e3b8b2c1ad11..d2dcbdda92232 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts @@ -353,8 +353,8 @@ export class NotebookCellOutline implements IOutline { this._onDidChange.fire({}); })); - this._dispoables.add(_notebookExecutionStateService.onDidChangeCellExecution(e => { - if (!!this._editor.textModel && e.affectsNotebook(this._editor.textModel?.uri)) { + this._dispoables.add(_notebookExecutionStateService.onDidChangeExecution(e => { + if (e.type === 'cell' && !!this._editor.textModel && e.affectsNotebook(this._editor.textModel?.uri)) { this._recomputeState(); } })); diff --git a/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts index b5c039dcd34ad..e9a1ea438ecdd 100644 --- a/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts @@ -29,10 +29,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo private readonly _cellListeners = new ResourceMap(); private readonly _lastFailedCells = new ResourceMap(); - private readonly _onDidChangeCellExecution = this._register(new Emitter()); - onDidChangeCellExecution = this._onDidChangeCellExecution.event; - - private readonly _onDidChangeExecution = this._register(new Emitter()); + private readonly _onDidChangeExecution = this._register(new Emitter()); onDidChangeExecution = this._onDidChangeExecution.event; private readonly _onDidChangeLastRunFailState = this._register(new Emitter()); @@ -92,7 +89,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo } private _onCellExecutionDidChange(notebookUri: URI, cellHandle: number, exe: CellExecution): void { - this._onDidChangeCellExecution.fire(new NotebookCellExecutionEvent(notebookUri, cellHandle, exe)); + this._onDidChangeExecution.fire(new NotebookCellExecutionEvent(notebookUri, cellHandle, exe)); } private _onCellExecutionDidComplete(notebookUri: URI, cellHandle: number, exe: CellExecution, lastRunSuccess?: boolean): void { @@ -125,7 +122,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo } } - this._onDidChangeCellExecution.fire(new NotebookCellExecutionEvent(notebookUri, cellHandle)); + this._onDidChangeExecution.fire(new NotebookCellExecutionEvent(notebookUri, cellHandle)); } private _onExecutionDidChange(notebookUri: URI, exe: NotebookExecution): void { @@ -164,7 +161,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo exe = this._createNotebookCellExecution(notebook, cellHandle); notebookExecutionMap.set(cellHandle, exe); exe.initialize(); - this._onDidChangeCellExecution.fire(new NotebookCellExecutionEvent(notebookUri, cellHandle, exe)); + this._onDidChangeExecution.fire(new NotebookCellExecutionEvent(notebookUri, cellHandle, exe)); } return exe; @@ -290,6 +287,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo } class NotebookCellExecutionEvent implements ICellExecutionStateChangedEvent { + readonly type = 'cell'; constructor( readonly notebook: URI, readonly cellHandle: number, @@ -307,6 +305,7 @@ class NotebookCellExecutionEvent implements ICellExecutionStateChangedEvent { } class NotebookExecutionEvent implements IExecutionStateChangedEvent { + readonly type = 'notebook'; constructor( readonly notebook: URI, readonly changed?: NotebookExecution diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellContextKeys.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellContextKeys.ts index 15f224725ffdf..bc4dbf40b43cf 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellContextKeys.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellContextKeys.ts @@ -77,8 +77,8 @@ export class CellContextKeyManager extends Disposable { } }); - this._register(this._notebookExecutionStateService.onDidChangeCellExecution(e => { - if (this.element && e.affectsCell(this.element.uri)) { + this._register(this._notebookExecutionStateService.onDidChangeExecution(e => { + if (e.type === 'cell' && this.element && e.affectsCell(this.element.uri)) { this.updateForExecutionState(); } })); diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCellExecutionIcon.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCellExecutionIcon.ts index a997fc93e8a2b..4ab2f9b4ac8b5 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCellExecutionIcon.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCellExecutionIcon.ts @@ -30,8 +30,8 @@ export class CollapsedCodeCellExecutionIcon extends Disposable { super(); this._update(); - this._register(this._executionStateService.onDidChangeCellExecution(e => { - if (e.affectsCell(this._cell.uri)) { + this._register(this._executionStateService.onDidChangeExecution(e => { + if (e.type === 'cell' && e.affectsCell(this._cell.uri)) { this._update(); } })); diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts index d214e5793c737..5e2d32477f247 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts @@ -320,7 +320,10 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD } })); - this._register(notebookExecutionStateService.onDidChangeCellExecution(e => { + this._register(notebookExecutionStateService.onDidChangeExecution(e => { + if (e.type !== 'cell') { + return; + } const cell = this.getCellByHandle(e.cellHandle); if (cell instanceof CodeCellViewModel) { diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts index 88c7861f25027..fb15b7e557651 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts @@ -8,7 +8,7 @@ import { DisposableStore, dispose, IDisposable } from 'vs/base/common/lifecycle' import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { ICellViewModel, INotebookEditorDelegate, KERNEL_EXTENSIONS } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { NOTEBOOK_CELL_TOOLBAR_LOCATION, NOTEBOOK_HAS_OUTPUTS, NOTEBOOK_HAS_RUNNING_CELL, NOTEBOOK_HAS_SOMETHING_RUNNING, NOTEBOOK_INTERRUPTIBLE_KERNEL, NOTEBOOK_KERNEL, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_KERNEL_SELECTED, NOTEBOOK_KERNEL_SOURCE_COUNT, NOTEBOOK_LAST_CELL_FAILED, NOTEBOOK_MISSING_KERNEL_EXTENSION, NOTEBOOK_USE_CONSOLIDATED_OUTPUT_BUTTON, NOTEBOOK_VIEW_TYPE } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; -import { INotebookExecutionStateService, INotebookFailStateChangedEvent } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { ICellExecutionStateChangedEvent, IExecutionStateChangedEvent, INotebookExecutionStateService, INotebookFailStateChangedEvent } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; @@ -63,7 +63,6 @@ export class NotebookEditorContextKeys { this._disposables.add(_notebookKernelService.onDidChangeSourceActions(this._updateKernelContext, this)); this._disposables.add(_editor.notebookOptions.onDidChangeOptions(this._updateForNotebookOptions, this)); this._disposables.add(_extensionService.onDidChangeExtensions(this._updateForInstalledExtension, this)); - this._disposables.add(_notebookExecutionStateService.onDidChangeCellExecution(this._updateForCellExecution, this)); this._disposables.add(_notebookExecutionStateService.onDidChangeExecution(this._updateForExecution, this)); this._disposables.add(_notebookExecutionStateService.onDidChangeLastRunFailState(this._updateForLastRunFailState, this)); } @@ -137,24 +136,19 @@ export class NotebookEditorContextKeys { })); this._viewType.set(this._editor.textModel.viewType); } - - private _updateForCellExecution(): void { - if (this._editor.textModel) { - const notebookCellExe = this._notebookExecutionStateService.getCellExecutionsForNotebook(this._editor.textModel.uri); - const notebookExe = this._notebookExecutionStateService.getExecution(this._editor.textModel.uri); - this._someCellRunning.set(notebookCellExe.length > 0 || !!notebookExe); - } else { - this._someCellRunning.set(false); - this._kernelRunning.set(false); - } - } - - private _updateForExecution(): void { + private _updateForExecution(e: ICellExecutionStateChangedEvent | IExecutionStateChangedEvent): void { if (this._editor.textModel) { const notebookExe = this._notebookExecutionStateService.getExecution(this._editor.textModel.uri); this._kernelRunning.set(!!notebookExe); + if (e.type === 'cell') { + const notebookCellExe = this._notebookExecutionStateService.getCellExecutionsForNotebook(this._editor.textModel.uri); + this._someCellRunning.set(notebookCellExe.length > 0 || !!notebookExe); + } } else { this._kernelRunning.set(false); + if (e.type === 'cell') { + this._someCellRunning.set(false); + } } } diff --git a/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts b/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts index 0f4b5d4ccdf60..0ba4af628f33e 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts @@ -26,6 +26,7 @@ export interface ICellExecutionComplete { } export interface ICellExecutionStateChangedEvent { + type: 'cell'; notebook: URI; cellHandle: number; changed?: INotebookCellExecution; // undefined -> execution was completed @@ -33,6 +34,7 @@ export interface ICellExecutionStateChangedEvent { affectsNotebook(notebook: URI): boolean; } export interface IExecutionStateChangedEvent { + type: 'notebook'; notebook: URI; changed?: INotebookExecution; // undefined -> execution was completed affectsNotebook(notebook: URI): boolean; @@ -53,8 +55,7 @@ export const INotebookExecutionStateService = createDecorator; - onDidChangeCellExecution: Event; + onDidChangeExecution: Event; onDidChangeLastRunFailState: Event; forceCancelNotebookExecutions(notebookUri: URI): void; diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts index dd524a0eda05b..84f65a7face82 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts @@ -137,8 +137,10 @@ suite('NotebookExecutionStateService', () => { const exe = executionStateService.createCellExecution(viewModel.uri, cell.handle); let didFire = false; - disposables.add(executionStateService.onDidChangeCellExecution(e => { - didFire = !e.changed; + disposables.add(executionStateService.onDidChangeExecution(e => { + if (e.type === 'cell') { + didFire = !e.changed; + } })); viewModel.notebookDocument.applyEdits([{ @@ -162,8 +164,10 @@ suite('NotebookExecutionStateService', () => { const exe = executionStateService.createCellExecution(viewModel.uri, cell.handle); let didFire = false; - disposables.add(executionStateService.onDidChangeCellExecution(e => { - didFire = true; + disposables.add(executionStateService.onDidChangeExecution(e => { + if (e.type === 'cell') { + didFire = true; + } })); exe.update([{ editType: CellExecutionUpdateType.OutputItems, items: [], outputId: '1' }]); @@ -187,17 +191,19 @@ suite('NotebookExecutionStateService', () => { const cell = insertCellAtIndex(viewModel, 0, 'var c = 3', 'javascript', CellKind.Code, {}, [], true, true); const deferred = new DeferredPromise(); - disposables.add(executionStateService.onDidChangeCellExecution(e => { - const cellUri = CellUri.generate(e.notebook, e.cellHandle); - const exe = executionStateService.getCellExecution(cellUri); - assert.ok(exe); - assert.strictEqual(e.notebook.toString(), exe.notebook.toString()); - assert.strictEqual(e.cellHandle, exe.cellHandle); - - assert.strictEqual(exe.notebook.toString(), e.changed?.notebook.toString()); - assert.strictEqual(exe.cellHandle, e.changed?.cellHandle); - - deferred.complete(); + disposables.add(executionStateService.onDidChangeExecution(e => { + if (e.type === 'cell') { + const cellUri = CellUri.generate(e.notebook, e.cellHandle); + const exe = executionStateService.getCellExecution(cellUri); + assert.ok(exe); + assert.strictEqual(e.notebook.toString(), exe.notebook.toString()); + assert.strictEqual(e.cellHandle, exe.cellHandle); + + assert.strictEqual(exe.notebook.toString(), e.changed?.notebook.toString()); + assert.strictEqual(exe.cellHandle, e.changed?.cellHandle); + + deferred.complete(); + } })); executionStateService.createCellExecution(viewModel.uri, cell.handle); diff --git a/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts b/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts index f2e33d6c5c950..25506f54286d3 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts @@ -419,8 +419,7 @@ class TestNotebookExecutionStateService implements INotebookExecutionStateServic private _executions = new ResourceMap(); - onDidChangeCellExecution = new Emitter().event; - onDidChangeExecution = new Emitter().event; + onDidChangeExecution = new Emitter().event; onDidChangeLastRunFailState = new Emitter().event; forceCancelNotebookExecutions(notebookUri: URI): void { From 4046a6e7f85c14d15da0c70b5489c55257f8c667 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 6 Apr 2023 05:37:04 +1000 Subject: [PATCH 5/8] oops --- .../services/extensions/common/extensionsApiProposals.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts b/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts index c2f7ff775cf20..c59f8bbbb8216 100644 --- a/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts +++ b/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts @@ -48,7 +48,6 @@ export const allApiProposals = Object.freeze({ notebookCellExecutionState: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookCellExecutionState.d.ts', notebookControllerAffinityHidden: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookControllerAffinityHidden.d.ts', notebookDeprecated: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookDeprecated.d.ts', - notebookDocumentWillSave: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookDocumentWillSave.d.ts', notebookExecution: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookExecution.d.ts', notebookKernelSource: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookKernelSource.d.ts', notebookLiveShare: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookLiveShare.d.ts', From 055118ccb9b4d7abcbb27fc6a13cd9515723681d Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 6 Apr 2023 06:54:50 +1000 Subject: [PATCH 6/8] Fix context key --- .../browser/viewParts/notebookEditorWidgetContextKeys.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts index fb15b7e557651..6a090560295e3 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts @@ -139,10 +139,10 @@ export class NotebookEditorContextKeys { private _updateForExecution(e: ICellExecutionStateChangedEvent | IExecutionStateChangedEvent): void { if (this._editor.textModel) { const notebookExe = this._notebookExecutionStateService.getExecution(this._editor.textModel.uri); - this._kernelRunning.set(!!notebookExe); + const notebookCellExe = this._notebookExecutionStateService.getCellExecutionsForNotebook(this._editor.textModel.uri); + this._kernelRunning.set(notebookCellExe.length > 0 || !!notebookExe); if (e.type === 'cell') { - const notebookCellExe = this._notebookExecutionStateService.getCellExecutionsForNotebook(this._editor.textModel.uri); - this._someCellRunning.set(notebookCellExe.length > 0 || !!notebookExe); + this._someCellRunning.set(notebookCellExe.length > 0); } } else { this._kernelRunning.set(false); From b6d104df7ef2b8a6f6d2a5e1dbb12135af1805b9 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 6 Apr 2023 08:13:35 +1000 Subject: [PATCH 7/8] Resolve code review issues --- .../workbench/api/browser/mainThreadNotebookKernels.ts | 4 ++-- src/vs/workbench/api/common/extHostNotebookKernels.ts | 1 + .../contrib/interactive/browser/interactiveEditor.ts | 4 ++-- .../cellStatusBar/executionStatusBarItemController.ts | 4 ++-- .../browser/contrib/debug/notebookDebugDecorations.ts | 4 ++-- .../browser/contrib/execute/executionEditorProgress.ts | 9 ++++++--- .../browser/contrib/outline/notebookOutline.ts | 4 ++-- .../services/notebookExecutionStateServiceImpl.ts | 10 +++++++--- .../notebook/browser/view/cellParts/cellContextKeys.ts | 4 ++-- .../browser/view/cellParts/codeCellExecutionIcon.ts | 4 ++-- .../browser/viewModel/notebookViewModelImpl.ts | 4 ++-- .../viewParts/notebookEditorWidgetContextKeys.ts | 6 +++--- .../notebook/common/notebookExecutionStateService.ts | 9 ++++++--- .../test/browser/notebookExecutionStateService.test.ts | 8 ++++---- 14 files changed, 43 insertions(+), 32 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts index 65b1087b0208c..512e611c49956 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts @@ -15,7 +15,7 @@ import { NotebookDto } from 'vs/workbench/api/browser/mainThreadNotebookDto'; import { extHostNamedCustomer, IExtHostContext } from 'vs/workbench/services/extensions/common/extHostCustomers'; import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/services/notebookEditorService'; -import { INotebookCellExecution, INotebookExecution, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { INotebookCellExecution, INotebookExecution, INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { IKernelSourceActionProvider, INotebookKernel, INotebookKernelChangeEvent, INotebookKernelDetectionTask, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { SerializableObjectWithBuffers } from 'vs/workbench/services/extensions/common/proxyIdentifier'; import { ExtHostContext, ExtHostNotebookKernelsShape, ICellExecuteUpdateDto, ICellExecutionCompleteDto, INotebookKernelDto2, MainContext, MainThreadNotebookKernelsShape } from '../common/extHost.protocol'; @@ -139,7 +139,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape })); this._disposables.add(this._notebookExecutionStateService.onDidChangeExecution(e => { - if (e.type === 'cell') { + if (e.type === NotebookExecutionType.cell) { this._proxy.$cellExecutionChanged(e.notebook, e.cellHandle, e.changed?.state); } })); diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index f705e114dd389..efef57e08fd4e 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -221,6 +221,7 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { return that._createNotebookCellExecution(cell, createKernelId(extension.identifier, this.id)); }, createNotebookExecution(notebook) { + checkProposedApiEnabled(extension, 'notebookExecution'); if (isDisposed) { throw new Error('notebook controller is DISPOSED'); } diff --git a/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts b/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts index 1620b31ab6088..445f6c6a1a971 100644 --- a/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts +++ b/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts @@ -52,7 +52,7 @@ import { MarkerController } from 'vs/editor/contrib/gotoError/browser/gotoError' import { EditorInput } from 'vs/workbench/common/editor/editorInput'; import { ITextResourceConfigurationService } from 'vs/editor/common/services/textResourceConfiguration'; import { ITextEditorOptions, TextEditorSelectionSource } from 'vs/platform/editor/common/editor'; -import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { NOTEBOOK_KERNEL } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; import { ICursorPositionChangedEvent } from 'vs/editor/common/cursorEvents'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; @@ -154,7 +154,7 @@ export class InteractiveEditor extends EditorPane { codeEditorService.registerDecorationType('interactive-decoration', DECORATION_KEY, {}); this._register(this.#keybindingService.onDidUpdateKeybindings(this.#updateInputDecoration, this)); this._register(this.#notebookExecutionStateService.onDidChangeExecution((e) => { - if (e.type === 'cell' && isEqual(e.notebook, this.#notebookWidget.value?.viewModel?.notebookDocument.uri)) { + if (e.type === NotebookExecutionType.cell && isEqual(e.notebook, this.#notebookWidget.value?.viewModel?.notebookDocument.uri)) { const cell = this.#notebookWidget.value?.getCellByHandle(e.cellHandle); if (cell && e.changed?.state) { this.#scrollIfNecessary(cell); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts b/src/vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts index 804b9942bac8e..c0e9521b89727 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts @@ -16,7 +16,7 @@ import { registerNotebookContribution } from 'vs/workbench/contrib/notebook/brow import { cellStatusIconError, cellStatusIconSuccess } from 'vs/workbench/contrib/notebook/browser/notebookEditorWidget'; import { errorStateIcon, executingStateIcon, pendingStateIcon, successStateIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons'; import { CellStatusbarAlignment, INotebookCellStatusBarItem, NotebookCellExecutionState, NotebookCellInternalMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { INotebookCellExecution, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { INotebookCellExecution, INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { IMarkdownString } from 'vs/base/common/htmlContent'; @@ -113,7 +113,7 @@ class ExecutionStateCellStatusBarItem extends Disposable { this._update(); this._register(this._executionStateService.onDidChangeExecution(e => { - if (e.type === 'cell' && e.affectsCell(this._cell.uri)) { + if (e.type === NotebookExecutionType.cell && e.affectsCell(this._cell.uri)) { this._update(); } })); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/debug/notebookDebugDecorations.ts b/src/vs/workbench/contrib/notebook/browser/contrib/debug/notebookDebugDecorations.ts index 931430ca6065f..6039f34b71990 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/debug/notebookDebugDecorations.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/debug/notebookDebugDecorations.ts @@ -13,7 +13,7 @@ import { INotebookCellDecorationOptions, INotebookDeltaDecoration, INotebookEdit import { registerNotebookContribution } from 'vs/workbench/contrib/notebook/browser/notebookEditorExtensions'; import { runningCellRulerDecorationColor } from 'vs/workbench/contrib/notebook/browser/notebookEditorWidget'; import { CellUri, NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; interface ICellAndRange { handle: number; @@ -37,7 +37,7 @@ export class PausedCellDecorationContribution extends Disposable implements INot this._register(_debugService.getModel().onDidChangeCallStack(() => this.updateExecutionDecorations())); this._register(_debugService.getViewModel().onDidFocusStackFrame(() => this.updateExecutionDecorations())); this._register(_notebookExecutionStateService.onDidChangeExecution(e => { - if (e.type === 'cell' && this._notebookEditor.textModel && e.affectsNotebook(this._notebookEditor.textModel.uri)) { + if (e.type === NotebookExecutionType.cell && this._notebookEditor.textModel && e.affectsNotebook(this._notebookEditor.textModel.uri)) { this.updateExecutionDecorations(); } })); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/execute/executionEditorProgress.ts b/src/vs/workbench/contrib/notebook/browser/contrib/execute/executionEditorProgress.ts index 6d23520e2efda..aad5a774045ba 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/execute/executionEditorProgress.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/execute/executionEditorProgress.ts @@ -22,7 +22,7 @@ export class ExecutionEditorProgressController extends Disposable implements INo this._register(_notebookEditor.onDidScroll(() => this._update())); this._register(_notebookExecutionStateService.onDidChangeExecution(e => { - if (e.type === 'cell' && e.notebook.toString() !== this._notebookEditor.textModel?.uri.toString()) { + if (e.notebook.toString() !== this._notebookEditor.textModel?.uri.toString()) { return; } @@ -40,8 +40,9 @@ export class ExecutionEditorProgressController extends Disposable implements INo const scrollPadding = this._notebookEditor.notebookOptions.computeTopInsertToolbarHeight(this._notebookEditor.textModel.viewType); - const executing = this._notebookExecutionStateService.getCellExecutionsForNotebook(this._notebookEditor.textModel?.uri) + const cellExecutions = this._notebookExecutionStateService.getCellExecutionsForNotebook(this._notebookEditor.textModel?.uri) .filter(exe => exe.state === NotebookCellExecutionState.Executing); + const notebookExecution = this._notebookExecutionStateService.getExecution(this._notebookEditor.textModel?.uri); const executionIsVisible = (exe: INotebookCellExecution) => { for (const range of this._notebookEditor.visibleRanges) { for (const cell of this._notebookEditor.getCellsInRange(range)) { @@ -56,7 +57,9 @@ export class ExecutionEditorProgressController extends Disposable implements INo return false; }; - if (!executing.length || executing.some(executionIsVisible) || executing.some(e => e.isPaused)) { + const noCellsRunning = !cellExecutions.length || cellExecutions.some(executionIsVisible) || cellExecutions.some(e => e.isPaused); + const isSomethingRunning = !!notebookExecution || !noCellsRunning; + if (isSomethingRunning) { this._notebookEditor.hideProgress(); } else { this._notebookEditor.showProgress(); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts b/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts index d2dcbdda92232..acd6f718da6d2 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/outline/notebookOutline.ts @@ -35,7 +35,7 @@ import { IdleValue } from 'vs/base/common/async'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { IConfigurationRegistry, Extensions as ConfigurationExtensions } from 'vs/platform/configuration/common/configurationRegistry'; import { renderMarkdownAsPlaintext } from 'vs/base/browser/markdownRenderer'; -import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { executingStateIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons'; import { URI } from 'vs/base/common/uri'; import { getMarkdownHeadersInCell } from 'vs/workbench/contrib/notebook/browser/viewModel/foldingModel'; @@ -354,7 +354,7 @@ export class NotebookCellOutline implements IOutline { })); this._dispoables.add(_notebookExecutionStateService.onDidChangeExecution(e => { - if (e.type === 'cell' && !!this._editor.textModel && e.affectsNotebook(this._editor.textModel?.uri)) { + if (e.type === NotebookExecutionType.cell && !!this._editor.textModel && e.affectsNotebook(this._editor.textModel?.uri)) { this._recomputeState(); } })); diff --git a/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts index e9a1ea438ecdd..072c1601535a2 100644 --- a/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts @@ -16,7 +16,7 @@ import { ILogService } from 'vs/platform/log/common/log'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { CellEditType, CellUri, ICellEditOperation, NotebookCellExecutionState, NotebookCellInternalMetadata, NotebookExecutionState, NotebookTextModelWillAddRemoveEvent } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { CellExecutionUpdateType, INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; -import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionStateChangedEvent, ICellExecutionStateUpdate, IExecutionStateChangedEvent, IFailedCellInfo, INotebookCellExecution, INotebookExecution, INotebookExecutionStateService, INotebookFailStateChangedEvent } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionStateChangedEvent, ICellExecutionStateUpdate, IExecutionStateChangedEvent, IFailedCellInfo, INotebookCellExecution, INotebookExecution, INotebookExecutionStateService, INotebookFailStateChangedEvent, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; @@ -279,6 +279,10 @@ export class NotebookExecutionStateService extends Disposable implements INotebo executionMap.clear(); }); this._executions.clear(); + this._notebookExecutions.forEach(disposables => { + disposables.forEach(d => d.dispose()); + }); + this._notebookExecutions.clear(); this._cellListeners.forEach(disposable => disposable.dispose()); this._notebookListeners.forEach(disposable => disposable.dispose()); @@ -287,7 +291,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo } class NotebookCellExecutionEvent implements ICellExecutionStateChangedEvent { - readonly type = 'cell'; + readonly type = NotebookExecutionType.cell; constructor( readonly notebook: URI, readonly cellHandle: number, @@ -305,7 +309,7 @@ class NotebookCellExecutionEvent implements ICellExecutionStateChangedEvent { } class NotebookExecutionEvent implements IExecutionStateChangedEvent { - readonly type = 'notebook'; + readonly type = NotebookExecutionType.notebook; constructor( readonly notebook: URI, readonly changed?: NotebookExecution diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellContextKeys.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellContextKeys.ts index bc4dbf40b43cf..eb2c45f2f8ac8 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellContextKeys.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellContextKeys.ts @@ -13,7 +13,7 @@ import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewMod import { MarkupCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel'; import { NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { NotebookCellExecutionStateContext, NOTEBOOK_CELL_EDITABLE, NOTEBOOK_CELL_EDITOR_FOCUSED, NOTEBOOK_CELL_EXECUTING, NOTEBOOK_CELL_EXECUTION_STATE, NOTEBOOK_CELL_FOCUSED, NOTEBOOK_CELL_HAS_OUTPUTS, NOTEBOOK_CELL_INPUT_COLLAPSED, NOTEBOOK_CELL_LINE_NUMBERS, NOTEBOOK_CELL_MARKDOWN_EDIT_MODE, NOTEBOOK_CELL_OUTPUT_COLLAPSED, NOTEBOOK_CELL_RESOURCE, NOTEBOOK_CELL_TYPE } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; -import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; export class CellContextKeyPart extends CellContentPart { private cellContextKeyManager: CellContextKeyManager; @@ -78,7 +78,7 @@ export class CellContextKeyManager extends Disposable { }); this._register(this._notebookExecutionStateService.onDidChangeExecution(e => { - if (e.type === 'cell' && this.element && e.affectsCell(this.element.uri)) { + if (e.type === NotebookExecutionType.cell && this.element && e.affectsCell(this.element.uri)) { this.updateForExecutionState(); } })); diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCellExecutionIcon.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCellExecutionIcon.ts index 4ab2f9b4ac8b5..bf5326c5c2910 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCellExecutionIcon.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCellExecutionIcon.ts @@ -11,7 +11,7 @@ import { ThemeIcon } from 'vs/base/common/themables'; import { ICellViewModel, INotebookEditorDelegate } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { errorStateIcon, executingStateIcon, pendingStateIcon, successStateIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons'; import { NotebookCellExecutionState, NotebookCellInternalMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { INotebookCellExecution, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { INotebookCellExecution, INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; interface IExecutionItem { text: string; @@ -31,7 +31,7 @@ export class CollapsedCodeCellExecutionIcon extends Disposable { this._update(); this._register(this._executionStateService.onDidChangeExecution(e => { - if (e.type === 'cell' && e.affectsCell(this._cell.uri)) { + if (e.type === NotebookExecutionType.cell && e.affectsCell(this._cell.uri)) { this._update(); } })); diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts index 5e2d32477f247..653d624c6a19c 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl.ts @@ -33,7 +33,7 @@ import { CellKind, ICell, INotebookSearchOptions, ISelectionState, NotebookCells import { cellIndexesToRanges, cellRangesToIndexes, ICellRange, reduceCellRanges } from 'vs/workbench/contrib/notebook/common/notebookRange'; import { NotebookLayoutInfo, NotebookMetadataChangedEvent } from 'vs/workbench/contrib/notebook/browser/notebookViewEvents'; import { CellFindMatchModel } from 'vs/workbench/contrib/notebook/browser/contrib/find/findModel'; -import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; const invalidFunc = () => { throw new Error(`Invalid change accessor`); }; @@ -321,7 +321,7 @@ export class NotebookViewModel extends Disposable implements EditorFoldingStateD })); this._register(notebookExecutionStateService.onDidChangeExecution(e => { - if (e.type !== 'cell') { + if (e.type !== NotebookExecutionType.cell) { return; } const cell = this.getCellByHandle(e.cellHandle); diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts index 6a090560295e3..c7b0471d67642 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts @@ -8,7 +8,7 @@ import { DisposableStore, dispose, IDisposable } from 'vs/base/common/lifecycle' import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { ICellViewModel, INotebookEditorDelegate, KERNEL_EXTENSIONS } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { NOTEBOOK_CELL_TOOLBAR_LOCATION, NOTEBOOK_HAS_OUTPUTS, NOTEBOOK_HAS_RUNNING_CELL, NOTEBOOK_HAS_SOMETHING_RUNNING, NOTEBOOK_INTERRUPTIBLE_KERNEL, NOTEBOOK_KERNEL, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_KERNEL_SELECTED, NOTEBOOK_KERNEL_SOURCE_COUNT, NOTEBOOK_LAST_CELL_FAILED, NOTEBOOK_MISSING_KERNEL_EXTENSION, NOTEBOOK_USE_CONSOLIDATED_OUTPUT_BUTTON, NOTEBOOK_VIEW_TYPE } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; -import { ICellExecutionStateChangedEvent, IExecutionStateChangedEvent, INotebookExecutionStateService, INotebookFailStateChangedEvent } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { ICellExecutionStateChangedEvent, IExecutionStateChangedEvent, INotebookExecutionStateService, INotebookFailStateChangedEvent, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; @@ -141,12 +141,12 @@ export class NotebookEditorContextKeys { const notebookExe = this._notebookExecutionStateService.getExecution(this._editor.textModel.uri); const notebookCellExe = this._notebookExecutionStateService.getCellExecutionsForNotebook(this._editor.textModel.uri); this._kernelRunning.set(notebookCellExe.length > 0 || !!notebookExe); - if (e.type === 'cell') { + if (e.type === NotebookExecutionType.cell) { this._someCellRunning.set(notebookCellExe.length > 0); } } else { this._kernelRunning.set(false); - if (e.type === 'cell') { + if (e.type === NotebookExecutionType.cell) { this._someCellRunning.set(false); } } diff --git a/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts b/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts index 0ba4af628f33e..98a24d28adff8 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts @@ -24,9 +24,12 @@ export interface ICellExecutionComplete { runEndTime?: number; lastRunSuccess?: boolean; } - +export enum NotebookExecutionType { + cell, + notebook +} export interface ICellExecutionStateChangedEvent { - type: 'cell'; + type: NotebookExecutionType.cell; notebook: URI; cellHandle: number; changed?: INotebookCellExecution; // undefined -> execution was completed @@ -34,7 +37,7 @@ export interface ICellExecutionStateChangedEvent { affectsNotebook(notebook: URI): boolean; } export interface IExecutionStateChangedEvent { - type: 'notebook'; + type: NotebookExecutionType.notebook; notebook: URI; changed?: INotebookExecution; // undefined -> execution was completed affectsNotebook(notebook: URI): boolean; diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts index 84f65a7face82..6b13b6de41b1d 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts @@ -21,7 +21,7 @@ import { NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewMod import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { CellEditType, CellKind, CellUri, IOutputDto, NotebookCellMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { CellExecutionUpdateType, INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; -import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { INotebookKernel, INotebookKernelService } 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'; @@ -138,7 +138,7 @@ suite('NotebookExecutionStateService', () => { let didFire = false; disposables.add(executionStateService.onDidChangeExecution(e => { - if (e.type === 'cell') { + if (e.type === NotebookExecutionType.cell) { didFire = !e.changed; } })); @@ -165,7 +165,7 @@ suite('NotebookExecutionStateService', () => { let didFire = false; disposables.add(executionStateService.onDidChangeExecution(e => { - if (e.type === 'cell') { + if (e.type === NotebookExecutionType.cell) { didFire = true; } })); @@ -192,7 +192,7 @@ suite('NotebookExecutionStateService', () => { const deferred = new DeferredPromise(); disposables.add(executionStateService.onDidChangeExecution(e => { - if (e.type === 'cell') { + if (e.type === NotebookExecutionType.cell) { const cellUri = CellUri.generate(e.notebook, e.cellHandle); const exe = executionStateService.getCellExecution(cellUri); assert.ok(exe); From a81c11e798c758bda89e2dc4243a9d7a3b322f3e Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 6 Apr 2023 09:28:18 +1000 Subject: [PATCH 8/8] added tests --- .../api/common/extHostNotebookKernels.ts | 10 +- .../notebookExecutionStateServiceImpl.ts | 3 +- .../notebookExecutionStateService.test.ts | 118 +++++++++++++++++- 3 files changed, 123 insertions(+), 8 deletions(-) diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index efef57e08fd4e..f3e1263c357f7 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -400,10 +400,12 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { } } - // If we're interrupting all cells, we also need to cancel the notebook level execution. - const items = this._activeNotebookExecutions.get(document.uri); - if (handles.length && Array.isArray(items) && items.length) { - items.forEach(d => d.dispose()); + if (obj.controller.interruptHandler) { + // If we're interrupting all cells, we also need to cancel the notebook level execution. + const items = this._activeNotebookExecutions.get(document.uri); + if (handles.length && Array.isArray(items) && items.length) { + items.forEach(d => d.dispose()); + } } } diff --git a/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts index 072c1601535a2..3d763f0c45341 100644 --- a/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/services/notebookExecutionStateServiceImpl.ts @@ -136,9 +136,9 @@ export class NotebookExecutionStateService extends Disposable implements INotebo return; } - disposables.forEach(d => d.dispose()); this._notebookExecutions.delete(notebookUri); this._onDidChangeExecution.fire(new NotebookExecutionEvent(notebookUri)); + disposables.forEach(d => d.dispose()); } createCellExecution(notebookUri: URI, cellHandle: number): INotebookCellExecution { @@ -181,7 +181,6 @@ export class NotebookExecutionStateService extends Disposable implements INotebo if (!info) { info = this._createNotebookExecution(notebook); this._notebookExecutions.set(notebookUri, info); - // exe.initialize(); this._onDidChangeExecution.fire(new NotebookExecutionEvent(notebookUri, info[0])); } diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts index 6b13b6de41b1d..13c2d90875674 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts @@ -19,7 +19,7 @@ import { NotebookExecutionStateService } from 'vs/workbench/contrib/notebook/bro import { NotebookKernelService } from 'vs/workbench/contrib/notebook/browser/services/notebookKernelServiceImpl'; import { NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; -import { CellEditType, CellKind, CellUri, IOutputDto, NotebookCellMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellEditType, CellKind, CellUri, IOutputDto, NotebookCellMetadata, NotebookExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { CellExecutionUpdateType, INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; import { INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { INotebookKernel, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; @@ -211,8 +211,77 @@ suite('NotebookExecutionStateService', () => { return deferred.p; }); }); + test('getExecution and onDidChangeExecution', async function () { + return withTestNotebook([], async viewModel => { + testNotebookModel = viewModel.notebookDocument; + + const kernel = new TestNotebookKernel(); + kernelService.registerKernel(kernel); + kernelService.selectKernelForNotebook(kernel, viewModel.notebookDocument); + + const eventRaisedWithExecution: boolean[] = []; + const executionStateService: INotebookExecutionStateService = instantiationService.get(INotebookExecutionStateService); + executionStateService.onDidChangeExecution(e => eventRaisedWithExecution.push(e.type === NotebookExecutionType.notebook && !!e.changed), this, disposables); + + const deferred = new DeferredPromise(); + disposables.add(executionStateService.onDidChangeExecution(e => { + if (e.type === NotebookExecutionType.notebook) { + const exe = executionStateService.getExecution(viewModel.uri); + assert.ok(exe); + assert.strictEqual(e.notebook.toString(), exe.notebook.toString()); + assert.ok(e.affectsNotebook(viewModel.uri)); + assert.deepStrictEqual(eventRaisedWithExecution, [true]); + deferred.complete(); + } + })); + + executionStateService.createExecution(viewModel.uri); + + return deferred.p; + }); + }); + + test('getExecution and onDidChangeExecution', async function () { + return withTestNotebook([], async viewModel => { + testNotebookModel = viewModel.notebookDocument; + + const kernel = new TestNotebookKernel(); + kernelService.registerKernel(kernel); + kernelService.selectKernelForNotebook(kernel, viewModel.notebookDocument); + + const executionStateService: INotebookExecutionStateService = instantiationService.get(INotebookExecutionStateService); + + const deferred = new DeferredPromise(); + const expectedNotebookEventStates: (NotebookExecutionState | undefined)[] = [NotebookExecutionState.Unconfirmed, NotebookExecutionState.Pending, NotebookExecutionState.Executing, undefined]; + executionStateService.onDidChangeExecution(e => { + if (e.type === NotebookExecutionType.notebook) { + const expectedState = expectedNotebookEventStates.shift(); + if (typeof expectedState === 'number') { + const exe = executionStateService.getExecution(viewModel.uri); + assert.ok(exe); + assert.strictEqual(e.notebook.toString(), exe.notebook.toString()); + assert.strictEqual(e.changed?.state, expectedState); + } else { + assert.ok(e.changed === undefined); + } + + assert.ok(e.affectsNotebook(viewModel.uri)); + if (expectedNotebookEventStates.length === 0) { + deferred.complete(); + } + } + }, this, disposables); - test('force-cancel works', async function () { + const execution = executionStateService.createExecution(viewModel.uri); + execution.confirm(); + execution.begin(); + execution.complete(); + + return deferred.p; + }); + }); + + test('force-cancel works for Cell Execution', async function () { return withTestNotebook([], async viewModel => { testNotebookModel = viewModel.notebookDocument; @@ -231,6 +300,51 @@ suite('NotebookExecutionStateService', () => { assert.strictEqual(exe2, undefined); }); }); + test('force-cancel works for Notebook Execution', async function () { + return withTestNotebook([], async viewModel => { + testNotebookModel = viewModel.notebookDocument; + + const kernel = new TestNotebookKernel(); + kernelService.registerKernel(kernel); + kernelService.selectKernelForNotebook(kernel, viewModel.notebookDocument); + const eventRaisedWithExecution: boolean[] = []; + + const executionStateService: INotebookExecutionStateService = instantiationService.get(INotebookExecutionStateService); + executionStateService.onDidChangeExecution(e => eventRaisedWithExecution.push(e.type === NotebookExecutionType.notebook && !!e.changed), this, disposables); + executionStateService.createExecution(viewModel.uri); + const exe = executionStateService.getExecution(viewModel.uri); + assert.ok(exe); + assert.deepStrictEqual(eventRaisedWithExecution, [true]); + + executionStateService.forceCancelNotebookExecutions(viewModel.uri); + const exe2 = executionStateService.getExecution(viewModel.uri); + assert.deepStrictEqual(eventRaisedWithExecution, [true, false]); + assert.strictEqual(exe2, undefined); + }); + }); + test('force-cancel works for Cell and Notebook Execution', async function () { + return withTestNotebook([], async viewModel => { + testNotebookModel = viewModel.notebookDocument; + + const kernel = new TestNotebookKernel(); + kernelService.registerKernel(kernel); + kernelService.selectKernelForNotebook(kernel, viewModel.notebookDocument); + + const executionStateService: INotebookExecutionStateService = instantiationService.get(INotebookExecutionStateService); + executionStateService.createExecution(viewModel.uri); + executionStateService.createExecution(viewModel.uri); + const cellExe = executionStateService.getExecution(viewModel.uri); + const exe = executionStateService.getExecution(viewModel.uri); + assert.ok(cellExe); + assert.ok(exe); + + executionStateService.forceCancelNotebookExecutions(viewModel.uri); + const cellExe2 = executionStateService.getExecution(viewModel.uri); + const exe2 = executionStateService.getExecution(viewModel.uri); + assert.strictEqual(cellExe2, undefined); + assert.strictEqual(exe2, undefined); + }); + }); }); class TestNotebookKernel implements INotebookKernel {