From 33d81fdc8c25ea1905dca0d89c61b48b7d0296fe Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Mon, 29 Nov 2021 14:00:53 -0800 Subject: [PATCH 1/7] Improve NotebookExecutionService and cancel executions when closing the notebook, deleting cells, and changing the kernel --- .../api/browser/mainThreadNotebookDto.ts | 6 +- .../api/browser/mainThreadNotebookKernels.ts | 45 ++- .../workbench/api/common/extHost.protocol.ts | 17 +- .../api/common/extHostNotebookKernels.ts | 30 +- .../browser/notebookEditorKernelManager.ts | 71 ---- .../notebook/browser/notebookEditorWidget.ts | 49 ++- .../browser/notebookExecutionServiceImpl.ts | 318 +++++++++++++++--- .../common/model/notebookTextModel.ts | 7 + .../common/notebookExecutionService.ts | 22 +- 9 files changed, 365 insertions(+), 200 deletions(-) delete mode 100644 src/vs/workbench/contrib/notebook/browser/notebookEditorKernelManager.ts diff --git a/src/vs/workbench/api/browser/mainThreadNotebookDto.ts b/src/vs/workbench/api/browser/mainThreadNotebookDto.ts index 5d582e16f86..d63c6412198 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookDto.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookDto.ts @@ -6,7 +6,7 @@ import * as extHostProtocol from 'vs/workbench/api/common/extHost.protocol'; import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import * as notebookCommon from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { CellExecutionUpdateType, ICellExecuteUpdate } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; +import { CellExecutionUpdateType, ICellExecuteUpdate, ICellExecutionComplete } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; export namespace NotebookDto { @@ -112,6 +112,10 @@ export namespace NotebookDto { } } + export function fromCellExecuteCompleteDto(data: extHostProtocol.ICellExecutionCompleteDto): ICellExecutionComplete { + return data; + } + export function fromCellEditOperationDto(edit: extHostProtocol.ICellEditOperationDto): notebookCommon.ICellEditOperation { if (edit.editType === notebookCommon.CellEditType.Replace) { return { diff --git a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts index b79261dc74a..71aa6d1b0a4 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts @@ -6,7 +6,7 @@ import { flatten, groupBy, isNonEmptyArray } from 'vs/base/common/arrays'; import { onUnexpectedError } from 'vs/base/common/errors'; import { Emitter, Event } from 'vs/base/common/event'; -import { combinedDisposable, DisposableStore, IDisposable } from 'vs/base/common/lifecycle'; +import { combinedDisposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; import { URI, UriComponents } from 'vs/base/common/uri'; import { ILanguageService } from 'vs/editor/common/services/languageService'; import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; @@ -14,10 +14,11 @@ import { NotebookDto } from 'vs/workbench/api/browser/mainThreadNotebookDto'; import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/notebookEditorService'; -import { INotebookCellExecution, INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; +import { CellUri } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; import { INotebookKernel, INotebookKernelChangeEvent, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { SerializableObjectWithBuffers } from 'vs/workbench/services/extensions/common/proxyIdentifier'; -import { ExtHostContext, ExtHostNotebookKernelsShape, ICellExecuteUpdateDto, IExtHostContext, INotebookKernelDto2, MainContext, MainThreadNotebookKernelsShape } from '../common/extHost.protocol'; +import { ExtHostContext, ExtHostNotebookKernelsShape, ICellExecuteUpdateDto, ICellExecutionCompleteDto, IExtHostContext, INotebookKernelDto2, MainContext, MainThreadNotebookKernelsShape } from '../common/extHost.protocol'; abstract class MainThreadKernel implements INotebookKernel { @@ -106,7 +107,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape private readonly _kernels = new Map(); private readonly _proxy: ExtHostNotebookKernelsShape; - private readonly _executions = new Map(); + private readonly _executions = new Set(); constructor( extHostContext: IExtHostContext, @@ -121,6 +122,16 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape notebookEditorService.listNotebookEditors().forEach(this._onEditorAdd, this); notebookEditorService.onDidAddNotebookEditor(this._onEditorAdd, this, this._disposables); notebookEditorService.onDidRemoveNotebookEditor(this._onEditorRemove, this, this._disposables); + + this._disposables.add(toDisposable(() => { + // EH shut down, complete all executions started by this EH + this._executions.forEach(e => { + const uri = CellUri.parse(URI.parse(e)); + if (uri) { + this._notebookExecutionService.completeNotebookCellExecution(uri.notebook, uri.handle, { }); + } + }); + })); } dispose(): void { @@ -235,30 +246,34 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape // --- execution - $addExecution(handle: number, uri: UriComponents, cellHandle: number): void { - const execution = this._notebookExecutionService.createNotebookCellExecution(URI.revive(uri), cellHandle); - this._executions.set(handle, execution); + $addExecution(rawUri: UriComponents, cellHandle: number): void { + const uri = URI.revive(rawUri); + this._notebookExecutionService.createNotebookCellExecution(uri, cellHandle); + + const cellUri = CellUri.generateCellUri(uri, cellHandle, uri.scheme); + this._executions.add(cellUri.toString()); } $updateExecutions(data: SerializableObjectWithBuffers): void { const updates = data.value; - const groupedUpdates = groupBy(updates, (a, b) => a.executionHandle - b.executionHandle); + const groupedUpdates = groupBy(updates, (a, b) => a.cellHandle - b.cellHandle); groupedUpdates.forEach(datas => { const first = datas[0]; - const execution = this._executions.get(first.executionHandle); - if (!execution) { - return; - } try { - execution.update(datas.map(NotebookDto.fromCellExecuteUpdateDto)); + const uri = URI.revive(first.uri); + this._notebookExecutionService.updateNotebookCellExecution(uri, first.cellHandle, datas.map(NotebookDto.fromCellExecuteUpdateDto)); } catch (e) { onUnexpectedError(e); } }); } - $removeExecution(handle: number): void { - this._executions.delete(handle); + $completeExecution(rawUri: UriComponents, cellHandle: number, data: SerializableObjectWithBuffers): void { + const uri = URI.revive(rawUri); + this._notebookExecutionService.completeNotebookCellExecution(uri, cellHandle, NotebookDto.fromCellExecuteCompleteDto(data.value)); + + const cellUri = CellUri.generateCellUri(uri, cellHandle, uri.scheme); + this._executions.delete(cellUri.toString()); } } diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index f6e82bc208a..4742f3bdf5d 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -922,7 +922,7 @@ export interface INotebookKernelDto2 { export interface ICellExecuteOutputEditDto { editType: CellExecutionUpdateType.Output; - executionHandle: number; + uri: UriComponents; cellHandle: number; append?: boolean; outputs: NotebookOutputDto[] @@ -930,21 +930,24 @@ export interface ICellExecuteOutputEditDto { export interface ICellExecuteOutputItemEditDto { editType: CellExecutionUpdateType.OutputItems; - executionHandle: number; + uri: UriComponents; + cellHandle: number; append?: boolean; outputId: string; items: NotebookOutputItemDto[] } export interface ICellExecutionStateUpdateDto extends ICellExecutionStateUpdate { - executionHandle: number; + uri: UriComponents; + cellHandle: number; } export interface ICellExecutionCompleteDto extends ICellExecutionComplete { - executionHandle: number; + uri: UriComponents; + cellHandle: number; } -export type ICellExecuteUpdateDto = ICellExecuteOutputEditDto | ICellExecuteOutputItemEditDto | ICellExecutionStateUpdateDto | ICellExecutionCompleteDto; +export type ICellExecuteUpdateDto = ICellExecuteOutputEditDto | ICellExecuteOutputItemEditDto | ICellExecutionStateUpdateDto; export interface MainThreadNotebookKernelsShape extends IDisposable { $postMessage(handle: number, editorId: string | undefined, message: any): Promise; @@ -953,9 +956,9 @@ export interface MainThreadNotebookKernelsShape extends IDisposable { $removeKernel(handle: number): void; $updateNotebookPriority(handle: number, uri: UriComponents, value: number | undefined): void; - $addExecution(handle: number, uri: UriComponents, cellHandle: number): void; + $addExecution(uri: UriComponents, cellHandle: number): void; $updateExecutions(data: SerializableObjectWithBuffers): void; - $removeExecution(handle: number): void; + $completeExecution(uri: UriComponents, cellHandle: number, data: SerializableObjectWithBuffers): void; } export interface MainThreadNotebookRenderersShape extends IDisposable { diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index 75895d19dcf..b519a071f6c 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -366,9 +366,6 @@ enum NotebookCellExecutionTaskState { } class NotebookCellExecutionTask extends Disposable { - private static HANDLE = 0; - private _handle = NotebookCellExecutionTask.HANDLE++; - private _onDidChangeState = new Emitter(); readonly onDidChangeState = this._onDidChangeState.event; @@ -391,7 +388,7 @@ class NotebookCellExecutionTask extends Disposable { this._collector = new TimeoutBasedCollector(10, updates => this.update(updates)); this._executionOrder = _cell.internalMetadata.executionOrder; - this._proxy.$addExecution(this._handle, this._cell.notebook.uri, this._cell.handle); + this._proxy.$addExecution(this._cell.notebook.uri, this._cell.handle); } cancel(): void { @@ -448,7 +445,7 @@ class NotebookCellExecutionTask extends Disposable { return this.updateSoon( { editType: CellExecutionUpdateType.Output, - executionHandle: this._handle, + uri: this._document.uri, cellHandle: handle, append, outputs: outputDtos @@ -459,7 +456,8 @@ class NotebookCellExecutionTask extends Disposable { items = NotebookCellOutput.ensureUniqueMimeTypes(asArray(items), true); return this.updateSoon({ editType: CellExecutionUpdateType.OutputItems, - executionHandle: this._handle, + uri: this._document.uri, + cellHandle: this._cell.handle, items: items.map(extHostTypeConverters.NotebookCellOutputItem.from), outputId: output.id, append @@ -476,7 +474,8 @@ class NotebookCellExecutionTask extends Disposable { that._executionOrder = v; that.update([{ editType: CellExecutionUpdateType.ExecutionState, - executionHandle: that._handle, + uri: that._document.uri, + cellHandle: that._cell.handle, executionOrder: that._executionOrder }]); }, @@ -491,7 +490,8 @@ class NotebookCellExecutionTask extends Disposable { that.update({ editType: CellExecutionUpdateType.ExecutionState, - executionHandle: that._handle, + uri: that._document.uri, + cellHandle: that._cell.handle, runStartTime: startTime }); }, @@ -504,18 +504,16 @@ class NotebookCellExecutionTask extends Disposable { that._state = NotebookCellExecutionTaskState.Resolved; that._onDidChangeState.fire(); - that.updateSoon({ - editType: CellExecutionUpdateType.Complete, - executionHandle: that._handle, - runEndTime: endTime, - lastRunSuccess: success - }); - // The last update needs to be ordered correctly and applied immediately, // so we use updateSoon and immediately flush. that._collector.flush(); - that._proxy.$removeExecution(that._handle); + that._proxy.$completeExecution(that._document.uri, that._cell.handle, new SerializableObjectWithBuffers({ + uri: that._document.uri, + cellHandle: that._cell.handle, + runEndTime: endTime, + lastRunSuccess: success + })); }, clearOutput(cell?: vscode.NotebookCell): Thenable { diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorKernelManager.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorKernelManager.ts deleted file mode 100644 index 37a5b0f439e..00000000000 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorKernelManager.ts +++ /dev/null @@ -1,71 +0,0 @@ -/*--------------------------------------------------------------------------------------------- - * Copyright (c) Microsoft Corporation. All rights reserved. - * Licensed under the MIT License. See License.txt in the project root for license information. - *--------------------------------------------------------------------------------------------*/ - -import * as nls from 'vs/nls'; -import { Disposable } from 'vs/base/common/lifecycle'; -import { ICellViewModel } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { CellKind, INotebookTextModel, NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { ICommandService } from 'vs/platform/commands/common/commands'; -import { INotebookKernel, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; -import { IWorkspaceTrustRequestService } from 'vs/platform/workspace/common/workspaceTrust'; -import { SELECT_KERNEL_ID } from 'vs/workbench/contrib/notebook/browser/controller/coreActions'; - -export class NotebookEditorKernelManager extends Disposable { - - constructor( - @ICommandService private readonly _commandService: ICommandService, - @INotebookKernelService private readonly _notebookKernelService: INotebookKernelService, - @IWorkspaceTrustRequestService private readonly _workspaceTrustRequestService: IWorkspaceTrustRequestService, - ) { - super(); - } - - getSelectedOrSuggestedKernel(notebook: INotebookTextModel): INotebookKernel | undefined { - // returns SELECTED or the ONLY available kernel - const info = this._notebookKernelService.getMatchingKernel(notebook); - return info.selected ?? (info.all.length === 1 ? info.all[0] : undefined); - } - - async executeNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise { - const message = nls.localize('notebookRunTrust', "Executing a notebook cell will run code from this workspace."); - const trust = await this._workspaceTrustRequestService.requestWorkspaceTrust({ message }); - if (!trust) { - return; - } - - let kernel = this.getSelectedOrSuggestedKernel(notebook); - if (!kernel) { - await this._commandService.executeCommand(SELECT_KERNEL_ID); - kernel = this.getSelectedOrSuggestedKernel(notebook); - } - - if (!kernel) { - return; - } - - const cellHandles: number[] = []; - for (const cell of cells) { - if (cell.cellKind !== CellKind.Code || cell.internalMetadata.runState === NotebookCellExecutionState.Pending || cell.internalMetadata.runState === NotebookCellExecutionState.Executing) { - continue; - } - if (!kernel.supportedLanguages.includes(cell.language)) { - continue; - } - cellHandles.push(cell.handle); - } - - if (cellHandles.length > 0) { - this._notebookKernelService.selectKernelForNotebook(kernel, notebook); - await kernel.executeNotebookCellsRequest(notebook.uri, cellHandles); - } - } - - async cancelNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise { - let kernel = this.getSelectedOrSuggestedKernel(notebook); - if (kernel) { - await kernel.cancelNotebookCellExecution(notebook.uri, Array.from(cells, cell => cell.handle)); - } - } -} diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index d8f6ba6ef4b..ff5bf9a1154 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -5,8 +5,8 @@ import { getPixelRatio, getZoomLevel } from 'vs/base/browser/browser'; import * as DOM from 'vs/base/browser/dom'; -import * as aria from 'vs/base/browser/ui/aria/aria'; import { IMouseWheelEvent, StandardMouseEvent } from 'vs/base/browser/mouseEvent'; +import * as aria from 'vs/base/browser/ui/aria/aria'; import { IListContextMenuEvent } from 'vs/base/browser/ui/list/list'; import { IAction } from 'vs/base/common/actions'; import { SequencerByKey } from 'vs/base/common/async'; @@ -18,11 +18,13 @@ import { extname, isEqual } from 'vs/base/common/resources'; import { URI } from 'vs/base/common/uri'; import { generateUuid } from 'vs/base/common/uuid'; import 'vs/css!./media/notebook'; +import { readFontInfo } from 'vs/editor/browser/config/configuration'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IEditorOptions } from 'vs/editor/common/config/editorOptions'; import { BareFontInfo, FontInfo } from 'vs/editor/common/config/fontInfo'; import { Range } from 'vs/editor/common/core/range'; import { IEditor } from 'vs/editor/common/editorCommon'; +import { SuggestController } from 'vs/editor/contrib/suggest/suggestController'; import * as nls from 'vs/nls'; import { createAndFillInContextMenuActions } from 'vs/platform/actions/browser/menuEntryActionViewItem'; import { IMenuService, MenuId } from 'vs/platform/actions/common/actions'; @@ -32,45 +34,43 @@ import { IContextMenuService } from 'vs/platform/contextview/browser/contextView import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection'; import { ILayoutService } from 'vs/platform/layout/browser/layoutService'; +import { registerZIndex, ZIndex } from 'vs/platform/layout/browser/zIndexRegistry'; import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry'; import { contrastBorder, diffInserted, diffRemoved, editorBackground, errorForeground, focusBorder, foreground, iconForeground, listInactiveSelectionBackground, registerColor, scrollbarSliderActiveBackground, scrollbarSliderBackground, scrollbarSliderHoverBackground, textBlockQuoteBackground, textBlockQuoteBorder, textLinkActiveForeground, textLinkForeground, textPreformatForeground, toolbarHoverBackground, transparent } from 'vs/platform/theme/common/colorRegistry'; import { IThemeService, registerThemingParticipant } from 'vs/platform/theme/common/themeService'; import { PANEL_BORDER, SIDE_BAR_BACKGROUND } from 'vs/workbench/common/theme'; import { debugIconStartForeground } from 'vs/workbench/contrib/debug/browser/debugColors'; import { CellEditState, CellFocusMode, IActiveNotebookEditorDelegate, ICellOutputViewModel, ICellViewModel, ICommonCellInfo, IDisplayOutputLayoutUpdateRequest, IFocusNotebookCellOptions, IGenericCellViewModel, IInsetRenderOutput, INotebookCellOutputLayoutInfo, INotebookDeltaDecoration, INotebookEditorContribution, INotebookEditorContributionDescription, INotebookEditorCreationOptions, INotebookEditorDelegate, INotebookEditorMouseEvent, INotebookEditorOptions, NotebookCellStateChangedEvent, NotebookLayoutChangedEvent, NotebookLayoutInfo, NOTEBOOK_EDITOR_EDITABLE, NOTEBOOK_EDITOR_FOCUSED, NOTEBOOK_OUTPUT_FOCUSED, RenderOutputType } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { NotebookDecorationCSSRules, NotebookRefCountedStyleSheet } from 'vs/workbench/contrib/notebook/browser/viewParts/notebookEditorDecorations'; -import { NotebookEditorContextKeys } from 'vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys'; -import { NotebookEditorToolbar } from 'vs/workbench/contrib/notebook/browser/viewParts/notebookEditorToolbar'; import { NotebookEditorExtensionsRegistry } from 'vs/workbench/contrib/notebook/browser/notebookEditorExtensions'; -import { NotebookEditorKernelManager } from 'vs/workbench/contrib/notebook/browser/notebookEditorKernelManager'; import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/notebookEditorService'; import { NotebookCellList, NOTEBOOK_WEBVIEW_BOUNDARY } from 'vs/workbench/contrib/notebook/browser/view/notebookCellList'; -import { OutputRenderer } from 'vs/workbench/contrib/notebook/browser/view/output/outputRenderer'; -import { BackLayerWebView, INotebookWebviewMessage } from 'vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView'; +import { notebookDebug } from 'vs/workbench/contrib/notebook/browser/notebookLogger'; import { CellContextKeyManager } from 'vs/workbench/contrib/notebook/browser/view/cellParts/cellContextKeys'; import { CellDragAndDropController } from 'vs/workbench/contrib/notebook/browser/view/cellParts/cellDnd'; +import { INotebookCellList } from 'vs/workbench/contrib/notebook/browser/view/notebookRenderingCommon'; +import { OutputRenderer } from 'vs/workbench/contrib/notebook/browser/view/output/outputRenderer'; +import { BackLayerWebView, INotebookWebviewMessage } from 'vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView'; import { CodeCellRenderer, MarkupCellRenderer, NotebookCellListDelegate } from 'vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer'; +import { IAckOutputHeight, IMarkupCellInitialization } from 'vs/workbench/contrib/notebook/browser/view/renderers/webviewMessages'; import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel'; import { NotebookEventDispatcher } from 'vs/workbench/contrib/notebook/browser/viewModel/eventDispatcher'; import { MarkupCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel'; import { CellViewModel, IModelDecorationsChangeAccessor, INotebookEditorViewState, INotebookViewCellsUpdateEvent, NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; +import { ViewContext } from 'vs/workbench/contrib/notebook/browser/viewModel/viewContext'; +import { NotebookDecorationCSSRules, NotebookRefCountedStyleSheet } from 'vs/workbench/contrib/notebook/browser/viewParts/notebookEditorDecorations'; +import { NotebookEditorToolbar } from 'vs/workbench/contrib/notebook/browser/viewParts/notebookEditorToolbar'; +import { NotebookEditorContextKeys } from 'vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys'; +import { ListTopCellToolbar } from 'vs/workbench/contrib/notebook/browser/viewParts/notebookTopCellToolbar'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { CellKind, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange'; -import { editorGutterModifiedBackground } from 'vs/workbench/contrib/scm/browser/dirtydiffDecorator'; -import { IWebview } from 'vs/workbench/contrib/webview/browser/webview'; -import { mark } from 'vs/workbench/contrib/notebook/common/notebookPerformance'; -import { readFontInfo } from 'vs/editor/browser/config/configuration'; +import { INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { NotebookOptions, OutputInnerContainerTopPadding } from 'vs/workbench/contrib/notebook/common/notebookOptions'; -import { ViewContext } from 'vs/workbench/contrib/notebook/browser/viewModel/viewContext'; +import { mark } from 'vs/workbench/contrib/notebook/common/notebookPerformance'; +import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange'; import { INotebookRendererMessagingService } from 'vs/workbench/contrib/notebook/common/notebookRendererMessagingService'; -import { IAckOutputHeight, IMarkupCellInitialization } from 'vs/workbench/contrib/notebook/browser/view/renderers/webviewMessages'; -import { SuggestController } from 'vs/editor/contrib/suggest/suggestController'; -import { registerZIndex, ZIndex } from 'vs/platform/layout/browser/zIndexRegistry'; -import { INotebookCellList } from 'vs/workbench/contrib/notebook/browser/view/notebookRenderingCommon'; -import { notebookDebug } from 'vs/workbench/contrib/notebook/browser/notebookLogger'; -import { ListTopCellToolbar } from 'vs/workbench/contrib/notebook/browser/viewParts/notebookTopCellToolbar'; +import { editorGutterModifiedBackground } from 'vs/workbench/contrib/scm/browser/dirtydiffDecorator'; +import { IWebview } from 'vs/workbench/contrib/webview/browser/webview'; const $ = DOM.$; @@ -323,7 +323,6 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD protected readonly _contributions = new Map(); private _scrollBeyondLastLine: boolean; private readonly _insetModifyQueueByOutputId = new SequencerByKey(); - private _kernelManger: NotebookEditorKernelManager; private _cellContextKeyManager: CellContextKeyManager | null = null; private _isVisible = false; private readonly _uuid = generateUuid(); @@ -399,7 +398,8 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD @IContextMenuService private readonly contextMenuService: IContextMenuService, @IMenuService private readonly menuService: IMenuService, @IThemeService private readonly themeService: IThemeService, - @ITelemetryService private readonly telemetryService: ITelemetryService + @ITelemetryService private readonly telemetryService: ITelemetryService, + @INotebookExecutionService private readonly notebookExecutionService: INotebookExecutionService, ) { super(); this.isEmbedded = creationOptions.isEmbedded ?? false; @@ -418,7 +418,6 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD this._register(this.instantiationService.createInstance(NotebookEditorContextKeys, this)); - this._kernelManger = this.instantiationService.createInstance(NotebookEditorKernelManager); this._register(notebookKernelService.onDidChangeSelectedNotebooks(e => { if (isEqual(e.notebook, this.viewModel?.uri)) { this._loadKernelPreloads(); @@ -2052,7 +2051,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD } get activeKernel() { - return this.textModel && this._kernelManger.getSelectedOrSuggestedKernel(this.textModel); + return this.textModel && this.notebookExecutionService.getSelectedOrSuggestedKernel(this.textModel); } async cancelNotebookCells(cells?: Iterable): Promise { @@ -2062,7 +2061,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD if (!cells) { cells = this.viewModel.viewCells; } - return this._kernelManger.cancelNotebookCells(this.textModel, cells); + return this.notebookExecutionService.cancelNotebookCellHandles(this.textModel, Array.from(cells).map(cell => cell.handle)); } async executeNotebookCells(cells?: Iterable): Promise { @@ -2072,7 +2071,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD if (!cells) { cells = this.viewModel.viewCells; } - return this._kernelManger.executeNotebookCells(this.textModel, cells); + return this.notebookExecutionService.executeNotebookCells(this.textModel, cells); } //#endregion diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts index 6ae2fba1350..445612437f7 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts @@ -3,28 +3,259 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { IDisposable } from 'vs/base/common/lifecycle'; +import { Disposable } from 'vs/base/common/lifecycle'; +import { ResourceMap } from 'vs/base/common/map'; import { URI } from 'vs/base/common/uri'; -import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; +import * as nls from 'vs/nls'; +import { ICommandService } from 'vs/platform/commands/common/commands'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; +import { ILogService } from 'vs/platform/log/common/log'; +import { IWorkspaceTrustRequestService } from 'vs/platform/workspace/common/workspaceTrust'; +import { SELECT_KERNEL_ID } from 'vs/workbench/contrib/notebook/browser/controller/coreActions'; +import { ICellViewModel } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; -import { CellEditType, ICellEditOperation, NotebookCellExecutionState, NotebookCellInternalMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { CellExecutionUpdateType, ICellExecuteUpdate, INotebookCellExecution, INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; +import { CellEditType, CellKind, ICellEditOperation, INotebookTextModel, NotebookCellExecutionState, NotebookCellInternalMetadata, NotebookTextModelWillAddRemoveEvent } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellExecutionUpdateType, ICellExecuteUpdate, ICellExecutionComplete, INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; +import { INotebookKernel, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; -export class NotebookExecutionService implements INotebookExecutionService { +export class NotebookExecutionService extends Disposable implements INotebookExecutionService { declare _serviceBrand: undefined; + private readonly _executions = new ResourceMap(); + constructor( - @INotebookService private readonly _notebookService: INotebookService, + @ICommandService private readonly _commandService: ICommandService, + @INotebookKernelService private readonly _notebookKernelService: INotebookKernelService, + @IWorkspaceTrustRequestService private readonly _workspaceTrustRequestService: IWorkspaceTrustRequestService, + @IInstantiationService private readonly _instantiationService: IInstantiationService, + @ILogService private readonly _logService: ILogService, ) { + super(); + + this._register(this._notebookKernelService.onDidChangeSelectedNotebooks(e => { + if (e.newKernel) { + const notebookExecution = this._executions.get(e.notebook); + if (notebookExecution) { + notebookExecution.cancelAll(); + this.checkNotebookExecutionEmpty(e.notebook); + } + } + })); } - createNotebookCellExecution(notebook: URI, cellHandle: number): INotebookCellExecution { - return new CellExecution(notebook, cellHandle, this._notebookService); + createNotebookCellExecution(notebook: URI, cellHandle: number): void { + let notebookExecution = this._executions.get(notebook); + if (!notebookExecution) { + notebookExecution = this._instantiationService.createInstance(NotebookExecution, notebook); + this._executions.set(notebook, notebookExecution); + } + + notebookExecution.addExecution(cellHandle); + } + + getSelectedOrSuggestedKernel(notebook: INotebookTextModel): INotebookKernel | undefined { + // TODO later can be inlined in notebookEditorWidget + // returns SELECTED or the ONLY available kernel + const info = this._notebookKernelService.getMatchingKernel(notebook); + return info.selected ?? (info.all.length === 1 ? info.all[0] : undefined); + } + + async executeNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise { + const cellsArr = Array.from(cells); + this._logService.debug(`NotebookExecutionService#executeNotebookCells ${JSON.stringify(cellsArr.map(c => c.handle))}`); + const message = nls.localize('notebookRunTrust', "Executing a notebook cell will run code from this workspace."); + const trust = await this._workspaceTrustRequestService.requestWorkspaceTrust({ message }); + if (!trust) { + return; + } + + let kernel = this.getSelectedOrSuggestedKernel(notebook); + if (!kernel) { + await this._commandService.executeCommand(SELECT_KERNEL_ID); + kernel = this.getSelectedOrSuggestedKernel(notebook); + } + + if (!kernel) { + return; + } + + const cellHandles: number[] = []; + for (const cell of cellsArr) { + if (cell.cellKind !== CellKind.Code || cell.internalMetadata.runState === NotebookCellExecutionState.Pending || cell.internalMetadata.runState === NotebookCellExecutionState.Executing) { + continue; + } + if (!kernel.supportedLanguages.includes(cell.language)) { + continue; + } + cellHandles.push(cell.handle); + } + + if (cellHandles.length > 0) { + this._notebookKernelService.selectKernelForNotebook(kernel, notebook); + await kernel.executeNotebookCellsRequest(notebook.uri, cellHandles); + } + } + + async cancelNotebookCellHandles(notebook: INotebookTextModel, cells: Iterable): Promise { + const cellsArr = Array.from(cells); + this._logService.debug(`NotebookExecutionService#cancelNotebookCellHandles ${JSON.stringify(cellsArr)}`); + const kernel = this.getSelectedOrSuggestedKernel(notebook); + if (kernel) { + await kernel.cancelNotebookCellExecution(notebook.uri, cellsArr); + } + } + + async cancelNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise { + this.cancelNotebookCellHandles(notebook, Array.from(cells, cell => cell.handle)); + } + + updateNotebookCellExecution(notebook: URI, cellHandle: number, updates: ICellExecuteUpdate[]): void { + const notebookExecution = this._executions.get(notebook); + if (!notebookExecution) { + this._logService.error(`notebook execution not found for ${notebook}`); + return; + } + + notebookExecution.updateExecution(cellHandle, updates); + } + + completeNotebookCellExecution(notebook: URI, cellHandle: number, complete: ICellExecutionComplete): void { + const notebookExecution = this._executions.get(notebook); + if (!notebookExecution) { + this._logService.error(`notebook execution not found for ${notebook}`); + return; + } + + notebookExecution.completeExecution(cellHandle, complete); + this.checkNotebookExecutionEmpty(notebook); + } + + private checkNotebookExecutionEmpty(notebook: URI): void { + const notebookExecution = this._executions.get(notebook); + if (!notebookExecution) { + return; + } + + if (notebookExecution.isEmpty()) { + this._logService.debug(`NotebookExecution#dispose ${notebook.toString()}`); + notebookExecution.dispose(); + this._executions.delete(notebook); + } + } + + override dispose(): void { + super.dispose(); + this._executions.forEach(e => e.dispose()); + this._executions.clear(); } } -function updateToEdit(update: ICellExecuteUpdate, cellHandle: number, model: NotebookCellTextModel): ICellEditOperation { +class NotebookExecution extends Disposable { + private readonly _notebookModel: NotebookTextModel; + + private readonly _cellExecutions = new Map(); + + constructor( + notebook: URI, + @INotebookService private readonly _notebookService: INotebookService, + @IInstantiationService private readonly _instantiationService: IInstantiationService, + @INotebookExecutionService private readonly _notebookExecutionService: INotebookExecutionService, + @ILogService private readonly _logService: ILogService, + ) { + super(); + this._logService.debug(`NotebookExecution#ctor ${notebook.toString()}`); + + const notebookModel = this._notebookService.getNotebookTextModel(notebook); + if (!notebookModel) { + throw new Error('Notebook not found: ' + notebook); + } + + this._notebookModel = notebookModel; + this._register(this._notebookModel.onWillAddRemoveCells(e => this.onWillAddRemoveCells(e))); + this._register(this._notebookModel.onWillDispose(() => this.onWillDisposeDocument())); + } + + private cellLog(cellHandle: number): string { + return `${this._notebookModel.uri.toString()}, ${cellHandle}`; + } + + isEmpty(): boolean { + return this._cellExecutions.size === 0; + } + + cancelAll(): void { + this._logService.debug(`NotebookExecution#cancelAll`); + this._notebookExecutionService.cancelNotebookCellHandles(this._notebookModel, this._cellExecutions.keys()); + } + + addExecution(cellHandle: number) { + this._logService.debug(`NotebookExecution#addExecution ${this.cellLog(cellHandle)}`); + const execution = this._instantiationService.createInstance(CellExecution, cellHandle, this._notebookModel); + this._cellExecutions.set(cellHandle, execution); + } + + updateExecution(cellHandle: number, updates: ICellExecuteUpdate[]): void { + this.logUpdates(cellHandle, updates); + const execution = this._cellExecutions.get(cellHandle); + if (!execution) { + this._logService.error(`no execution for cell ${cellHandle}`); + return; + } + + execution.update(updates); + } + + private logUpdates(cellHandle: number, updates: ICellExecuteUpdate[]): void { + const updateTypes = updates.map(u => CellExecutionUpdateType[u.editType]).join(', '); + this._logService.debug(`NotebookExecution#updateExecution ${this.cellLog(cellHandle)}, [${updateTypes}]`); + } + + completeExecution(cellHandle: number, complete: ICellExecutionComplete): void { + this._logService.debug(`NotebookExecution#completeExecution ${this.cellLog(cellHandle)}`); + + const execution = this._cellExecutions.get(cellHandle); + if (!execution) { + this._logService.error(`no execution for cell ${cellHandle}`); + return; + } + + try { + execution.complete(complete); + } finally { + this._cellExecutions.delete(cellHandle); + } + } + + private onWillDisposeDocument(): void { + this._logService.debug(`NotebookExecution#onWillDisposeDocument`); + this.cancelAll(); + } + + private onWillAddRemoveCells(e: NotebookTextModelWillAddRemoveEvent): void { + const handles = new Set(this._cellExecutions.keys()); + const myDeletedHandles = new Set(); + e.rawEvent.changes.forEach(([start, deleteCount]) => { + if (deleteCount) { + const deletedHandles = this._notebookModel.cells.slice(start, start + deleteCount).map(c => c.handle); + deletedHandles.forEach(h => { + if (handles.has(h)) { + myDeletedHandles.add(h); + } + }); + } + + return false; + }); + + if (myDeletedHandles.size) { + this._logService.debug(`NotebookExecution#onWillAddRemoveCells, ${JSON.stringify([...myDeletedHandles])}`); + this._notebookExecutionService.cancelNotebookCellHandles(this._notebookModel, myDeletedHandles); + } + } +} + +function updateToEdit(update: ICellExecuteUpdate, cellHandle: number): ICellEditOperation { if (update.editType === CellExecutionUpdateType.Output) { return { editType: CellEditType.Output, @@ -39,19 +270,6 @@ function updateToEdit(update: ICellExecuteUpdate, cellHandle: number, model: Not append: update.append, outputId: update.outputId }; - } else if (update.editType === CellExecutionUpdateType.Complete) { - return { - editType: CellEditType.PartialInternalMetadata, - handle: cellHandle, - internalMetadata: { - runState: null, - lastRunSuccess: update.lastRunSuccess, - runStartTime: model.internalMetadata.didPause ? null : model.internalMetadata.runStartTime, - runEndTime: model.internalMetadata.didPause ? null : update.runEndTime, - isPaused: false, - didPause: false - } - }; } else if (update.editType === CellExecutionUpdateType.ExecutionState) { const newInternalMetadata: Partial = { runState: NotebookCellExecutionState.Executing, @@ -72,26 +290,14 @@ function updateToEdit(update: ICellExecuteUpdate, cellHandle: number, model: Not throw new Error('Unknown cell update type'); } -class CellExecution implements INotebookCellExecution, IDisposable { - private readonly _notebookModel: NotebookTextModel; - - private _isDisposed = false; - +class CellExecution { constructor( - readonly notebook: URI, - readonly cellHandle: number, - private readonly _notebookService: INotebookService, + private readonly _cellHandle: number, + private readonly _notebookModel: NotebookTextModel, ) { - const notebookModel = this._notebookService.getNotebookTextModel(notebook); - if (!notebookModel) { - throw new Error('Notebook not found: ' + notebook); - } - - this._notebookModel = notebookModel; - const startExecuteEdit: ICellEditOperation = { editType: CellEditType.PartialInternalMetadata, - handle: cellHandle, + handle: this._cellHandle, internalMetadata: { runState: NotebookCellExecutionState.Pending, executionOrder: null, @@ -102,25 +308,29 @@ class CellExecution implements INotebookCellExecution, IDisposable { } update(updates: ICellExecuteUpdate[]): void { - if (this._isDisposed) { - throw new Error('Cannot update disposed execution'); - } - - const cellModel = this._notebookModel.cells.find(c => c.handle === this.cellHandle); - if (!cellModel) { - throw new Error('Cell not found: ' + this.cellHandle); - } - - const edits = updates.map(update => updateToEdit(update, this.cellHandle, cellModel)); + const edits = updates.map(update => updateToEdit(update, this._cellHandle)); this._applyExecutionEdits(edits); - - if (updates.some(u => u.editType === CellExecutionUpdateType.Complete)) { - this.dispose(); - } } - dispose(): void { - this._isDisposed = true; + complete(completionData: ICellExecutionComplete): void { + const cellModel = this._notebookModel.cells.find(c => c.handle === this._cellHandle); + if (!cellModel) { + throw new Error('Cell not found: ' + this._cellHandle); + } + + const edit: ICellEditOperation = { + editType: CellEditType.PartialInternalMetadata, + handle: this._cellHandle, + internalMetadata: { + runState: null, + lastRunSuccess: completionData.lastRunSuccess, + runStartTime: cellModel.internalMetadata.didPause ? null : cellModel.internalMetadata.runStartTime, + runEndTime: cellModel.internalMetadata.didPause ? null : completionData.runEndTime, + isPaused: false, + didPause: false + } + }; + this._applyExecutionEdits([edit]); } private _applyExecutionEdits(edits: ICellEditOperation[]): void { diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts index c6ebc80fc36..27ec81cefc9 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookTextModel.ts @@ -159,6 +159,7 @@ export class NotebookEventEmitter extends PauseableEmitter = this._register(new Emitter()); private readonly _onWillAddRemoveCells = this._register(new Emitter()); private readonly _onDidChangeContent = this._register(new Emitter()); @@ -344,6 +345,12 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel } override dispose() { + if (this._isDisposed) { + // NotebookEditorModel can be disposed twice, don't fire onWillDispose again + return; + } + + this._isDisposed = true; this._onWillDispose.fire(); this._undoService.removeElements(this.uri); diff --git a/src/vs/workbench/contrib/notebook/common/notebookExecutionService.ts b/src/vs/workbench/contrib/notebook/common/notebookExecutionService.ts index 0f14b023a82..72568f60fa5 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookExecutionService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookExecutionService.ts @@ -5,13 +5,14 @@ import { URI } from 'vs/base/common/uri'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; -import { IOutputDto, IOutputItemDto } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { ICellViewModel } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { INotebookTextModel, IOutputDto, IOutputItemDto } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { INotebookKernel } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; export enum CellExecutionUpdateType { Output = 1, OutputItems = 2, ExecutionState = 3, - Complete = 4, } export interface ICellExecuteOutputEdit { @@ -28,7 +29,7 @@ export interface ICellExecuteOutputItemEdit { items: IOutputItemDto[] } -export type ICellExecuteUpdate = ICellExecuteOutputEdit | ICellExecuteOutputItemEdit | ICellExecutionStateUpdate | ICellExecutionComplete; +export type ICellExecuteUpdate = ICellExecuteOutputEdit | ICellExecuteOutputItemEdit | ICellExecutionStateUpdate; export interface ICellExecutionStateUpdate { editType: CellExecutionUpdateType.ExecutionState; @@ -37,21 +38,20 @@ export interface ICellExecutionStateUpdate { } export interface ICellExecutionComplete { - editType: CellExecutionUpdateType.Complete; runEndTime?: number; lastRunSuccess?: boolean; } -export interface INotebookCellExecution { - readonly notebook: URI; - readonly cellHandle: number; - update(updates: ICellExecuteUpdate[]): void; -} - export const INotebookExecutionService = createDecorator('INotebookExecutionService'); export interface INotebookExecutionService { _serviceBrand: undefined; - createNotebookCellExecution(notebook: URI, cellHandle: number): INotebookCellExecution; + createNotebookCellExecution(notebook: URI, cellHandle: number): void; + updateNotebookCellExecution(notebook: URI, cellHandle: number, updates: ICellExecuteUpdate[]): void; + completeNotebookCellExecution(notebook: URI, cellHandle: number, complete: ICellExecutionComplete): void; + getSelectedOrSuggestedKernel(notebook: INotebookTextModel): INotebookKernel | undefined; + executeNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise; + cancelNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise; + cancelNotebookCellHandles(notebook: INotebookTextModel, cells: Iterable): Promise; } From 25d41d15f905ef4ea3403890aada3289ecc2b11a Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Fri, 10 Dec 2021 14:54:10 -0800 Subject: [PATCH 2/7] Split Execution and ExecutionState services --- .../api/browser/mainThreadNotebookDto.ts | 3 +- .../api/browser/mainThreadNotebookKernels.ts | 13 +- .../workbench/api/common/extHost.protocol.ts | 3 +- .../notebook/browser/notebook.contribution.ts | 3 + .../browser/notebookExecutionServiceImpl.ts | 263 +---------------- .../notebookExecutionStateServiceImpl.ts | 277 ++++++++++++++++++ .../common/notebookExecutionService.ts | 17 -- .../common/notebookExecutionStateService.ts | 31 ++ ...st.ts => notebookExecutionService.test.ts} | 38 +-- .../notebookExecutionStateService.test.ts | 116 ++++++++ 10 files changed, 459 insertions(+), 305 deletions(-) create mode 100644 src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts create mode 100644 src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts rename src/vs/workbench/contrib/notebook/test/browser/{notebookEditorKernelManager.test.ts => notebookExecutionService.test.ts} (85%) create mode 100644 src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts diff --git a/src/vs/workbench/api/browser/mainThreadNotebookDto.ts b/src/vs/workbench/api/browser/mainThreadNotebookDto.ts index d63c6412198..893100761b0 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookDto.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookDto.ts @@ -6,7 +6,8 @@ import * as extHostProtocol from 'vs/workbench/api/common/extHost.protocol'; import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import * as notebookCommon from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { CellExecutionUpdateType, ICellExecuteUpdate, ICellExecutionComplete } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; +import { CellExecutionUpdateType } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; +import { ICellExecuteUpdate, ICellExecutionComplete } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; export namespace NotebookDto { diff --git a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts index 71aa6d1b0a4..c53fec123a0 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts @@ -15,7 +15,7 @@ import { extHostNamedCustomer } from 'vs/workbench/api/common/extHostCustomers'; import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/notebookEditorService'; import { CellUri } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; +import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { INotebookKernel, INotebookKernelChangeEvent, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { SerializableObjectWithBuffers } from 'vs/workbench/services/extensions/common/proxyIdentifier'; import { ExtHostContext, ExtHostNotebookKernelsShape, ICellExecuteUpdateDto, ICellExecutionCompleteDto, IExtHostContext, INotebookKernelDto2, MainContext, MainThreadNotebookKernelsShape } from '../common/extHost.protocol'; @@ -113,8 +113,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape extHostContext: IExtHostContext, @ILanguageService private readonly _languageService: ILanguageService, @INotebookKernelService private readonly _notebookKernelService: INotebookKernelService, - @INotebookExecutionService private readonly _notebookExecutionService: INotebookExecutionService, - // @INotebookService private readonly _notebookService: INotebookService, + @INotebookExecutionStateService private readonly _notebookExecutionStateService: INotebookExecutionStateService, @INotebookEditorService notebookEditorService: INotebookEditorService ) { this._proxy = extHostContext.getProxy(ExtHostContext.ExtHostNotebookKernels); @@ -128,7 +127,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape this._executions.forEach(e => { const uri = CellUri.parse(URI.parse(e)); if (uri) { - this._notebookExecutionService.completeNotebookCellExecution(uri.notebook, uri.handle, { }); + this._notebookExecutionStateService.completeNotebookCellExecution(uri.notebook, uri.handle, { }); } }); })); @@ -248,7 +247,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape $addExecution(rawUri: UriComponents, cellHandle: number): void { const uri = URI.revive(rawUri); - this._notebookExecutionService.createNotebookCellExecution(uri, cellHandle); + this._notebookExecutionStateService.createNotebookCellExecution(uri, cellHandle); const cellUri = CellUri.generateCellUri(uri, cellHandle, uri.scheme); this._executions.add(cellUri.toString()); @@ -262,7 +261,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape try { const uri = URI.revive(first.uri); - this._notebookExecutionService.updateNotebookCellExecution(uri, first.cellHandle, datas.map(NotebookDto.fromCellExecuteUpdateDto)); + this._notebookExecutionStateService.updateNotebookCellExecution(uri, first.cellHandle, datas.map(NotebookDto.fromCellExecuteUpdateDto)); } catch (e) { onUnexpectedError(e); } @@ -271,7 +270,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape $completeExecution(rawUri: UriComponents, cellHandle: number, data: SerializableObjectWithBuffers): void { const uri = URI.revive(rawUri); - this._notebookExecutionService.completeNotebookCellExecution(uri, cellHandle, NotebookDto.fromCellExecuteCompleteDto(data.value)); + this._notebookExecutionStateService.completeNotebookCellExecution(uri, cellHandle, NotebookDto.fromCellExecuteCompleteDto(data.value)); const cellUri = CellUri.generateCellUri(uri, cellHandle, uri.scheme); this._executions.delete(cellUri.toString()); diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 4742f3bdf5d..a06e93b8668 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -54,7 +54,8 @@ import { IRevealOptions, ITreeItem } from 'vs/workbench/common/views'; import { CallHierarchyItem } from 'vs/workbench/contrib/callHierarchy/common/callHierarchy'; import { IAdapterDescriptor, IConfig, IDebugSessionReplMode } from 'vs/workbench/contrib/debug/common/debug'; import * as notebookCommon from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { CellExecutionUpdateType, ICellExecutionComplete, ICellExecutionStateUpdate } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; +import { CellExecutionUpdateType } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; +import { ICellExecutionComplete, ICellExecutionStateUpdate } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange'; import { OutputChannelUpdateMode } from 'vs/workbench/contrib/output/common/output'; import { InputValidationType } from 'vs/workbench/contrib/scm/common/scm'; diff --git a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts index 7449d1148ec..369f05b2a9d 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts @@ -95,12 +95,14 @@ import 'vs/workbench/contrib/notebook/browser/diff/notebookDiffActions'; // Output renderers registration import 'vs/workbench/contrib/notebook/browser/view/output/transforms/richTransform'; import { editorOptionsRegistry } from 'vs/editor/common/config/editorOptions'; +import { NotebookExecutionStateService } from 'vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl'; import { NotebookExecutionService } from 'vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl'; import { INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; import { INotebookKeymapService } from 'vs/workbench/contrib/notebook/common/notebookKeymapService'; import { NotebookKeymapService } from 'vs/workbench/contrib/notebook/browser/notebookKeymapServiceImpl'; import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import { PLAINTEXT_LANGUAGE_ID } from 'vs/editor/common/modes/modesRegistry'; +import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; /*--------------------------------------------------------------------------------------------- */ @@ -627,6 +629,7 @@ registerSingleton(INotebookCellStatusBarService, NotebookCellStatusBarService, t registerSingleton(INotebookEditorService, NotebookEditorWidgetService, true); registerSingleton(INotebookKernelService, NotebookKernelService, true); registerSingleton(INotebookExecutionService, NotebookExecutionService, true); +registerSingleton(INotebookExecutionStateService, NotebookExecutionStateService, true); registerSingleton(INotebookRendererMessagingService, NotebookRendererMessagingService, true); registerSingleton(INotebookKeymapService, NotebookKeymapService, true); diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts index 445612437f7..ab7580d55b8 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts @@ -3,55 +3,25 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { Disposable } from 'vs/base/common/lifecycle'; -import { ResourceMap } from 'vs/base/common/map'; -import { URI } from 'vs/base/common/uri'; import * as nls from 'vs/nls'; import { ICommandService } from 'vs/platform/commands/common/commands'; -import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { ILogService } from 'vs/platform/log/common/log'; import { IWorkspaceTrustRequestService } from 'vs/platform/workspace/common/workspaceTrust'; import { SELECT_KERNEL_ID } from 'vs/workbench/contrib/notebook/browser/controller/coreActions'; import { ICellViewModel } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; -import { CellEditType, CellKind, ICellEditOperation, INotebookTextModel, NotebookCellExecutionState, NotebookCellInternalMetadata, NotebookTextModelWillAddRemoveEvent } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { CellExecutionUpdateType, ICellExecuteUpdate, ICellExecutionComplete, INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; +import { CellKind, INotebookTextModel, NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; import { INotebookKernel, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; -import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; -export class NotebookExecutionService extends Disposable implements INotebookExecutionService { +export class NotebookExecutionService implements INotebookExecutionService { declare _serviceBrand: undefined; - private readonly _executions = new ResourceMap(); - constructor( @ICommandService private readonly _commandService: ICommandService, @INotebookKernelService private readonly _notebookKernelService: INotebookKernelService, @IWorkspaceTrustRequestService private readonly _workspaceTrustRequestService: IWorkspaceTrustRequestService, - @IInstantiationService private readonly _instantiationService: IInstantiationService, @ILogService private readonly _logService: ILogService, ) { - super(); - - this._register(this._notebookKernelService.onDidChangeSelectedNotebooks(e => { - if (e.newKernel) { - const notebookExecution = this._executions.get(e.notebook); - if (notebookExecution) { - notebookExecution.cancelAll(); - this.checkNotebookExecutionEmpty(e.notebook); - } - } - })); - } - - createNotebookCellExecution(notebook: URI, cellHandle: number): void { - let notebookExecution = this._executions.get(notebook); - if (!notebookExecution) { - notebookExecution = this._instantiationService.createInstance(NotebookExecution, notebook); - this._executions.set(notebook, notebookExecution); - } - - notebookExecution.addExecution(cellHandle); } getSelectedOrSuggestedKernel(notebook: INotebookTextModel): INotebookKernel | undefined { @@ -109,231 +79,4 @@ export class NotebookExecutionService extends Disposable implements INotebookExe async cancelNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise { this.cancelNotebookCellHandles(notebook, Array.from(cells, cell => cell.handle)); } - - updateNotebookCellExecution(notebook: URI, cellHandle: number, updates: ICellExecuteUpdate[]): void { - const notebookExecution = this._executions.get(notebook); - if (!notebookExecution) { - this._logService.error(`notebook execution not found for ${notebook}`); - return; - } - - notebookExecution.updateExecution(cellHandle, updates); - } - - completeNotebookCellExecution(notebook: URI, cellHandle: number, complete: ICellExecutionComplete): void { - const notebookExecution = this._executions.get(notebook); - if (!notebookExecution) { - this._logService.error(`notebook execution not found for ${notebook}`); - return; - } - - notebookExecution.completeExecution(cellHandle, complete); - this.checkNotebookExecutionEmpty(notebook); - } - - private checkNotebookExecutionEmpty(notebook: URI): void { - const notebookExecution = this._executions.get(notebook); - if (!notebookExecution) { - return; - } - - if (notebookExecution.isEmpty()) { - this._logService.debug(`NotebookExecution#dispose ${notebook.toString()}`); - notebookExecution.dispose(); - this._executions.delete(notebook); - } - } - - override dispose(): void { - super.dispose(); - this._executions.forEach(e => e.dispose()); - this._executions.clear(); - } -} - -class NotebookExecution extends Disposable { - private readonly _notebookModel: NotebookTextModel; - - private readonly _cellExecutions = new Map(); - - constructor( - notebook: URI, - @INotebookService private readonly _notebookService: INotebookService, - @IInstantiationService private readonly _instantiationService: IInstantiationService, - @INotebookExecutionService private readonly _notebookExecutionService: INotebookExecutionService, - @ILogService private readonly _logService: ILogService, - ) { - super(); - this._logService.debug(`NotebookExecution#ctor ${notebook.toString()}`); - - const notebookModel = this._notebookService.getNotebookTextModel(notebook); - if (!notebookModel) { - throw new Error('Notebook not found: ' + notebook); - } - - this._notebookModel = notebookModel; - this._register(this._notebookModel.onWillAddRemoveCells(e => this.onWillAddRemoveCells(e))); - this._register(this._notebookModel.onWillDispose(() => this.onWillDisposeDocument())); - } - - private cellLog(cellHandle: number): string { - return `${this._notebookModel.uri.toString()}, ${cellHandle}`; - } - - isEmpty(): boolean { - return this._cellExecutions.size === 0; - } - - cancelAll(): void { - this._logService.debug(`NotebookExecution#cancelAll`); - this._notebookExecutionService.cancelNotebookCellHandles(this._notebookModel, this._cellExecutions.keys()); - } - - addExecution(cellHandle: number) { - this._logService.debug(`NotebookExecution#addExecution ${this.cellLog(cellHandle)}`); - const execution = this._instantiationService.createInstance(CellExecution, cellHandle, this._notebookModel); - this._cellExecutions.set(cellHandle, execution); - } - - updateExecution(cellHandle: number, updates: ICellExecuteUpdate[]): void { - this.logUpdates(cellHandle, updates); - const execution = this._cellExecutions.get(cellHandle); - if (!execution) { - this._logService.error(`no execution for cell ${cellHandle}`); - return; - } - - execution.update(updates); - } - - private logUpdates(cellHandle: number, updates: ICellExecuteUpdate[]): void { - const updateTypes = updates.map(u => CellExecutionUpdateType[u.editType]).join(', '); - this._logService.debug(`NotebookExecution#updateExecution ${this.cellLog(cellHandle)}, [${updateTypes}]`); - } - - completeExecution(cellHandle: number, complete: ICellExecutionComplete): void { - this._logService.debug(`NotebookExecution#completeExecution ${this.cellLog(cellHandle)}`); - - const execution = this._cellExecutions.get(cellHandle); - if (!execution) { - this._logService.error(`no execution for cell ${cellHandle}`); - return; - } - - try { - execution.complete(complete); - } finally { - this._cellExecutions.delete(cellHandle); - } - } - - private onWillDisposeDocument(): void { - this._logService.debug(`NotebookExecution#onWillDisposeDocument`); - this.cancelAll(); - } - - private onWillAddRemoveCells(e: NotebookTextModelWillAddRemoveEvent): void { - const handles = new Set(this._cellExecutions.keys()); - const myDeletedHandles = new Set(); - e.rawEvent.changes.forEach(([start, deleteCount]) => { - if (deleteCount) { - const deletedHandles = this._notebookModel.cells.slice(start, start + deleteCount).map(c => c.handle); - deletedHandles.forEach(h => { - if (handles.has(h)) { - myDeletedHandles.add(h); - } - }); - } - - return false; - }); - - if (myDeletedHandles.size) { - this._logService.debug(`NotebookExecution#onWillAddRemoveCells, ${JSON.stringify([...myDeletedHandles])}`); - this._notebookExecutionService.cancelNotebookCellHandles(this._notebookModel, myDeletedHandles); - } - } -} - -function updateToEdit(update: ICellExecuteUpdate, cellHandle: number): ICellEditOperation { - if (update.editType === CellExecutionUpdateType.Output) { - return { - editType: CellEditType.Output, - handle: update.cellHandle, - append: update.append, - outputs: update.outputs, - }; - } else if (update.editType === CellExecutionUpdateType.OutputItems) { - return { - editType: CellEditType.OutputItems, - items: update.items, - append: update.append, - outputId: update.outputId - }; - } else if (update.editType === CellExecutionUpdateType.ExecutionState) { - const newInternalMetadata: Partial = { - runState: NotebookCellExecutionState.Executing, - }; - if (typeof update.executionOrder !== 'undefined') { - newInternalMetadata.executionOrder = update.executionOrder; - } - if (typeof update.runStartTime !== 'undefined') { - newInternalMetadata.runStartTime = update.runStartTime; - } - return { - editType: CellEditType.PartialInternalMetadata, - handle: cellHandle, - internalMetadata: newInternalMetadata - }; - } - - throw new Error('Unknown cell update type'); -} - -class CellExecution { - constructor( - private readonly _cellHandle: number, - private readonly _notebookModel: NotebookTextModel, - ) { - const startExecuteEdit: ICellEditOperation = { - editType: CellEditType.PartialInternalMetadata, - handle: this._cellHandle, - internalMetadata: { - runState: NotebookCellExecutionState.Pending, - executionOrder: null, - didPause: false - } - }; - this._applyExecutionEdits([startExecuteEdit]); - } - - update(updates: ICellExecuteUpdate[]): void { - const edits = updates.map(update => updateToEdit(update, this._cellHandle)); - this._applyExecutionEdits(edits); - } - - complete(completionData: ICellExecutionComplete): void { - const cellModel = this._notebookModel.cells.find(c => c.handle === this._cellHandle); - if (!cellModel) { - throw new Error('Cell not found: ' + this._cellHandle); - } - - const edit: ICellEditOperation = { - editType: CellEditType.PartialInternalMetadata, - handle: this._cellHandle, - internalMetadata: { - runState: null, - lastRunSuccess: completionData.lastRunSuccess, - runStartTime: cellModel.internalMetadata.didPause ? null : cellModel.internalMetadata.runStartTime, - runEndTime: cellModel.internalMetadata.didPause ? null : completionData.runEndTime, - isPaused: false, - didPause: false - } - }; - this._applyExecutionEdits([edit]); - } - - private _applyExecutionEdits(edits: ICellEditOperation[]): void { - this._notebookModel.applyEdits(edits, true, undefined, () => undefined, undefined, false); - } } diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts new file mode 100644 index 00000000000..327e42e8307 --- /dev/null +++ b/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts @@ -0,0 +1,277 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Disposable } from 'vs/base/common/lifecycle'; +import { ResourceMap } from 'vs/base/common/map'; +import { URI } from 'vs/base/common/uri'; +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, ICellEditOperation, NotebookCellExecutionState, NotebookCellInternalMetadata, NotebookTextModelWillAddRemoveEvent } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellExecutionUpdateType, INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; +import { ICellExecuteUpdate, ICellExecutionComplete, INotebookExecutionStateService } 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'; + +export class NotebookExecutionStateService extends Disposable implements INotebookExecutionStateService { + declare _serviceBrand: undefined; + + private readonly _executions = new ResourceMap(); + + constructor( + @INotebookKernelService private readonly _notebookKernelService: INotebookKernelService, + @IInstantiationService private readonly _instantiationService: IInstantiationService, + @ILogService private readonly _logService: ILogService, + ) { + super(); + + this._register(this._notebookKernelService.onDidChangeSelectedNotebooks(e => { + if (e.newKernel) { + const notebookExecution = this._executions.get(e.notebook); + if (notebookExecution) { + notebookExecution.cancelAll(); + this.checkNotebookExecutionEmpty(e.notebook); + } + } + })); + } + + createNotebookCellExecution(notebook: URI, cellHandle: number): void { + let notebookExecution = this._executions.get(notebook); + if (!notebookExecution) { + notebookExecution = this._instantiationService.createInstance(NotebookExecution, notebook); + this._executions.set(notebook, notebookExecution); + } + + notebookExecution.addExecution(cellHandle); + } + + updateNotebookCellExecution(notebook: URI, cellHandle: number, updates: ICellExecuteUpdate[]): void { + const notebookExecution = this._executions.get(notebook); + if (!notebookExecution) { + this._logService.error(`notebook execution not found for ${notebook}`); + return; + } + + notebookExecution.updateExecution(cellHandle, updates); + } + + completeNotebookCellExecution(notebook: URI, cellHandle: number, complete: ICellExecutionComplete): void { + const notebookExecution = this._executions.get(notebook); + if (!notebookExecution) { + this._logService.error(`notebook execution not found for ${notebook}`); + return; + } + + notebookExecution.completeExecution(cellHandle, complete); + this.checkNotebookExecutionEmpty(notebook); + } + + private checkNotebookExecutionEmpty(notebook: URI): void { + const notebookExecution = this._executions.get(notebook); + if (!notebookExecution) { + return; + } + + if (notebookExecution.isEmpty()) { + this._logService.debug(`NotebookExecution#dispose ${notebook.toString()}`); + notebookExecution.dispose(); + this._executions.delete(notebook); + } + } + + override dispose(): void { + super.dispose(); + this._executions.forEach(e => e.dispose()); + this._executions.clear(); + } +} + +class NotebookExecution extends Disposable { + private readonly _notebookModel: NotebookTextModel; + + private readonly _cellExecutions = new Map(); + + constructor( + notebook: URI, + @INotebookService private readonly _notebookService: INotebookService, + @IInstantiationService private readonly _instantiationService: IInstantiationService, + @INotebookExecutionService private readonly _notebookExecutionService: INotebookExecutionService, + @ILogService private readonly _logService: ILogService, + ) { + super(); + this._logService.debug(`NotebookExecution#ctor ${notebook.toString()}`); + + const notebookModel = this._notebookService.getNotebookTextModel(notebook); + if (!notebookModel) { + throw new Error('Notebook not found: ' + notebook); + } + + this._notebookModel = notebookModel; + this._register(this._notebookModel.onWillAddRemoveCells(e => this.onWillAddRemoveCells(e))); + this._register(this._notebookModel.onWillDispose(() => this.onWillDisposeDocument())); + } + + private cellLog(cellHandle: number): string { + return `${this._notebookModel.uri.toString()}, ${cellHandle}`; + } + + isEmpty(): boolean { + return this._cellExecutions.size === 0; + } + + cancelAll(): void { + this._logService.debug(`NotebookExecution#cancelAll`); + this._notebookExecutionService.cancelNotebookCellHandles(this._notebookModel, this._cellExecutions.keys()); + } + + addExecution(cellHandle: number) { + this._logService.debug(`NotebookExecution#addExecution ${this.cellLog(cellHandle)}`); + const execution = this._instantiationService.createInstance(CellExecution, cellHandle, this._notebookModel); + this._cellExecutions.set(cellHandle, execution); + } + + updateExecution(cellHandle: number, updates: ICellExecuteUpdate[]): void { + this.logUpdates(cellHandle, updates); + const execution = this._cellExecutions.get(cellHandle); + if (!execution) { + this._logService.error(`no execution for cell ${cellHandle}`); + return; + } + + execution.update(updates); + } + + private logUpdates(cellHandle: number, updates: ICellExecuteUpdate[]): void { + const updateTypes = updates.map(u => CellExecutionUpdateType[u.editType]).join(', '); + this._logService.debug(`NotebookExecution#updateExecution ${this.cellLog(cellHandle)}, [${updateTypes}]`); + } + + completeExecution(cellHandle: number, complete: ICellExecutionComplete): void { + this._logService.debug(`NotebookExecution#completeExecution ${this.cellLog(cellHandle)}`); + + const execution = this._cellExecutions.get(cellHandle); + if (!execution) { + this._logService.error(`no execution for cell ${cellHandle}`); + return; + } + + try { + execution.complete(complete); + } finally { + this._cellExecutions.delete(cellHandle); + } + } + + private onWillDisposeDocument(): void { + this._logService.debug(`NotebookExecution#onWillDisposeDocument`); + this.cancelAll(); + } + + private onWillAddRemoveCells(e: NotebookTextModelWillAddRemoveEvent): void { + const handles = new Set(this._cellExecutions.keys()); + const myDeletedHandles = new Set(); + e.rawEvent.changes.forEach(([start, deleteCount]) => { + if (deleteCount) { + const deletedHandles = this._notebookModel.cells.slice(start, start + deleteCount).map(c => c.handle); + deletedHandles.forEach(h => { + if (handles.has(h)) { + myDeletedHandles.add(h); + } + }); + } + + return false; + }); + + if (myDeletedHandles.size) { + this._logService.debug(`NotebookExecution#onWillAddRemoveCells, ${JSON.stringify([...myDeletedHandles])}`); + this._notebookExecutionService.cancelNotebookCellHandles(this._notebookModel, myDeletedHandles); + } + } +} + +function updateToEdit(update: ICellExecuteUpdate, cellHandle: number): ICellEditOperation { + if (update.editType === CellExecutionUpdateType.Output) { + return { + editType: CellEditType.Output, + handle: update.cellHandle, + append: update.append, + outputs: update.outputs, + }; + } else if (update.editType === CellExecutionUpdateType.OutputItems) { + return { + editType: CellEditType.OutputItems, + items: update.items, + append: update.append, + outputId: update.outputId + }; + } else if (update.editType === CellExecutionUpdateType.ExecutionState) { + const newInternalMetadata: Partial = { + runState: NotebookCellExecutionState.Executing, + }; + if (typeof update.executionOrder !== 'undefined') { + newInternalMetadata.executionOrder = update.executionOrder; + } + if (typeof update.runStartTime !== 'undefined') { + newInternalMetadata.runStartTime = update.runStartTime; + } + return { + editType: CellEditType.PartialInternalMetadata, + handle: cellHandle, + internalMetadata: newInternalMetadata + }; + } + + throw new Error('Unknown cell update type'); +} + +class CellExecution { + constructor( + private readonly _cellHandle: number, + private readonly _notebookModel: NotebookTextModel, + ) { + const startExecuteEdit: ICellEditOperation = { + editType: CellEditType.PartialInternalMetadata, + handle: this._cellHandle, + internalMetadata: { + runState: NotebookCellExecutionState.Pending, + executionOrder: null, + didPause: false + } + }; + this._applyExecutionEdits([startExecuteEdit]); + } + + update(updates: ICellExecuteUpdate[]): void { + const edits = updates.map(update => updateToEdit(update, this._cellHandle)); + this._applyExecutionEdits(edits); + } + + complete(completionData: ICellExecutionComplete): void { + const cellModel = this._notebookModel.cells.find(c => c.handle === this._cellHandle); + if (!cellModel) { + throw new Error('Cell not found: ' + this._cellHandle); + } + + const edit: ICellEditOperation = { + editType: CellEditType.PartialInternalMetadata, + handle: this._cellHandle, + internalMetadata: { + runState: null, + lastRunSuccess: completionData.lastRunSuccess, + runStartTime: cellModel.internalMetadata.didPause ? null : cellModel.internalMetadata.runStartTime, + runEndTime: cellModel.internalMetadata.didPause ? null : completionData.runEndTime, + isPaused: false, + didPause: false + } + }; + this._applyExecutionEdits([edit]); + } + + private _applyExecutionEdits(edits: ICellEditOperation[]): void { + this._notebookModel.applyEdits(edits, true, undefined, () => undefined, undefined, false); + } +} diff --git a/src/vs/workbench/contrib/notebook/common/notebookExecutionService.ts b/src/vs/workbench/contrib/notebook/common/notebookExecutionService.ts index 72568f60fa5..0013cfbfbfa 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookExecutionService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookExecutionService.ts @@ -3,7 +3,6 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { URI } from 'vs/base/common/uri'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; import { ICellViewModel } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { INotebookTextModel, IOutputDto, IOutputItemDto } from 'vs/workbench/contrib/notebook/common/notebookCommon'; @@ -29,27 +28,11 @@ export interface ICellExecuteOutputItemEdit { items: IOutputItemDto[] } -export type ICellExecuteUpdate = ICellExecuteOutputEdit | ICellExecuteOutputItemEdit | ICellExecutionStateUpdate; - -export interface ICellExecutionStateUpdate { - editType: CellExecutionUpdateType.ExecutionState; - executionOrder?: number; - runStartTime?: number; -} - -export interface ICellExecutionComplete { - runEndTime?: number; - lastRunSuccess?: boolean; -} - export const INotebookExecutionService = createDecorator('INotebookExecutionService'); export interface INotebookExecutionService { _serviceBrand: undefined; - createNotebookCellExecution(notebook: URI, cellHandle: number): void; - updateNotebookCellExecution(notebook: URI, cellHandle: number, updates: ICellExecuteUpdate[]): void; - completeNotebookCellExecution(notebook: URI, cellHandle: number, complete: ICellExecutionComplete): void; getSelectedOrSuggestedKernel(notebook: INotebookTextModel): INotebookKernel | undefined; executeNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise; cancelNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise; diff --git a/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts b/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts new file mode 100644 index 00000000000..b39ff197eae --- /dev/null +++ b/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts @@ -0,0 +1,31 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { URI } from 'vs/base/common/uri'; +import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; +import { CellExecutionUpdateType, ICellExecuteOutputEdit, ICellExecuteOutputItemEdit } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; + +export type ICellExecuteUpdate = ICellExecuteOutputEdit | ICellExecuteOutputItemEdit | ICellExecutionStateUpdate; + +export interface ICellExecutionStateUpdate { + editType: CellExecutionUpdateType.ExecutionState; + executionOrder?: number; + runStartTime?: number; +} + +export interface ICellExecutionComplete { + runEndTime?: number; + lastRunSuccess?: boolean; +} + +export const INotebookExecutionStateService = createDecorator('INotebookExecutionStateService'); + +export interface INotebookExecutionStateService { + _serviceBrand: undefined; + + createNotebookCellExecution(notebook: URI, cellHandle: number): void; + updateNotebookCellExecution(notebook: URI, cellHandle: number, updates: ICellExecuteUpdate[]): void; + completeNotebookCellExecution(notebook: URI, cellHandle: number, complete: ICellExecutionComplete): void; +} diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookEditorKernelManager.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts similarity index 85% rename from src/vs/workbench/contrib/notebook/test/browser/notebookEditorKernelManager.test.ts rename to src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts index 185848f5a3c..adc4ade9386 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookEditorKernelManager.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts @@ -5,25 +5,25 @@ import * as assert from 'assert'; import * as sinon from 'sinon'; +import { Event } from 'vs/base/common/event'; +import { DisposableStore } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; +import { mock } from 'vs/base/test/common/mock'; import { assertThrowsAsync } from 'vs/base/test/common/utils'; +import { PLAINTEXT_LANGUAGE_ID } from 'vs/editor/common/modes/modesRegistry'; import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; -import { NotebookEditorKernelManager } from 'vs/workbench/contrib/notebook/browser/notebookEditorKernelManager'; +import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; +import { insertCellAtIndex } from 'vs/workbench/contrib/notebook/browser/controller/cellOperations'; +import { NotebookExecutionService } from 'vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl'; +import { NotebookKernelService } from 'vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl'; import { NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { CellKind, IOutputDto, NotebookCellMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { setupInstantiationService, withTestNotebook as _withTestNotebook } from 'vs/workbench/contrib/notebook/test/browser/testNotebookEditor'; -import { Event } from 'vs/base/common/event'; -import { ISelectedNotebooksChangeEvent, INotebookKernelService, INotebookKernel } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; -import { NotebookKernelService } from 'vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl'; +import { INotebookKernel, INotebookKernelService, ISelectedNotebooksChangeEvent } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; -import { mock } from 'vs/base/test/common/mock'; -import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; -import { DisposableStore } from 'vs/base/common/lifecycle'; -import { PLAINTEXT_LANGUAGE_ID } from 'vs/editor/common/modes/modesRegistry'; -import { insertCellAtIndex } from 'vs/workbench/contrib/notebook/browser/controller/cellOperations'; +import { setupInstantiationService, withTestNotebook as _withTestNotebook } from 'vs/workbench/contrib/notebook/test/browser/testNotebookEditor'; -suite('NotebookEditorKernelManager', () => { +suite('NotebookExecutionService', () => { let instantiationService: TestInstantiationService; let kernelService: INotebookKernelService; @@ -65,10 +65,10 @@ suite('NotebookEditorKernelManager', () => { await withTestNotebook( [], async (viewModel) => { - const kernelManager = instantiationService.createInstance(NotebookEditorKernelManager); + const executionService = instantiationService.createInstance(NotebookExecutionService); const cell = insertCellAtIndex(viewModel, 1, 'var c = 3', 'javascript', CellKind.Code, {}, [], true); - await assertThrowsAsync(async () => await kernelManager.executeNotebookCell(cell)); + await assertThrowsAsync(async () => await executionService.executeNotebookCell(cell)); }); }); @@ -78,9 +78,9 @@ suite('NotebookEditorKernelManager', () => { async (viewModel) => { kernelService.registerKernel(new TestNotebookKernel({ languages: ['testlang'] })); - const kernelManager = instantiationService.createInstance(NotebookEditorKernelManager); + const executionService = instantiationService.createInstance(NotebookExecutionService); const cell = insertCellAtIndex(viewModel, 1, 'var c = 3', 'javascript', CellKind.Code, {}, [], true); - await assertThrowsAsync(async () => await kernelManager.executeNotebookCell(cell)); + await assertThrowsAsync(async () => await executionService.executeNotebookCell(cell)); }); }); @@ -91,12 +91,12 @@ suite('NotebookEditorKernelManager', () => { async (viewModel) => { const kernel = new TestNotebookKernel({ languages: ['javascript'] }); kernelService.registerKernel(kernel); - const kernelManager = instantiationService.createInstance(NotebookEditorKernelManager); + const executionService = instantiationService.createInstance(NotebookExecutionService); const executeSpy = sinon.spy(); kernel.executeNotebookCellsRequest = executeSpy; const cell = insertCellAtIndex(viewModel, 0, 'var c = 3', 'javascript', CellKind.Code, {}, [], true); - await kernelManager.executeNotebookCells(viewModel.notebookDocument, [cell]); + await executionService.executeNotebookCells(viewModel.notebookDocument, [cell]); assert.strictEqual(executeSpy.calledOnce, true); }); }); @@ -121,13 +121,13 @@ suite('NotebookEditorKernelManager', () => { }; kernelService.registerKernel(kernel); - const kernelManager = instantiationService.createInstance(NotebookEditorKernelManager); + const executionService = instantiationService.createInstance(NotebookExecutionService); let event: ISelectedNotebooksChangeEvent | undefined; kernelService.onDidChangeSelectedNotebooks(e => event = e); const cell = insertCellAtIndex(viewModel, 0, 'var c = 3', 'javascript', CellKind.Code, {}, [], true, true); - await kernelManager.executeNotebookCells(viewModel.notebookDocument, [cell]); + await executionService.executeNotebookCells(viewModel.notebookDocument, [cell]); assert.strictEqual(didExecute, true); assert.ok(event !== undefined); diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts new file mode 100644 index 00000000000..de512bd4011 --- /dev/null +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts @@ -0,0 +1,116 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import * as assert from 'assert'; +import { timeout } from 'vs/base/common/async'; +import { Event } from 'vs/base/common/event'; +import { DisposableStore } from 'vs/base/common/lifecycle'; +import { URI } from 'vs/base/common/uri'; +import { mock } from 'vs/base/test/common/mock'; +import { PLAINTEXT_MODE_ID } from 'vs/editor/common/modes/modesRegistry'; +import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; +import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; +import { insertCellAtIndex } from 'vs/workbench/contrib/notebook/browser/controller/cellOperations'; +import { NotebookExecutionService } from 'vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl'; +import { NotebookExecutionStateService } from 'vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl'; +import { NotebookKernelService } from 'vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl'; +import { NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; +import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; +import { CellEditType, CellKind, IOutputDto, NotebookCellMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; +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'; + +suite('NotebookExecutionStateService', () => { + + let instantiationService: TestInstantiationService; + let kernelService: INotebookKernelService; + let disposables: DisposableStore; + + setup(function () { + + disposables = new DisposableStore(); + + instantiationService = setupInstantiationService(disposables); + + instantiationService.stub(INotebookService, new class extends mock() { + override onDidAddNotebookDocument = Event.None; + override onWillRemoveNotebookDocument = Event.None; + override getNotebookTextModels() { return []; } + }); + + kernelService = instantiationService.createInstance(NotebookKernelService); + instantiationService.set(INotebookKernelService, kernelService); + instantiationService.set(INotebookExecutionService, instantiationService.createInstance(NotebookExecutionService)); + }); + + teardown(() => { + disposables.dispose(); + }); + + async function withTestNotebook(cells: [string, string, CellKind, IOutputDto[], NotebookCellMetadata][], callback: (viewModel: NotebookViewModel, textModel: NotebookTextModel) => void | Promise) { + return _withTestNotebook(cells, (editor, viewModel) => callback(viewModel, viewModel.notebookDocument)); + } + + test('cancel execution when cell is deleted', async function () { + return withTestNotebook([], async viewModel => { + instantiationService.stub(INotebookService, new class extends mock() { + override getNotebookTextModel(uri: URI): NotebookTextModel | undefined { + return viewModel.notebookDocument; + } + }); + + let didCancel = false; + const kernel = new class extends TestNotebookKernel { + constructor() { + super({ languages: ['javascript'] }); + } + + override async executeNotebookCellsRequest(): Promise { } + + override async cancelNotebookCellExecution(): Promise { + didCancel = true; + } + }; + kernelService.registerKernel(kernel); + + const executionStateService: NotebookExecutionStateService = instantiationService.createInstance(NotebookExecutionStateService); + + const cell = insertCellAtIndex(viewModel, 0, 'var c = 3', 'javascript', CellKind.Code, {}, [], true, true); + executionStateService.createNotebookCellExecution(viewModel.uri, cell.handle); + assert.strictEqual(didCancel, false); + viewModel.notebookDocument.applyEdits([{ + editType: CellEditType.Replace, index: 0, count: 1, cells: [] + }], true, undefined, () => undefined, undefined, false); + await timeout(0); + assert.strictEqual(didCancel, true); + }); + }); +}); + +class TestNotebookKernel implements INotebookKernel { + id: string = 'test'; + label: string = ''; + viewType = '*'; + onDidChange = Event.None; + extension: ExtensionIdentifier = new ExtensionIdentifier('test'); + localResourceRoot: URI = URI.file('/test'); + description?: string | undefined; + detail?: string | undefined; + preloadUris: URI[] = []; + preloadProvides: string[] = []; + supportedLanguages: string[] = []; + executeNotebookCellsRequest(): Promise { + throw new Error('Method not implemented.'); + } + cancelNotebookCellExecution(): Promise { + throw new Error('Method not implemented.'); + } + + constructor(opts?: { languages: string[] }) { + this.supportedLanguages = opts?.languages ?? [PLAINTEXT_MODE_ID]; + } +} From 73bd5f437a9734300521cfa199dd47337ead151d Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Fri, 10 Dec 2021 15:46:36 -0800 Subject: [PATCH 3/7] Fix layer-breaker in notebook execution service --- .../contrib/notebook/browser/notebookEditorWidget.ts | 2 +- .../notebook/browser/notebookExecutionServiceImpl.ts | 6 +++--- .../contrib/notebook/common/notebookExecutionService.ts | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index ff5bf9a1154..2e23e548aa9 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -2071,7 +2071,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD if (!cells) { cells = this.viewModel.viewCells; } - return this.notebookExecutionService.executeNotebookCells(this.textModel, cells); + return this.notebookExecutionService.executeNotebookCells(this.textModel, Array.from(cells).map(c => c.model)); } //#endregion diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts index ab7580d55b8..3ef8b615df1 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts @@ -8,7 +8,7 @@ import { ICommandService } from 'vs/platform/commands/common/commands'; import { ILogService } from 'vs/platform/log/common/log'; import { IWorkspaceTrustRequestService } from 'vs/platform/workspace/common/workspaceTrust'; import { SELECT_KERNEL_ID } from 'vs/workbench/contrib/notebook/browser/controller/coreActions'; -import { ICellViewModel } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import { CellKind, INotebookTextModel, NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; import { INotebookKernel, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; @@ -31,7 +31,7 @@ export class NotebookExecutionService implements INotebookExecutionService { return info.selected ?? (info.all.length === 1 ? info.all[0] : undefined); } - async executeNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise { + async executeNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise { const cellsArr = Array.from(cells); this._logService.debug(`NotebookExecutionService#executeNotebookCells ${JSON.stringify(cellsArr.map(c => c.handle))}`); const message = nls.localize('notebookRunTrust', "Executing a notebook cell will run code from this workspace."); @@ -76,7 +76,7 @@ export class NotebookExecutionService implements INotebookExecutionService { } } - async cancelNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise { + async cancelNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise { this.cancelNotebookCellHandles(notebook, Array.from(cells, cell => cell.handle)); } } diff --git a/src/vs/workbench/contrib/notebook/common/notebookExecutionService.ts b/src/vs/workbench/contrib/notebook/common/notebookExecutionService.ts index 0013cfbfbfa..329f984d402 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookExecutionService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookExecutionService.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; -import { ICellViewModel } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import { INotebookTextModel, IOutputDto, IOutputItemDto } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookKernel } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; @@ -34,7 +34,7 @@ export interface INotebookExecutionService { _serviceBrand: undefined; getSelectedOrSuggestedKernel(notebook: INotebookTextModel): INotebookKernel | undefined; - executeNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise; - cancelNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise; + executeNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise; + cancelNotebookCells(notebook: INotebookTextModel, cells: Iterable): Promise; cancelNotebookCellHandles(notebook: INotebookTextModel, cells: Iterable): Promise; } From 962a6c85e85d6a9bf0253d7533e57542a7640404 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Fri, 17 Dec 2021 14:57:41 -0800 Subject: [PATCH 4/7] Add test for new execution behavior --- .../src/singlefolder-tests/notebook.test.ts | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts index 87b1f0e4504..70165fb39ee 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts @@ -872,6 +872,63 @@ suite('Notebook API tests', function () { assert.strictEqual(cell.executionSummary?.timing?.endTime, 20); }); + + test.skip('execution cancelled when delete while executing', async () => { + const document = await openRandomNotebookDocument(); + const cell = document.cellAt(0); + + let executionWasCancelled = false; + const cancelledKernel = new class extends Kernel { + constructor() { + super('cancelledKernel', ''); + } + + override async _execute(cells: vscode.NotebookCell[]) { + console.log(`execute`); + const [cell] = cells; + const exe = this.controller.createNotebookCellExecution(cell); + exe.token.onCancellationRequested(() => executionWasCancelled = true); + } + }; + + const notebook = await openRandomNotebookDocument(); + await vscode.window.showNotebookDocument(notebook); + await assertKernel(cancelledKernel, notebook); + await vscode.commands.executeCommand('notebook.cell.execute'); + + // Delete executing cell + const edit = new vscode.WorkspaceEdit(); + edit.replaceNotebookCells(cell!.notebook.uri, new vscode.NotebookRange(cell!.index, cell!.index + 1), []); + await vscode.workspace.applyEdit(edit); + + assert.strictEqual(executionWasCancelled, true); + }); + + test('execution cancelled when kernel changed', async () => { + await openRandomNotebookDocument(); + let executionWasCancelled = false; + const cancelledKernel = new class extends Kernel { + constructor() { + super('cancelledKernel', ''); + } + + override async _execute(cells: vscode.NotebookCell[]) { + console.log(`execute`); + const [cell] = cells; + const exe = this.controller.createNotebookCellExecution(cell); + exe.token.onCancellationRequested(() => executionWasCancelled = true); + } + }; + + const notebook = await openRandomNotebookDocument(); + await vscode.window.showNotebookDocument(notebook); + await assertKernel(cancelledKernel, notebook); + await vscode.commands.executeCommand('notebook.cell.execute'); + + const newKernel = new Kernel('newKernel', 'kernel'); + await assertKernel(newKernel, notebook); + assert.strictEqual(executionWasCancelled, true); + }); }); suite('statusbar', () => { From 27a4b497728b692ff35bfef927904b6bdca6e1eb Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Mon, 20 Dec 2021 14:27:07 -0800 Subject: [PATCH 5/7] Implement onDidChangeCellExecution and getCellExecutionState. Adopt for status bar item controller. #125668 --- .../executionStatusBarItemController.ts | 55 ++++++++++++------- .../notebookExecutionStateServiceImpl.ts | 50 ++++++++++++++--- .../common/notebookExecutionStateService.ts | 15 +++++ 3 files changed, 94 insertions(+), 26 deletions(-) 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 4dedf9ea273..866c0f190e2 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts @@ -6,6 +6,7 @@ import { disposableTimeout, RunOnceScheduler } from 'vs/base/common/async'; import { Disposable, dispose, IDisposable } from 'vs/base/common/lifecycle'; import { localize } from 'vs/nls'; +import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { themeColorFromId } from 'vs/platform/theme/common/themeService'; import { ICellVisibilityChangeEvent, NotebookVisibleCellObserver } from 'vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/notebookVisibleCellObserver'; import { formatCellDuration, ICellViewModel, INotebookEditor, INotebookEditorContribution } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; @@ -13,6 +14,7 @@ import { registerNotebookContribution } from 'vs/workbench/contrib/notebook/brow import { cellStatusIconError, cellStatusIconSuccess } from 'vs/workbench/contrib/notebook/browser/notebookEditorWidget'; import { CellViewModel, NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; import { CellStatusbarAlignment, INotebookCellStatusBarItem, NotebookCellExecutionState, NotebookCellInternalMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { ICellExecutionState, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; export class NotebookStatusBarController extends Disposable { private readonly _visibleCells = new Map(); @@ -20,7 +22,7 @@ export class NotebookStatusBarController extends Disposable { constructor( private readonly _notebookEditor: INotebookEditor, - private readonly _itemFactory: (vm: NotebookViewModel, cell: CellViewModel) => IDisposable + private readonly _itemFactory: (vm: NotebookViewModel, cell: CellViewModel) => IDisposable, ) { super(); this._observer = this._register(new NotebookVisibleCellObserver(this._notebookEditor)); @@ -62,9 +64,11 @@ export class NotebookStatusBarController extends Disposable { export class ExecutionStateCellStatusBarContrib extends Disposable implements INotebookEditorContribution { static id: string = 'workbench.notebook.statusBar.execState'; - constructor(notebookEditor: INotebookEditor) { + constructor(notebookEditor: INotebookEditor, + @IInstantiationService instantiationService: IInstantiationService + ) { super(); - this._register(new NotebookStatusBarController(notebookEditor, (vm, cell) => new ExecutionStateCellStatusBarItem(vm, cell))); + this._register(new NotebookStatusBarController(notebookEditor, (vm, cell) => instantiationService.createInstance(ExecutionStateCellStatusBarItem, vm, cell))); } } registerNotebookContribution(ExecutionStateCellStatusBarContrib.id, ExecutionStateCellStatusBarContrib); @@ -81,16 +85,22 @@ class ExecutionStateCellStatusBarItem extends Disposable { constructor( private readonly _notebookViewModel: NotebookViewModel, - private readonly _cell: ICellViewModel + private readonly _cell: ICellViewModel, + @INotebookExecutionStateService private readonly _executionStateService: INotebookExecutionStateService ) { super(); this._update(); + this._register(this._executionStateService.onDidChangeCellExecution(e => { + if (e.notebook.toString() === this._notebookViewModel.uri.toString() && e.cellHandle === this._cell.handle) { + this._update(); + } + })); this._register(this._cell.model.onDidChangeInternalMetadata(() => this._update())); } private async _update() { - const items = this._getItemsForCell(this._cell); + const items = this._getItemsForCell(); if (Array.isArray(items)) { this._currentItemIds = this._notebookViewModel.deltaCellStatusBarItems(this._currentItemIds, [{ handle: this._cell.handle, items }]); } @@ -99,18 +109,20 @@ class ExecutionStateCellStatusBarItem extends Disposable { /** * Returns undefined if there should be no change, and an empty array if all items should be removed. */ - private _getItemsForCell(cell: ICellViewModel): INotebookCellStatusBarItem[] | undefined { - if (this._currentExecutingStateTimer && !cell.internalMetadata.isPaused) { + private _getItemsForCell(): INotebookCellStatusBarItem[] | undefined { + if (this._currentExecutingStateTimer && !this._cell.internalMetadata.isPaused) { return; } - const item = this._getItemForState(cell.internalMetadata); + const runState = this._executionStateService.getCellExecutionState(this._notebookViewModel.uri, this._cell.handle); + const item = this._getItemForState(runState, this._cell.internalMetadata); // Show the execution spinner for a minimum time - if (cell.internalMetadata.runState === NotebookCellExecutionState.Executing) { + if (runState?.state === NotebookCellExecutionState.Executing) { this._currentExecutingStateTimer = this._register(disposableTimeout(() => { + const runState = this._executionStateService.getCellExecutionState(this._notebookViewModel.uri, this._cell.handle); this._currentExecutingStateTimer = undefined; - if (cell.internalMetadata.runState !== NotebookCellExecutionState.Executing) { + if (runState?.state !== NotebookCellExecutionState.Executing) { this._update(); } }, ExecutionStateCellStatusBarItem.MIN_SPINNER_TIME)); @@ -119,9 +131,10 @@ class ExecutionStateCellStatusBarItem extends Disposable { return item ? [item] : []; } - private _getItemForState(internalMetadata: NotebookCellInternalMetadata): INotebookCellStatusBarItem | undefined { - const { runState, lastRunSuccess, isPaused } = internalMetadata; - if (!runState && lastRunSuccess) { + private _getItemForState(runState: ICellExecutionState | undefined, internalMetadata: NotebookCellInternalMetadata): INotebookCellStatusBarItem | undefined { + const state = runState?.state; + const { lastRunSuccess, isPaused } = internalMetadata; + if (!state && lastRunSuccess) { return { text: '$(notebook-state-success)', color: themeColorFromId(cellStatusIconSuccess), @@ -129,7 +142,7 @@ class ExecutionStateCellStatusBarItem extends Disposable { alignment: CellStatusbarAlignment.Left, priority: Number.MAX_SAFE_INTEGER }; - } else if (!runState && lastRunSuccess === false) { + } else if (!state && lastRunSuccess === false) { return { text: '$(notebook-state-error)', color: themeColorFromId(cellStatusIconError), @@ -137,14 +150,14 @@ class ExecutionStateCellStatusBarItem extends Disposable { alignment: CellStatusbarAlignment.Left, priority: Number.MAX_SAFE_INTEGER }; - } else if (runState === NotebookCellExecutionState.Pending) { + } else if (state === NotebookCellExecutionState.Pending) { return { text: '$(notebook-state-pending)', tooltip: localize('notebook.cell.status.pending', "Pending"), alignment: CellStatusbarAlignment.Left, priority: Number.MAX_SAFE_INTEGER }; - } else if (runState === NotebookCellExecutionState.Executing) { + } else if (state === NotebookCellExecutionState.Executing) { return { text: `$(notebook-state-executing${isPaused ? '' : '~spin'})`, tooltip: localize('notebook.cell.status.executing', "Executing"), @@ -166,9 +179,11 @@ class ExecutionStateCellStatusBarItem extends Disposable { export class TimerCellStatusBarContrib extends Disposable implements INotebookEditorContribution { static id: string = 'workbench.notebook.statusBar.execTimer'; - constructor(notebookEditor: INotebookEditor) { + constructor( + notebookEditor: INotebookEditor, + @IInstantiationService instantiationService: IInstantiationService) { super(); - this._register(new NotebookStatusBarController(notebookEditor, (vm, cell) => new TimerCellStatusBarItem(vm, cell))); + this._register(new NotebookStatusBarController(notebookEditor, (vm, cell) => instantiationService.createInstance(TimerCellStatusBarItem, vm, cell))); } } registerNotebookContribution(TimerCellStatusBarContrib.id, TimerCellStatusBarContrib); @@ -182,6 +197,7 @@ class TimerCellStatusBarItem extends Disposable { constructor( private readonly _notebookViewModel: NotebookViewModel, private readonly _cell: ICellViewModel, + @INotebookExecutionStateService private readonly _executionStateService: INotebookExecutionStateService ) { super(); @@ -192,7 +208,8 @@ class TimerCellStatusBarItem extends Disposable { private async _update() { let item: INotebookCellStatusBarItem | undefined; - const state = this._cell.internalMetadata.runState; + const runState = this._executionStateService.getCellExecutionState(this._notebookViewModel.uri, this._cell.handle); + const state = runState?.state; if (this._cell.internalMetadata.isPaused) { item = undefined; } else if (state === NotebookCellExecutionState.Executing) { diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts index 327e42e8307..d260b413797 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { Emitter } from 'vs/base/common/event'; import { Disposable } from 'vs/base/common/lifecycle'; import { ResourceMap } from 'vs/base/common/map'; import { URI } from 'vs/base/common/uri'; @@ -11,7 +12,7 @@ import { ILogService } from 'vs/platform/log/common/log'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { CellEditType, ICellEditOperation, NotebookCellExecutionState, NotebookCellInternalMetadata, NotebookTextModelWillAddRemoveEvent } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { CellExecutionUpdateType, INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; -import { ICellExecuteUpdate, ICellExecutionComplete, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionState, INotebookExecutionEvent, INotebookExecutionStateService } 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'; @@ -20,6 +21,9 @@ export class NotebookExecutionStateService extends Disposable implements INotebo private readonly _executions = new ResourceMap(); + private readonly _onDidChangeCellExecution = new Emitter(); + onDidChangeCellExecution = this._onDidChangeCellExecution.event; + constructor( @INotebookKernelService private readonly _notebookKernelService: INotebookKernelService, @IInstantiationService private readonly _instantiationService: IInstantiationService, @@ -38,6 +42,18 @@ export class NotebookExecutionStateService extends Disposable implements INotebo })); } + getCellExecutionState(notebook: URI, handle: number): ICellExecutionState | undefined { + const exe = this._executions.get(notebook); + if (exe) { + const cellExe = exe.getCellExecution(handle); + return cellExe?.state && { + state: cellExe.state + }; + } + + return undefined; + } + createNotebookCellExecution(notebook: URI, cellHandle: number): void { let notebookExecution = this._executions.get(notebook); if (!notebookExecution) { @@ -46,6 +62,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo } notebookExecution.addExecution(cellHandle); + this._onDidChangeCellExecution.fire({ notebook, cellHandle }); } updateNotebookCellExecution(notebook: URI, cellHandle: number, updates: ICellExecuteUpdate[]): void { @@ -56,6 +73,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo } notebookExecution.updateExecution(cellHandle, updates); + this._onDidChangeCellExecution.fire({ notebook, cellHandle }); } completeNotebookCellExecution(notebook: URI, cellHandle: number, complete: ICellExecutionComplete): void { @@ -67,6 +85,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo notebookExecution.completeExecution(cellHandle, complete); this.checkNotebookExecutionEmpty(notebook); + this._onDidChangeCellExecution.fire({ notebook, cellHandle }); } private checkNotebookExecutionEmpty(notebook: URI): void { @@ -114,7 +133,11 @@ class NotebookExecution extends Disposable { this._register(this._notebookModel.onWillDispose(() => this.onWillDisposeDocument())); } - private cellLog(cellHandle: number): string { + getCellExecution(cellHandle: number): CellExecution | undefined { + return this._cellExecutions.get(cellHandle); + } + + private getCellLog(cellHandle: number): string { return `${this._notebookModel.uri.toString()}, ${cellHandle}`; } @@ -128,7 +151,7 @@ class NotebookExecution extends Disposable { } addExecution(cellHandle: number) { - this._logService.debug(`NotebookExecution#addExecution ${this.cellLog(cellHandle)}`); + this._logService.debug(`NotebookExecution#addExecution ${this.getCellLog(cellHandle)}`); const execution = this._instantiationService.createInstance(CellExecution, cellHandle, this._notebookModel); this._cellExecutions.set(cellHandle, execution); } @@ -146,11 +169,11 @@ class NotebookExecution extends Disposable { private logUpdates(cellHandle: number, updates: ICellExecuteUpdate[]): void { const updateTypes = updates.map(u => CellExecutionUpdateType[u.editType]).join(', '); - this._logService.debug(`NotebookExecution#updateExecution ${this.cellLog(cellHandle)}, [${updateTypes}]`); + this._logService.debug(`NotebookExecution#updateExecution ${this.getCellLog(cellHandle)}, [${updateTypes}]`); } completeExecution(cellHandle: number, complete: ICellExecutionComplete): void { - this._logService.debug(`NotebookExecution#completeExecution ${this.cellLog(cellHandle)}`); + this._logService.debug(`NotebookExecution#completeExecution ${this.getCellLog(cellHandle)}`); const execution = this._cellExecutions.get(cellHandle); if (!execution) { @@ -229,15 +252,21 @@ function updateToEdit(update: ICellExecuteUpdate, cellHandle: number): ICellEdit } class CellExecution { + private _state?: NotebookCellExecutionState = NotebookCellExecutionState.Pending; + get state() { + return this._state; + } + constructor( private readonly _cellHandle: number, private readonly _notebookModel: NotebookTextModel, + @ILogService private readonly _logService: ILogService, ) { const startExecuteEdit: ICellEditOperation = { editType: CellEditType.PartialInternalMetadata, handle: this._cellHandle, internalMetadata: { - runState: NotebookCellExecutionState.Pending, + lastRunSuccess: null, executionOrder: null, didPause: false } @@ -246,14 +275,21 @@ class CellExecution { } update(updates: ICellExecuteUpdate[]): void { + if (updates.some(u => u.editType === CellExecutionUpdateType.ExecutionState)) { + this._state = NotebookCellExecutionState.Executing; + } + const edits = updates.map(update => updateToEdit(update, this._cellHandle)); this._applyExecutionEdits(edits); } complete(completionData: ICellExecutionComplete): void { + this._state = undefined; + const cellModel = this._notebookModel.cells.find(c => c.handle === this._cellHandle); if (!cellModel) { - throw new Error('Cell not found: ' + this._cellHandle); + this._logService.debug(`CellExecution#complete, updating cell not in notebook: ${this._notebookModel.uri.toString()}, ${this._cellHandle}`); + return; } const edit: ICellEditOperation = { diff --git a/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts b/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts index b39ff197eae..5d8599d067b 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts @@ -3,8 +3,10 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { Event } from 'vs/base/common/event'; 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 { CellExecutionUpdateType, ICellExecuteOutputEdit, ICellExecuteOutputItemEdit } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; export type ICellExecuteUpdate = ICellExecuteOutputEdit | ICellExecuteOutputItemEdit | ICellExecutionStateUpdate; @@ -20,11 +22,24 @@ export interface ICellExecutionComplete { lastRunSuccess?: boolean; } +export interface ICellExecutionState { + state: NotebookCellExecutionState; +} + +export interface INotebookExecutionEvent { + notebook: URI; + cellHandle: number; +} + export const INotebookExecutionStateService = createDecorator('INotebookExecutionStateService'); export interface INotebookExecutionStateService { _serviceBrand: undefined; + onDidChangeCellExecution: Event; + + getCellExecutionState(notebook: URI, handle: number): ICellExecutionState | undefined; + createNotebookCellExecution(notebook: URI, cellHandle: number): void; updateNotebookCellExecution(notebook: URI, cellHandle: number, updates: ICellExecuteUpdate[]): void; completeNotebookCellExecution(notebook: URI, cellHandle: number, complete: ICellExecutionComplete): void; From cb918387795a5f68bad4e7b8ea68633dd28d17ab Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Wed, 29 Dec 2021 15:32:25 -0800 Subject: [PATCH 6/7] Dispose kernels in test --- .../vscode-api-tests/src/singlefolder-tests/notebook.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts index 70165fb39ee..b5e72d11cc5 100644 --- a/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts +++ b/extensions/vscode-api-tests/src/singlefolder-tests/notebook.test.ts @@ -884,12 +884,12 @@ suite('Notebook API tests', function () { } override async _execute(cells: vscode.NotebookCell[]) { - console.log(`execute`); const [cell] = cells; const exe = this.controller.createNotebookCellExecution(cell); exe.token.onCancellationRequested(() => executionWasCancelled = true); } }; + testDisposables.push(cancelledKernel.controller); const notebook = await openRandomNotebookDocument(); await vscode.window.showNotebookDocument(notebook); @@ -913,7 +913,6 @@ suite('Notebook API tests', function () { } override async _execute(cells: vscode.NotebookCell[]) { - console.log(`execute`); const [cell] = cells; const exe = this.controller.createNotebookCellExecution(cell); exe.token.onCancellationRequested(() => executionWasCancelled = true); @@ -922,10 +921,12 @@ suite('Notebook API tests', function () { const notebook = await openRandomNotebookDocument(); await vscode.window.showNotebookDocument(notebook); + testDisposables.push(cancelledKernel.controller); await assertKernel(cancelledKernel, notebook); await vscode.commands.executeCommand('notebook.cell.execute'); const newKernel = new Kernel('newKernel', 'kernel'); + testDisposables.push(newKernel.controller); await assertKernel(newKernel, notebook); assert.strictEqual(executionWasCancelled, true); }); From f944a56538a9e6144db5996121fc7ec6ccdcbddc Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Wed, 29 Dec 2021 15:46:36 -0800 Subject: [PATCH 7/7] Remove runState from cell internalMetadata. Expose execution state data from service. For #125668 --- .../api/browser/mainThreadNotebookKernels.ts | 4 + .../workbench/api/common/extHost.api.impl.ts | 2 +- .../workbench/api/common/extHost.protocol.ts | 1 + .../workbench/api/common/extHostNotebook.ts | 5 - .../api/common/extHostNotebookDocument.ts | 9 -- .../api/common/extHostNotebookKernels.ts | 17 ++- .../interactive/browser/interactiveEditor.ts | 16 ++- .../executionStatusBarItemController.ts | 12 +-- .../browser/controller/editActions.ts | 11 +- .../browser/controller/executeActions.ts | 5 +- .../browser/diff/notebookTextDiffEditor.ts | 4 +- .../notebook/browser/notebookBrowser.ts | 1 - .../notebook/browser/notebookEditorWidget.ts | 4 +- .../browser/notebookExecutionServiceImpl.ts | 7 +- .../notebookExecutionStateServiceImpl.ts | 100 ++++++++++++------ .../browser/view/cellParts/cellContextKeys.ts | 50 +++++---- .../view/cellParts/cellEditorOptions.ts | 20 ++-- .../browser/view/cellParts/cellPart.ts | 6 ++ .../browser/view/cellParts/cellProgressBar.ts | 26 +++-- .../browser/view/cellParts/cellStatusPart.ts | 2 +- .../browser/view/cellParts/codeCell.ts | 32 ++++-- .../browser/view/cellParts/markdownCell.ts | 6 +- .../browser/view/renderers/cellRenderer.ts | 4 +- .../browser/viewModel/baseCellViewModel.ts | 8 +- .../browser/viewModel/codeCellViewModel.ts | 6 +- .../notebookEditorWidgetContextKeys.ts | 42 +++----- .../common/model/notebookCellTextModel.ts | 3 +- .../contrib/notebook/common/notebookCommon.ts | 2 - .../common/notebookExecutionStateService.ts | 15 ++- .../notebook/common/notebookOptions.ts | 21 ++-- .../test/browser/notebookCellList.test.ts | 3 +- .../notebookExecutionStateService.test.ts | 4 +- .../test/browser/notebookViewModel.test.ts | 3 +- .../test/browser/testNotebookEditor.ts | 37 +++++-- 34 files changed, 303 insertions(+), 185 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts index c53fec123a0..adc78373f97 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts @@ -131,6 +131,10 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape } }); })); + + this._disposables.add(this._notebookExecutionStateService.onDidChangeCellExecution(e => { + this._proxy.$cellExecutionChanged(e.notebook, e.cellHandle, e.changed?.state); + })); } dispose(): void { diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index 650ef41d110..514b1b5bf92 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -1124,7 +1124,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I }, onDidChangeNotebookCellExecutionState(listener, thisArgs?, disposables?) { checkProposedApiEnabled(extension, 'notebookCellExecutionState'); - return extHostNotebook.onDidChangeNotebookCellExecutionState(listener, thisArgs, disposables); + return extHostNotebookKernels.onDidChangeNotebookCellExecutionState(listener, thisArgs, disposables); }, onDidChangeCellOutputs(listener, thisArgs?, disposables?) { checkProposedApiEnabled(extension, 'notebookEditor'); diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index a06e93b8668..dbed8c942bd 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -2092,6 +2092,7 @@ export interface ExtHostNotebookKernelsShape { $executeCells(handle: number, uri: UriComponents, handles: number[]): Promise; $cancelCells(handle: number, uri: UriComponents, handles: number[]): Promise; $acceptKernelMessageFromRenderer(handle: number, editorId: string, message: any): void; + $cellExecutionChanged(uri: UriComponents, cellHandle: number, state: notebookCommon.NotebookCellExecutionState | undefined): void; } export interface ExtHostInteractiveShape { diff --git a/src/vs/workbench/api/common/extHostNotebook.ts b/src/vs/workbench/api/common/extHostNotebook.ts index 9c699d13746..62a9af86b00 100644 --- a/src/vs/workbench/api/common/extHostNotebook.ts +++ b/src/vs/workbench/api/common/extHostNotebook.ts @@ -56,8 +56,6 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { readonly onDidChangeCellMetadata = this._onDidChangeCellMetadata.event; private readonly _onDidChangeActiveNotebookEditor = new Emitter(); readonly onDidChangeActiveNotebookEditor = this._onDidChangeActiveNotebookEditor.event; - private readonly _onDidChangeCellExecutionState = new Emitter(); - readonly onDidChangeNotebookCellExecutionState = this._onDidChangeCellExecutionState.event; private _activeNotebookEditor: ExtHostNotebookEditor | undefined; get activeNotebookEditor(): vscode.NotebookEditor | undefined { @@ -459,9 +457,6 @@ export class ExtHostNotebookController implements ExtHostNotebookShape { }, emitCellMetadataChange(event: vscode.NotebookCellMetadataChangeEvent): void { that._onDidChangeCellMetadata.fire(event); - }, - emitCellExecutionStateChange(event: vscode.NotebookCellExecutionStateChangeEvent): void { - that._onDidChangeCellExecutionState.fire(event); } }, uri, diff --git a/src/vs/workbench/api/common/extHostNotebookDocument.ts b/src/vs/workbench/api/common/extHostNotebookDocument.ts index 2883c9eb904..055a0b7424b 100644 --- a/src/vs/workbench/api/common/extHostNotebookDocument.ts +++ b/src/vs/workbench/api/common/extHostNotebookDocument.ts @@ -10,7 +10,6 @@ import * as extHostProtocol from 'vs/workbench/api/common/extHost.protocol'; import { ExtHostDocuments } from 'vs/workbench/api/common/extHostDocuments'; import { ExtHostDocumentsAndEditors, IExtHostModelAddedData } from 'vs/workbench/api/common/extHostDocumentsAndEditors'; import * as extHostTypeConverters from 'vs/workbench/api/common/extHostTypeConverters'; -import * as extHostTypes from 'vs/workbench/api/common/extHostTypes'; import * as notebookCommon from 'vs/workbench/contrib/notebook/common/notebookCommon'; import * as vscode from 'vscode'; @@ -129,7 +128,6 @@ export interface INotebookEventEmitter { emitModelChange(events: vscode.NotebookCellsChangeEvent): void; emitCellOutputsChange(event: vscode.NotebookCellOutputsChangeEvent): void; emitCellMetadataChange(event: vscode.NotebookCellMetadataChangeEvent): void; - emitCellExecutionStateChange(event: vscode.NotebookCellExecutionStateChangeEvent): void; } @@ -373,14 +371,7 @@ export class ExtHostNotebookDocument { private _changeCellInternalMetadata(index: number, newInternalMetadata: notebookCommon.NotebookCellInternalMetadata): void { const cell = this._cells[index]; - - const originalInternalMetadata = cell.internalMetadata; cell.setInternalMetadata(newInternalMetadata); - - if (originalInternalMetadata.runState !== newInternalMetadata.runState) { - const executionState = newInternalMetadata.runState ?? extHostTypes.NotebookCellExecutionState.Idle; - this._emitter.emitCellExecutionStateChange(deepFreeze({ document: this.apiNotebook, cell: cell.apiCell, state: executionState })); - } } getCellFromApiCell(apiCell: vscode.NotebookCell): ExtHostCell | undefined { diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index b519a071f6c..923f5cc532d 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -18,8 +18,9 @@ import { IExtHostInitDataService } from 'vs/workbench/api/common/extHostInitData import { ExtHostNotebookController } from 'vs/workbench/api/common/extHostNotebook'; import { ExtHostCell, ExtHostNotebookDocument } from 'vs/workbench/api/common/extHostNotebookDocument'; import * as extHostTypeConverters from 'vs/workbench/api/common/extHostTypeConverters'; -import { NotebookCellOutput } from 'vs/workbench/api/common/extHostTypes'; +import { NotebookCellOutput, NotebookCellExecutionState as ExtHostNotebookCellExecutionState } from 'vs/workbench/api/common/extHostTypes'; import { asWebviewUri } from 'vs/workbench/api/common/shared/webview'; +import { NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { CellExecutionUpdateType } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; import { checkProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions'; import { SerializableObjectWithBuffers } from 'vs/workbench/services/extensions/common/proxyIdentifier'; @@ -46,6 +47,9 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { private readonly _kernelData = new Map(); private _handlePool: number = 0; + private readonly _onDidChangeCellExecutionState = new Emitter(); + readonly onDidChangeNotebookCellExecutionState = this._onDidChangeCellExecutionState.event; + constructor( mainContext: IMainContext, private readonly _initData: IExtHostInitDataService, @@ -331,6 +335,17 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { obj.onDidReceiveMessage.fire(Object.freeze({ editor: editor.apiEditor, message })); } + $cellExecutionChanged(uri: UriComponents, cellHandle: number, state: NotebookCellExecutionState | undefined): void { + const document = this._extHostNotebook.getNotebookDocument(URI.revive(uri)); + const cell = document.getCell(cellHandle); + if (cell) { + this._onDidChangeCellExecutionState.fire({ + cell: cell.apiCell, + state: state ?? ExtHostNotebookCellExecutionState.Idle + }); + } + } + // --- _createNotebookCellExecution(cell: vscode.NotebookCell): vscode.NotebookCellExecution { diff --git a/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts b/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts index 445aa60f2da..aebad140018 100644 --- a/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts +++ b/src/vs/workbench/contrib/interactive/browser/interactiveEditor.ts @@ -57,6 +57,7 @@ import { INotebookEditorViewState } from 'vs/workbench/contrib/notebook/browser/ import { EditorInput } from 'vs/workbench/common/editor/editorInput'; import { ITextResourceConfigurationService } from 'vs/editor/common/services/textResourceConfigurationService'; import { ITextEditorOptions } from 'vs/platform/editor/common/editor'; +import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; const DECORATION_KEY = 'interactiveInputDecoration'; const INTERACTIVE_EDITOR_VIEW_STATE_PREFERENCE_KEY = 'InteractiveEditorViewState'; @@ -102,6 +103,7 @@ export class InteractiveEditor extends EditorPane { #menuService: IMenuService; #contextMenuService: IContextMenuService; #editorGroupService: IEditorGroupsService; + #notebookExecutionStateService: INotebookExecutionStateService; #widgetDisposableStore: DisposableStore = this._register(new DisposableStore()); #dimension?: DOM.Dimension; #notebookOptions: NotebookOptions; @@ -127,7 +129,8 @@ export class InteractiveEditor extends EditorPane { @IMenuService menuService: IMenuService, @IContextMenuService contextMenuService: IContextMenuService, @IEditorGroupsService editorGroupService: IEditorGroupsService, - @ITextResourceConfigurationService textResourceConfigurationService: ITextResourceConfigurationService + @ITextResourceConfigurationService textResourceConfigurationService: ITextResourceConfigurationService, + @INotebookExecutionStateService notebookExecutionStateService: INotebookExecutionStateService ) { super( InteractiveEditor.ID, @@ -145,8 +148,9 @@ export class InteractiveEditor extends EditorPane { this.#menuService = menuService; this.#contextMenuService = contextMenuService; this.#editorGroupService = editorGroupService; + this.#notebookExecutionStateService = notebookExecutionStateService; - this.#notebookOptions = new NotebookOptions(configurationService, { cellToolbarInteraction: 'hover', globalToolbar: true, defaultCellCollapseConfig: { codeCell: { inputCollapsed: true } } }); + this.#notebookOptions = new NotebookOptions(configurationService, notebookExecutionStateService, { cellToolbarInteraction: 'hover', globalToolbar: true, defaultCellCollapseConfig: { codeCell: { inputCollapsed: true } } }); this.#editorMemento = this.getEditorMemento(editorGroupService, textResourceConfigurationService, INTERACTIVE_EDITOR_VIEW_STATE_PREFERENCE_KEY); codeEditorService.registerDecorationType('interactive-decoration', DECORATION_KEY, {}); @@ -566,8 +570,12 @@ export class InteractiveEditor extends EditorPane { return; } - if (this.#lastCell?.internalMetadata.runState === NotebookCellExecutionState.Executing) { - return; + if (this.#lastCell) { + const runState = this.#notebookExecutionStateService.getCellExecutionState(this.#lastCell.uri)?.state; + if (runState === NotebookCellExecutionState.Executing) { + return; + } + } // scroll to bottom 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 866c0f190e2..aaca806bafc 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/cellStatusBar/executionStatusBarItemController.ts @@ -14,7 +14,7 @@ import { registerNotebookContribution } from 'vs/workbench/contrib/notebook/brow import { cellStatusIconError, cellStatusIconSuccess } from 'vs/workbench/contrib/notebook/browser/notebookEditorWidget'; import { CellViewModel, NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; import { CellStatusbarAlignment, INotebookCellStatusBarItem, NotebookCellExecutionState, NotebookCellInternalMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { ICellExecutionState, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { ICellExecutionEntry, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; export class NotebookStatusBarController extends Disposable { private readonly _visibleCells = new Map(); @@ -92,7 +92,7 @@ class ExecutionStateCellStatusBarItem extends Disposable { this._update(); this._register(this._executionStateService.onDidChangeCellExecution(e => { - if (e.notebook.toString() === this._notebookViewModel.uri.toString() && e.cellHandle === this._cell.handle) { + if (e.affectsCell(this._cell.model)) { this._update(); } })); @@ -114,13 +114,13 @@ class ExecutionStateCellStatusBarItem extends Disposable { return; } - const runState = this._executionStateService.getCellExecutionState(this._notebookViewModel.uri, this._cell.handle); + const runState = this._executionStateService.getCellExecutionState(this._cell.uri); const item = this._getItemForState(runState, this._cell.internalMetadata); // Show the execution spinner for a minimum time if (runState?.state === NotebookCellExecutionState.Executing) { this._currentExecutingStateTimer = this._register(disposableTimeout(() => { - const runState = this._executionStateService.getCellExecutionState(this._notebookViewModel.uri, this._cell.handle); + const runState = this._executionStateService.getCellExecutionState(this._cell.uri); this._currentExecutingStateTimer = undefined; if (runState?.state !== NotebookCellExecutionState.Executing) { this._update(); @@ -131,7 +131,7 @@ class ExecutionStateCellStatusBarItem extends Disposable { return item ? [item] : []; } - private _getItemForState(runState: ICellExecutionState | undefined, internalMetadata: NotebookCellInternalMetadata): INotebookCellStatusBarItem | undefined { + private _getItemForState(runState: ICellExecutionEntry | undefined, internalMetadata: NotebookCellInternalMetadata): INotebookCellStatusBarItem | undefined { const state = runState?.state; const { lastRunSuccess, isPaused } = internalMetadata; if (!state && lastRunSuccess) { @@ -208,7 +208,7 @@ class TimerCellStatusBarItem extends Disposable { private async _update() { let item: INotebookCellStatusBarItem | undefined; - const runState = this._executionStateService.getCellExecutionState(this._notebookViewModel.uri, this._cell.handle); + const runState = this._executionStateService.getCellExecutionState(this._cell.uri); const state = runState?.state; if (this._cell.internalMetadata.isPaused) { item = undefined; diff --git a/src/vs/workbench/contrib/notebook/browser/controller/editActions.ts b/src/vs/workbench/contrib/notebook/browser/controller/editActions.ts index c1ca166bfbd..5b73f054b44 100644 --- a/src/vs/workbench/contrib/notebook/browser/controller/editActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/controller/editActions.ts @@ -25,6 +25,7 @@ import * as icons from 'vs/workbench/contrib/notebook/browser/notebookIcons'; import { CellEditType, CellKind, ICellEditOperation, NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange'; import { ILanguageDetectionService } from 'vs/workbench/services/languageDetection/common/languageDetectionWorkerService'; +import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; const CLEAR_ALL_CELLS_OUTPUTS_COMMAND_ID = 'notebook.clearAllCellsOutputs'; const EDIT_CELL_COMMAND_ID = 'notebook.cell.edit'; @@ -197,6 +198,7 @@ registerAction2(class ClearCellOutputsAction extends NotebookCellAction { } async runWithContext(accessor: ServicesAccessor, context: INotebookCellActionContext): Promise { + const notebookExecutionStateService = accessor.get(INotebookExecutionStateService); const editor = context.notebookEditor; if (!editor.hasModel() || !editor.textModel.length) { return; @@ -211,10 +213,10 @@ registerAction2(class ClearCellOutputsAction extends NotebookCellAction { editor.textModel.applyEdits([{ editType: CellEditType.Output, index, outputs: [] }], true, undefined, () => undefined, undefined); - if (context.cell.internalMetadata.runState !== NotebookCellExecutionState.Executing) { + const runState = notebookExecutionStateService.getCellExecutionState(context.cell.uri)?.state; + if (runState !== NotebookCellExecutionState.Executing) { context.notebookEditor.textModel.applyEdits([{ editType: CellEditType.PartialInternalMetadata, index, internalMetadata: { - runState: null, runStartTime: null, runStartTimeAdjustment: null, runEndTime: null, @@ -258,6 +260,7 @@ registerAction2(class ClearAllCellOutputsAction extends NotebookAction { } async runWithContext(accessor: ServicesAccessor, context: INotebookActionContext): Promise { + const notebookExecutionStateService = accessor.get(INotebookExecutionStateService); const editor = context.notebookEditor; if (!editor.hasModel() || !editor.textModel.length) { return; @@ -269,10 +272,10 @@ registerAction2(class ClearAllCellOutputsAction extends NotebookAction { })), true, undefined, () => undefined, undefined); const clearExecutionMetadataEdits = editor.textModel.cells.map((cell, index) => { - if (cell.internalMetadata.runState !== NotebookCellExecutionState.Executing) { + const runState = notebookExecutionStateService.getCellExecutionState(cell.uri)?.state; + if (runState !== NotebookCellExecutionState.Executing) { return { editType: CellEditType.PartialInternalMetadata, index, internalMetadata: { - runState: null, runStartTime: null, runStartTimeAdjustment: null, runEndTime: null, diff --git a/src/vs/workbench/contrib/notebook/browser/controller/executeActions.ts b/src/vs/workbench/contrib/notebook/browser/controller/executeActions.ts index abc5e26bdf6..9e70bd32c32 100644 --- a/src/vs/workbench/contrib/notebook/browser/controller/executeActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/controller/executeActions.ts @@ -16,7 +16,7 @@ import { insertCell } from 'vs/workbench/contrib/notebook/browser/controller/cel 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 { CellEditState, CellFocusMode, EXECUTE_CELL_COMMAND_ID, 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_MISSING_KERNEL_EXTENSION } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import * as icons from 'vs/workbench/contrib/notebook/browser/notebookIcons'; -import { CellKind, NotebookCellExecutionState, NotebookSetting } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellKind, NotebookSetting } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { NotebookEditorInput } from 'vs/workbench/contrib/notebook/common/notebookEditorInput'; import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editorGroupsService'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; @@ -65,9 +65,6 @@ async function runCell(accessor: ServicesAccessor, context: INotebookActionConte } if (context.ui && context.cell) { - if (context.cell.internalMetadata.runState === NotebookCellExecutionState.Executing) { - return; - } await context.notebookEditor.executeNotebookCells(Iterable.single(context.cell)); if (context.autoReveal) { const cellIndex = context.notebookEditor.getCellIndex(context.cell); diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookTextDiffEditor.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookTextDiffEditor.ts index eef7e7fff9a..f29db8974f4 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookTextDiffEditor.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookTextDiffEditor.ts @@ -41,6 +41,7 @@ import { BackLayerWebView, INotebookDelegateForWebview } from 'vs/workbench/cont import { NotebookDiffEditorEventDispatcher, NotebookDiffLayoutChangedEvent } from 'vs/workbench/contrib/notebook/browser/diff/eventDispatcher'; import { readFontInfo } from 'vs/editor/browser/config/configuration'; import { NotebookOptions } from 'vs/workbench/contrib/notebook/common/notebookOptions'; +import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; const $ = DOM.$; @@ -98,9 +99,10 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD @IConfigurationService private readonly configurationService: IConfigurationService, @ITelemetryService telemetryService: ITelemetryService, @IStorageService storageService: IStorageService, + @INotebookExecutionStateService notebookExecutionStateService: INotebookExecutionStateService, ) { super(NotebookTextDiffEditor.ID, telemetryService, themeService, storageService); - this._notebookOptions = new NotebookOptions(this.configurationService); + this._notebookOptions = new NotebookOptions(this.configurationService, notebookExecutionStateService); this._register(this._notebookOptions); const editorOptions = this.configurationService.getValue('editor'); this._fontInfo = readFontInfo(BareFontInfo.createFromRawSettings(editorOptions, getZoomLevel(), getPixelRatio())); diff --git a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts index f9b4f8b24ba..93096590d1d 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts @@ -723,7 +723,6 @@ export enum CursorAtBoundary { export interface CellViewModelStateChangeEvent { readonly metadataChanged?: boolean; readonly internalMetadataChanged?: boolean; - readonly runStateChanged?: boolean; readonly selectionChanged?: boolean; readonly focusModeChanged?: boolean; readonly editStateChanged?: boolean; diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index 2e23e548aa9..97a590ff63d 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -71,6 +71,7 @@ import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange'; import { INotebookRendererMessagingService } from 'vs/workbench/contrib/notebook/common/notebookRendererMessagingService'; import { editorGutterModifiedBackground } from 'vs/workbench/contrib/scm/browser/dirtydiffDecorator'; import { IWebview } from 'vs/workbench/contrib/webview/browser/webview'; +import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; const $ = DOM.$; @@ -400,12 +401,13 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD @IThemeService private readonly themeService: IThemeService, @ITelemetryService private readonly telemetryService: ITelemetryService, @INotebookExecutionService private readonly notebookExecutionService: INotebookExecutionService, + @INotebookExecutionStateService notebookExecutionStateService: INotebookExecutionStateService, ) { super(); this.isEmbedded = creationOptions.isEmbedded ?? false; this._readOnly = creationOptions.isReadOnly ?? false; - this._notebookOptions = creationOptions.options ?? new NotebookOptions(this.configurationService); + this._notebookOptions = creationOptions.options ?? new NotebookOptions(this.configurationService, notebookExecutionStateService); this._register(this._notebookOptions); this._viewContext = new ViewContext(this._notebookOptions, new NotebookEventDispatcher()); this._register(this._viewContext.eventDispatcher.onDidChangeCellState(e => { diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts index 3ef8b615df1..a62ada9faf7 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts @@ -9,8 +9,9 @@ import { ILogService } from 'vs/platform/log/common/log'; import { IWorkspaceTrustRequestService } from 'vs/platform/workspace/common/workspaceTrust'; import { SELECT_KERNEL_ID } from 'vs/workbench/contrib/notebook/browser/controller/coreActions'; import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; -import { CellKind, INotebookTextModel, NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellKind, INotebookTextModel } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; +import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { INotebookKernel, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; export class NotebookExecutionService implements INotebookExecutionService { @@ -21,6 +22,7 @@ export class NotebookExecutionService implements INotebookExecutionService { @INotebookKernelService private readonly _notebookKernelService: INotebookKernelService, @IWorkspaceTrustRequestService private readonly _workspaceTrustRequestService: IWorkspaceTrustRequestService, @ILogService private readonly _logService: ILogService, + @INotebookExecutionStateService private readonly _notebookExecutionStateService: INotebookExecutionStateService ) { } @@ -52,7 +54,8 @@ export class NotebookExecutionService implements INotebookExecutionService { const cellHandles: number[] = []; for (const cell of cellsArr) { - if (cell.cellKind !== CellKind.Code || cell.internalMetadata.runState === NotebookCellExecutionState.Pending || cell.internalMetadata.runState === NotebookCellExecutionState.Executing) { + const cellExe = this._notebookExecutionStateService.getCellExecutionState(cell.uri); + if (cell.cellKind !== CellKind.Code || !!cellExe) { continue; } if (!kernel.supportedLanguages.includes(cell.language)) { diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts index d260b413797..884f2ed5072 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookExecutionStateServiceImpl.ts @@ -6,13 +6,15 @@ import { Emitter } from 'vs/base/common/event'; import { Disposable } from 'vs/base/common/lifecycle'; import { ResourceMap } from 'vs/base/common/map'; +import { isEqual } from 'vs/base/common/resources'; import { URI } from 'vs/base/common/uri'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { ILogService } from 'vs/platform/log/common/log'; +import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; -import { CellEditType, ICellEditOperation, NotebookCellExecutionState, NotebookCellInternalMetadata, NotebookTextModelWillAddRemoveEvent } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { CellEditType, CellUri, ICellEditOperation, NotebookCellExecutionState, NotebookCellInternalMetadata, NotebookTextModelWillAddRemoveEvent } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { CellExecutionUpdateType, INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; -import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionState, INotebookExecutionEvent, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; +import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionEntry, ICellExecutionStateChangedEvent, INotebookExecutionStateService } 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'; @@ -21,7 +23,7 @@ export class NotebookExecutionStateService extends Disposable implements INotebo private readonly _executions = new ResourceMap(); - private readonly _onDidChangeCellExecution = new Emitter(); + private readonly _onDidChangeCellExecution = new Emitter(); onDidChangeCellExecution = this._onDidChangeCellExecution.event; constructor( @@ -42,18 +44,25 @@ export class NotebookExecutionStateService extends Disposable implements INotebo })); } - getCellExecutionState(notebook: URI, handle: number): ICellExecutionState | undefined { - const exe = this._executions.get(notebook); + getCellExecutionState(cellUri: URI): ICellExecutionEntry | undefined { + const parsed = CellUri.parse(cellUri); + if (!parsed) { + throw new Error(`Not a cell URI: ${cellUri}`); + } + + const exe = this._executions.get(parsed.notebook); if (exe) { - const cellExe = exe.getCellExecution(handle); - return cellExe?.state && { - state: cellExe.state - }; + return exe.getCellExecution(parsed.handle); } return undefined; } + getCellExecutionStatesForNotebook(notebook: URI): ICellExecutionEntry[] { + const exe = this._executions.get(notebook); + return (exe?.getCellExecutions() ?? []); + } + createNotebookCellExecution(notebook: URI, cellHandle: number): void { let notebookExecution = this._executions.get(notebook); if (!notebookExecution) { @@ -61,8 +70,8 @@ export class NotebookExecutionStateService extends Disposable implements INotebo this._executions.set(notebook, notebookExecution); } - notebookExecution.addExecution(cellHandle); - this._onDidChangeCellExecution.fire({ notebook, cellHandle }); + const exe = notebookExecution.addExecution(cellHandle); + this._onDidChangeCellExecution.fire(new NotebookExecutionEvent(notebook, cellHandle, exe)); } updateNotebookCellExecution(notebook: URI, cellHandle: number, updates: ICellExecuteUpdate[]): void { @@ -72,8 +81,10 @@ export class NotebookExecutionStateService extends Disposable implements INotebo return; } - notebookExecution.updateExecution(cellHandle, updates); - this._onDidChangeCellExecution.fire({ notebook, cellHandle }); + const exe = notebookExecution.updateExecution(cellHandle, updates); + if (exe) { + this._onDidChangeCellExecution.fire(new NotebookExecutionEvent(notebook, cellHandle, exe)); + } } completeNotebookCellExecution(notebook: URI, cellHandle: number, complete: ICellExecutionComplete): void { @@ -83,9 +94,11 @@ export class NotebookExecutionStateService extends Disposable implements INotebo return; } - notebookExecution.completeExecution(cellHandle, complete); - this.checkNotebookExecutionEmpty(notebook); - this._onDidChangeCellExecution.fire({ notebook, cellHandle }); + const exe = notebookExecution.completeExecution(cellHandle, complete); + if (exe) { + this.checkNotebookExecutionEmpty(notebook); + this._onDidChangeCellExecution.fire(new NotebookExecutionEvent(notebook, cellHandle)); + } } private checkNotebookExecutionEmpty(notebook: URI): void { @@ -108,6 +121,19 @@ export class NotebookExecutionStateService extends Disposable implements INotebo } } +class NotebookExecutionEvent implements ICellExecutionStateChangedEvent { + constructor( + readonly notebook: URI, + readonly cellHandle: number, + readonly changed?: CellExecution + ) { } + + affectsCell(cell: NotebookCellTextModel): boolean { + const parsedUri = CellUri.parse(cell.uri); + return !!parsedUri && isEqual(this.notebook, parsedUri.notebook) && this.cellHandle === parsedUri.handle; + } +} + class NotebookExecution extends Disposable { private readonly _notebookModel: NotebookTextModel; @@ -137,6 +163,10 @@ class NotebookExecution extends Disposable { return this._cellExecutions.get(cellHandle); } + getCellExecutions(): CellExecution[] { + return Array.from(this._cellExecutions.values()); + } + private getCellLog(cellHandle: number): string { return `${this._notebookModel.uri.toString()}, ${cellHandle}`; } @@ -150,13 +180,14 @@ class NotebookExecution extends Disposable { this._notebookExecutionService.cancelNotebookCellHandles(this._notebookModel, this._cellExecutions.keys()); } - addExecution(cellHandle: number) { + addExecution(cellHandle: number): CellExecution { this._logService.debug(`NotebookExecution#addExecution ${this.getCellLog(cellHandle)}`); const execution = this._instantiationService.createInstance(CellExecution, cellHandle, this._notebookModel); this._cellExecutions.set(cellHandle, execution); + return execution; } - updateExecution(cellHandle: number, updates: ICellExecuteUpdate[]): void { + updateExecution(cellHandle: number, updates: ICellExecuteUpdate[]): CellExecution | undefined { this.logUpdates(cellHandle, updates); const execution = this._cellExecutions.get(cellHandle); if (!execution) { @@ -165,6 +196,7 @@ class NotebookExecution extends Disposable { } execution.update(updates); + return execution; } private logUpdates(cellHandle: number, updates: ICellExecuteUpdate[]): void { @@ -172,7 +204,7 @@ class NotebookExecution extends Disposable { this._logService.debug(`NotebookExecution#updateExecution ${this.getCellLog(cellHandle)}, [${updateTypes}]`); } - completeExecution(cellHandle: number, complete: ICellExecutionComplete): void { + completeExecution(cellHandle: number, complete: ICellExecutionComplete): CellExecution | undefined { this._logService.debug(`NotebookExecution#completeExecution ${this.getCellLog(cellHandle)}`); const execution = this._cellExecutions.get(cellHandle); @@ -183,6 +215,7 @@ class NotebookExecution extends Disposable { try { execution.complete(complete); + return execution; } finally { this._cellExecutions.delete(cellHandle); } @@ -232,9 +265,7 @@ function updateToEdit(update: ICellExecuteUpdate, cellHandle: number): ICellEdit outputId: update.outputId }; } else if (update.editType === CellExecutionUpdateType.ExecutionState) { - const newInternalMetadata: Partial = { - runState: NotebookCellExecutionState.Executing, - }; + const newInternalMetadata: Partial = { }; if (typeof update.executionOrder !== 'undefined') { newInternalMetadata.executionOrder = update.executionOrder; } @@ -251,21 +282,27 @@ function updateToEdit(update: ICellExecuteUpdate, cellHandle: number): ICellEdit throw new Error('Unknown cell update type'); } -class CellExecution { - private _state?: NotebookCellExecutionState = NotebookCellExecutionState.Pending; +class CellExecution implements ICellExecutionEntry { + private _state: NotebookCellExecutionState = NotebookCellExecutionState.Pending; get state() { return this._state; } + get notebook(): URI { + return this._notebookModel.uri; + } + constructor( - private readonly _cellHandle: number, + readonly cellHandle: number, private readonly _notebookModel: NotebookTextModel, @ILogService private readonly _logService: ILogService, ) { const startExecuteEdit: ICellEditOperation = { editType: CellEditType.PartialInternalMetadata, - handle: this._cellHandle, + handle: this.cellHandle, internalMetadata: { + runStartTime: null, + runEndTime: null, lastRunSuccess: null, executionOrder: null, didPause: false @@ -279,24 +316,21 @@ class CellExecution { this._state = NotebookCellExecutionState.Executing; } - const edits = updates.map(update => updateToEdit(update, this._cellHandle)); + const edits = updates.map(update => updateToEdit(update, this.cellHandle)); this._applyExecutionEdits(edits); } complete(completionData: ICellExecutionComplete): void { - this._state = undefined; - - const cellModel = this._notebookModel.cells.find(c => c.handle === this._cellHandle); + const cellModel = this._notebookModel.cells.find(c => c.handle === this.cellHandle); if (!cellModel) { - this._logService.debug(`CellExecution#complete, updating cell not in notebook: ${this._notebookModel.uri.toString()}, ${this._cellHandle}`); + this._logService.debug(`CellExecution#complete, updating cell not in notebook: ${this._notebookModel.uri.toString()}, ${this.cellHandle}`); return; } const edit: ICellEditOperation = { editType: CellEditType.PartialInternalMetadata, - handle: this._cellHandle, + handle: this.cellHandle, internalMetadata: { - runState: null, lastRunSuccess: completionData.lastRunSuccess, runStartTime: cellModel.internalMetadata.didPause ? null : cellModel.internalMetadata.runStartTime, runEndTime: cellModel.internalMetadata.didPause ? null : completionData.runEndTime, 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 976e22e8bc4..f12c9936984 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellContextKeys.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellContextKeys.ts @@ -9,6 +9,7 @@ import { CellEditState, CellFocusMode, CellViewModelStateChangeEvent, ICellViewM import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel'; import { MarkupCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel'; import { NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; export class CellContextKeyManager extends Disposable { @@ -30,25 +31,32 @@ export class CellContextKeyManager extends Disposable { constructor( private readonly notebookEditor: INotebookEditorDelegate, private element: ICellViewModel, - @IContextKeyService private readonly contextKeyService: IContextKeyService, + @IContextKeyService private readonly _contextKeyService: IContextKeyService, + @INotebookExecutionStateService private readonly _notebookExecutionStateService: INotebookExecutionStateService ) { super(); - this.contextKeyService.bufferChangeEvents(() => { - this.cellType = NOTEBOOK_CELL_TYPE.bindTo(this.contextKeyService); - this.cellEditable = NOTEBOOK_CELL_EDITABLE.bindTo(this.contextKeyService); - this.cellFocused = NOTEBOOK_CELL_FOCUSED.bindTo(this.contextKeyService); - this.cellEditorFocused = NOTEBOOK_CELL_EDITOR_FOCUSED.bindTo(this.contextKeyService); - this.markdownEditMode = NOTEBOOK_CELL_MARKDOWN_EDIT_MODE.bindTo(this.contextKeyService); - this.cellRunState = NOTEBOOK_CELL_EXECUTION_STATE.bindTo(this.contextKeyService); - this.cellExecuting = NOTEBOOK_CELL_EXECUTING.bindTo(this.contextKeyService); - this.cellHasOutputs = NOTEBOOK_CELL_HAS_OUTPUTS.bindTo(this.contextKeyService); - this.cellContentCollapsed = NOTEBOOK_CELL_INPUT_COLLAPSED.bindTo(this.contextKeyService); - this.cellOutputCollapsed = NOTEBOOK_CELL_OUTPUT_COLLAPSED.bindTo(this.contextKeyService); - this.cellLineNumbers = NOTEBOOK_CELL_LINE_NUMBERS.bindTo(this.contextKeyService); + this._contextKeyService.bufferChangeEvents(() => { + this.cellType = NOTEBOOK_CELL_TYPE.bindTo(this._contextKeyService); + this.cellEditable = NOTEBOOK_CELL_EDITABLE.bindTo(this._contextKeyService); + this.cellFocused = NOTEBOOK_CELL_FOCUSED.bindTo(this._contextKeyService); + this.cellEditorFocused = NOTEBOOK_CELL_EDITOR_FOCUSED.bindTo(this._contextKeyService); + this.markdownEditMode = NOTEBOOK_CELL_MARKDOWN_EDIT_MODE.bindTo(this._contextKeyService); + this.cellRunState = NOTEBOOK_CELL_EXECUTION_STATE.bindTo(this._contextKeyService); + this.cellExecuting = NOTEBOOK_CELL_EXECUTING.bindTo(this._contextKeyService); + this.cellHasOutputs = NOTEBOOK_CELL_HAS_OUTPUTS.bindTo(this._contextKeyService); + this.cellContentCollapsed = NOTEBOOK_CELL_INPUT_COLLAPSED.bindTo(this._contextKeyService); + this.cellOutputCollapsed = NOTEBOOK_CELL_OUTPUT_COLLAPSED.bindTo(this._contextKeyService); + this.cellLineNumbers = NOTEBOOK_CELL_LINE_NUMBERS.bindTo(this._contextKeyService); this.updateForElement(element); }); + + this._register(this._notebookExecutionStateService.onDidChangeCellExecution(e => { + if (e.affectsCell(this.element.model)) { + this.updateForExecutionState(); + } + })); } public updateForElement(element: ICellViewModel) { @@ -68,9 +76,9 @@ export class CellContextKeyManager extends Disposable { this.cellType.set('code'); } - this.contextKeyService.bufferChangeEvents(() => { + this._contextKeyService.bufferChangeEvents(() => { this.updateForFocusState(); - this.updateForInternalMetadata(); + this.updateForExecutionState(); this.updateForEditState(); this.updateForCollapseState(); this.updateForOutputs(); @@ -80,9 +88,9 @@ export class CellContextKeyManager extends Disposable { } private onDidChangeState(e: CellViewModelStateChangeEvent) { - this.contextKeyService.bufferChangeEvents(() => { + this._contextKeyService.bufferChangeEvents(() => { if (e.internalMetadataChanged) { - this.updateForInternalMetadata(); + this.updateForExecutionState(); } if (e.editStateChanged) { @@ -115,18 +123,18 @@ export class CellContextKeyManager extends Disposable { } - private updateForInternalMetadata() { + private updateForExecutionState() { const internalMetadata = this.element.internalMetadata; this.cellEditable.set(!this.notebookEditor.isReadOnly); - const runState = internalMetadata.runState; + const exeState = this._notebookExecutionStateService.getCellExecutionState(this.element.uri); if (this.element instanceof MarkupCellViewModel) { this.cellRunState.reset(); this.cellExecuting.reset(); - } else if (runState === NotebookCellExecutionState.Executing) { + } else if (exeState?.state === NotebookCellExecutionState.Executing) { this.cellRunState.set('executing'); this.cellExecuting.set(true); - } else if (runState === NotebookCellExecutionState.Pending) { + } else if (exeState?.state === NotebookCellExecutionState.Pending) { this.cellRunState.set('pending'); this.cellExecuting.set(true); } else if (internalMetadata.lastRunSuccess === true) { diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellEditorOptions.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellEditorOptions.ts index 39aca6a4153..3c162b4484e 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellEditorOptions.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellEditorOptions.ts @@ -6,6 +6,7 @@ import { Emitter, Event } from 'vs/base/common/event'; import { DisposableStore } from 'vs/base/common/lifecycle'; import { deepClone } from 'vs/base/common/objects'; +import { URI } from 'vs/base/common/uri'; import { IEditorOptions, LineNumbersType } from 'vs/editor/common/config/editorOptions'; import { localize } from 'vs/nls'; import { Action2, MenuId, registerAction2 } from 'vs/platform/actions/common/actions'; @@ -133,20 +134,27 @@ export class CellEditorOptions extends CellPart { return computed; } - getUpdatedValue(internalMetadata?: NotebookCellInternalMetadata): IEditorOptions { - const options = this.getValue(internalMetadata); + getUpdatedValue(internalMetadata: NotebookCellInternalMetadata, cellUri: URI): IEditorOptions { + const options = this.getValue(internalMetadata, cellUri); delete options.hover; // This is toggled by a debug editor contribution return options; } - getValue(internalMetadata?: NotebookCellInternalMetadata): IEditorOptions { + getValue(internalMetadata: NotebookCellInternalMetadata, cellUri: URI): IEditorOptions { return { ...this._value, ...{ - padding: internalMetadata ? - this.notebookOptions.computeEditorPadding(internalMetadata) : - { top: 12, bottom: 12 } + padding: this.notebookOptions.computeEditorPadding(internalMetadata, cellUri) + } + }; + } + + getDefaultValue(): IEditorOptions { + return { + ...this._value, + ...{ + padding: { top: 12, bottom: 12 } } }; } diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellPart.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellPart.ts index abcae3cc22e..563e978ba66 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellPart.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellPart.ts @@ -6,6 +6,7 @@ import { Disposable } from 'vs/base/common/lifecycle'; import { CellViewModelStateChangeEvent, ICellViewModel } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { BaseCellRenderTemplate } from 'vs/workbench/contrib/notebook/browser/view/notebookRenderingCommon'; +import { ICellExecutionStateChangedEvent } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; export abstract class CellPart extends Disposable { constructor() { @@ -33,4 +34,9 @@ export abstract class CellPart extends Disposable { * Update per cell state change */ abstract updateState(element: ICellViewModel, e: CellViewModelStateChangeEvent): void; + + /** + * Update per execution state change. + */ + updateForExecutionState(element: ICellViewModel, e: ICellExecutionStateChangedEvent): void { } } diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellProgressBar.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellProgressBar.ts index b584752d27f..415d8c9d330 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellProgressBar.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellProgressBar.ts @@ -4,9 +4,10 @@ *--------------------------------------------------------------------------------------------*/ import { ProgressBar } from 'vs/base/browser/ui/progressbar/progressbar'; -import { ICellViewModel, CellViewModelStateChangeEvent } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { CellViewModelStateChangeEvent, ICellViewModel } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { CellPart } from 'vs/workbench/contrib/notebook/browser/view/cellParts/cellPart'; -import { NotebookCellExecutionState, NotebookCellInternalMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { ICellExecutionStateChangedEvent, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; export class CellProgressBar extends CellPart { private readonly _progressBar: ProgressBar; @@ -14,7 +15,8 @@ export class CellProgressBar extends CellPart { constructor( editorContainer: HTMLElement, - collapsedInputContainer: HTMLElement) { + collapsedInputContainer: HTMLElement, + @INotebookExecutionStateService private readonly _notebookExecutionStateService: INotebookExecutionStateService) { super(); this._progressBar = this._register(new ProgressBar(editorContainer)); @@ -25,7 +27,7 @@ export class CellProgressBar extends CellPart { } renderCell(element: ICellViewModel): void { - this.updateForInternalMetadata(element, element.internalMetadata); + this._updateForExecutionState(element); } prepareLayout(): void { @@ -36,29 +38,35 @@ export class CellProgressBar extends CellPart { // nothing to update } + override updateForExecutionState(element: ICellViewModel, e: ICellExecutionStateChangedEvent): void { + this._updateForExecutionState(element, e); + } + updateState(element: ICellViewModel, e: CellViewModelStateChangeEvent): void { if (e.metadataChanged || e.internalMetadataChanged) { - this.updateForInternalMetadata(element, element.internalMetadata); + this._updateForExecutionState(element); } if (e.inputCollapsedChanged) { + const exeState = this._notebookExecutionStateService.getCellExecutionState(element.uri); if (element.isInputCollapsed) { this._progressBar.hide(); - if (element.internalMetadata.runState === NotebookCellExecutionState.Executing) { + if (exeState?.state === NotebookCellExecutionState.Executing) { showProgressBar(this._collapsedProgressBar); } } else { this._collapsedProgressBar.hide(); - if (element.internalMetadata.runState === NotebookCellExecutionState.Executing) { + if (exeState?.state === NotebookCellExecutionState.Executing) { showProgressBar(this._progressBar); } } } } - updateForInternalMetadata(element: ICellViewModel, internalMetadata: NotebookCellInternalMetadata): void { + private _updateForExecutionState(element: ICellViewModel, e?: ICellExecutionStateChangedEvent): void { + const exeState = e?.changed ?? this._notebookExecutionStateService.getCellExecutionState(element.uri); const progressBar = element.isInputCollapsed ? this._collapsedProgressBar : this._progressBar; - if (internalMetadata.runState === NotebookCellExecutionState.Executing && !internalMetadata.isPaused) { + if (exeState?.state === NotebookCellExecutionState.Executing && !element.internalMetadata.isPaused) { showProgressBar(progressBar); } else { progressBar.hide(); diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellStatusPart.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellStatusPart.ts index 6532539a0b1..6583374b3ad 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellStatusPart.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellStatusPart.ts @@ -104,7 +104,7 @@ export class CellEditorStatusBar extends CellPart { updateInternalLayoutNow(element: ICellViewModel): void { // todo@rebornix layer breaker - this._cellContainer.classList.toggle('cell-statusbar-hidden', this._notebookEditor.notebookOptions.computeEditorStatusbarHeight(element.internalMetadata) === 0); + this._cellContainer.classList.toggle('cell-statusbar-hidden', this._notebookEditor.notebookOptions.computeEditorStatusbarHeight(element.internalMetadata, element.uri) === 0); const layoutInfo = element.layoutInfo; const width = layoutInfo.editorWidth; diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCell.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCell.ts index d0a8e2ede0e..c28981d9190 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCell.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/codeCell.ts @@ -5,9 +5,9 @@ import * as DOM from 'vs/base/browser/dom'; import { raceCancellation } from 'vs/base/common/async'; -import { Event } from 'vs/base/common/event'; import { CancellationTokenSource } from 'vs/base/common/cancellation'; import { Codicon, CSSIcon } from 'vs/base/common/codicons'; +import { Event } from 'vs/base/common/event'; import { Disposable, IDisposable } from 'vs/base/common/lifecycle'; import { EditorOption } from 'vs/editor/common/config/editorOptions'; import { IDimension } from 'vs/editor/common/editorCommon'; @@ -15,18 +15,19 @@ import { IReadonlyTextBuffer } from 'vs/editor/common/model'; import { tokenizeToStringSync } from 'vs/editor/common/modes/textToHtmlTokenizer'; import { ILanguageService } from 'vs/editor/common/services/languageService'; import { localize } from 'vs/nls'; +import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { IOpenerService } from 'vs/platform/opener/common/opener'; import { CellFocusMode, EXPAND_CELL_INPUT_COMMAND_ID, IActiveNotebookEditorDelegate } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { CodeCellRenderTemplate } from 'vs/workbench/contrib/notebook/browser/view/notebookRenderingCommon'; +import { CellEditorOptions } from 'vs/workbench/contrib/notebook/browser/view/cellParts/cellEditorOptions'; import { CellOutputContainer } from 'vs/workbench/contrib/notebook/browser/view/cellParts/cellOutput'; +import { CellPart } from 'vs/workbench/contrib/notebook/browser/view/cellParts/cellPart'; import { ClickTargetType } from 'vs/workbench/contrib/notebook/browser/view/cellParts/cellWidgets'; +import { CodeCellRenderTemplate } from 'vs/workbench/contrib/notebook/browser/view/notebookRenderingCommon'; import { CodeCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel'; import { INotebookCellStatusBarService } from 'vs/workbench/contrib/notebook/common/notebookCellStatusBarService'; -import { CellPart } from 'vs/workbench/contrib/notebook/browser/view/cellParts/cellPart'; -import { CellEditorOptions } from 'vs/workbench/contrib/notebook/browser/view/cellParts/cellEditorOptions'; -import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; export class CodeCell extends Disposable { @@ -49,6 +50,7 @@ export class CodeCell extends Disposable { @IOpenerService readonly openerService: IOpenerService, @ILanguageService readonly languageService: ILanguageService, @IConfigurationService private configurationService: IConfigurationService, + @INotebookExecutionStateService notebookExecutionStateService: INotebookExecutionStateService ) { super(); @@ -64,6 +66,14 @@ export class CodeCell extends Disposable { this.registerDecorations(); this.registerMouseListener(); + this._register(notebookExecutionStateService.onDidChangeCellExecution(e => { + if (e.affectsCell(this.viewCell.model)) { + this.cellParts.forEach(cellPart => { + cellPart.updateForExecutionState(this.viewCell, e); + }); + } + })); + this._register(this.viewCell.onDidChangeState(e => { this.cellParts.forEach(cellPart => { cellPart.updateState(this.viewCell, e); @@ -121,8 +131,8 @@ export class CodeCell extends Disposable { this._register(Event.runAndSubscribe(viewCell.onDidChangeOutputs, this.updateForOutputs.bind(this))); this._register(Event.runAndSubscribe(viewCell.onDidChangeLayout, this.updateForLayout.bind(this))); - this._register(cellEditorOptions.onDidChange(() => templateData.editor.updateOptions(cellEditorOptions.getUpdatedValue(this.viewCell.internalMetadata)))); - templateData.editor.updateOptions(cellEditorOptions.getUpdatedValue(this.viewCell.internalMetadata)); + this._register(cellEditorOptions.onDidChange(() => templateData.editor.updateOptions(cellEditorOptions.getUpdatedValue(this.viewCell.internalMetadata, this.viewCell.uri)))); + templateData.editor.updateOptions(cellEditorOptions.getUpdatedValue(this.viewCell.internalMetadata, this.viewCell.uri)); cellEditorOptions.setLineNumbers(this.viewCell.lineNumbers); } @@ -148,7 +158,7 @@ export class CodeCell extends Disposable { private calculateInitEditorHeight() { const lineNum = this.viewCell.lineCount; const lineHeight = this.viewCell.layoutInfo.fontInfo?.lineHeight || 17; - const editorPadding = this.notebookEditor.notebookOptions.computeEditorPadding(this.viewCell.internalMetadata); + const editorPadding = this.notebookEditor.notebookOptions.computeEditorPadding(this.viewCell.internalMetadata, this.viewCell.uri); const editorHeight = this.viewCell.layoutInfo.editorHeight === 0 ? lineNum * lineHeight + editorPadding.top + editorPadding.bottom : this.viewCell.layoutInfo.editorHeight; @@ -210,10 +220,10 @@ export class CodeCell extends Disposable { } const isReadonly = this.notebookEditor.isReadOnly; - const padding = this.notebookEditor.notebookOptions.computeEditorPadding(this.viewCell.internalMetadata); + const padding = this.notebookEditor.notebookOptions.computeEditorPadding(this.viewCell.internalMetadata, this.viewCell.uri); const options = editor.getOptions(); if (options.get(EditorOption.readOnly) !== isReadonly || options.get(EditorOption.padding) !== padding) { - editor.updateOptions({ readOnly: this.notebookEditor.isReadOnly, padding: this.notebookEditor.notebookOptions.computeEditorPadding(this.viewCell.internalMetadata) }); + editor.updateOptions({ readOnly: this.notebookEditor.isReadOnly, padding: this.notebookEditor.notebookOptions.computeEditorPadding(this.viewCell.internalMetadata, this.viewCell.uri) }); } } @@ -316,7 +326,7 @@ export class CodeCell extends Disposable { // Mouse click handlers this._register(this.templateData.statusBar.onDidClick(e => { if (e.type !== ClickTargetType.ContributedCommandItem) { - const target = this.templateData.editor.getTargetAtClientPoint(e.event.clientX, e.event.clientY - this.notebookEditor.notebookOptions.computeEditorStatusbarHeight(this.viewCell.internalMetadata)); + const target = this.templateData.editor.getTargetAtClientPoint(e.event.clientX, e.event.clientY - this.notebookEditor.notebookOptions.computeEditorStatusbarHeight(this.viewCell.internalMetadata, this.viewCell.uri)); if (target?.position) { this.templateData.editor.setPosition(target.position); this.templateData.editor.focus(); diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/markdownCell.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/markdownCell.ts index 77d3325c914..09902c5cb81 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/markdownCell.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/markdownCell.ts @@ -62,7 +62,7 @@ export class StatefulMarkdownCell extends Disposable { this.cellEditorOptions = this._register(new CellEditorOptions(this.notebookEditor, this.notebookEditor.notebookOptions, this.configurationService, this.viewCell.language)); this.cellEditorOptions.setLineNumbers(this.viewCell.lineNumbers); - this.editorOptions = this.cellEditorOptions.getValue(this.viewCell.internalMetadata); + this.editorOptions = this.cellEditorOptions.getValue(this.viewCell.internalMetadata, this.viewCell.uri); this._register(toDisposable(() => renderedEditors.delete(this.viewCell))); this.registerListeners(); @@ -172,7 +172,7 @@ export class StatefulMarkdownCell extends Disposable { })); this._register(this.cellEditorOptions.onDidChange(() => { - this.updateEditorOptions(this.cellEditorOptions.getUpdatedValue(this.viewCell.internalMetadata)); + this.updateEditorOptions(this.cellEditorOptions.getUpdatedValue(this.viewCell.internalMetadata, this.viewCell.uri)); })); } @@ -298,7 +298,7 @@ export class StatefulMarkdownCell extends Disposable { const width = this.notebookEditor.notebookOptions.computeMarkdownCellEditorWidth(this.notebookEditor.getLayoutInfo().width); const lineNum = this.viewCell.lineCount; const lineHeight = this.viewCell.layoutInfo.fontInfo?.lineHeight || 17; - const editorPadding = this.notebookEditor.notebookOptions.computeEditorPadding(this.viewCell.internalMetadata); + const editorPadding = this.notebookEditor.notebookOptions.computeEditorPadding(this.viewCell.internalMetadata, this.viewCell.uri); editorHeight = Math.max(lineNum, 1) * lineHeight + editorPadding.top + editorPadding.bottom; this.templateData.editorContainer.innerText = ''; diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts index dac839548b4..bf12c68e19a 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/cellRenderer.ts @@ -396,7 +396,7 @@ export class CodeCellRenderer extends AbstractCellRenderer implements IListRende EditorContextKeys.inCompositeEditor.bindTo(editorContextKeyService).set(true); const editor = editorInstaService.createInstance(CodeEditorWidget, editorContainer, { - ...this.editorOptions.getValue(), + ...this.editorOptions.getDefaultValue(), dimension: { width: 0, height: 0 @@ -408,7 +408,7 @@ export class CodeCellRenderer extends AbstractCellRenderer implements IListRende templateDisposables.add(editor); - const progressBar = templateDisposables.add(new CellProgressBar(editorPart, cellInputCollapsedContainer)); + const progressBar = templateDisposables.add(this.instantiationService.createInstance(CellProgressBar, editorPart, cellInputCollapsedContainer)); const statusBar = templateDisposables.add(this.instantiationService.createInstance(CellEditorStatusBar, this.notebookEditor, container, editorPart)); diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel.ts index eeb62ce906c..0cbafc68992 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/baseCellViewModel.ts @@ -188,8 +188,8 @@ export abstract class BaseCellViewModel extends Disposable { })); this._register(model.onDidChangeInternalMetadata(e => { - this._onDidChangeState.fire({ internalMetadataChanged: true, runStateChanged: e.runStateChanged }); - if (e.runStateChanged || e.lastRunSuccessChanged) { + this._onDidChangeState.fire({ internalMetadataChanged: true }); + if (e.lastRunSuccessChanged) { // Statusbar visibility may change this.layoutChange({}); } @@ -473,7 +473,7 @@ export abstract class BaseCellViewModel extends Disposable { return 0; } - const editorPadding = this._viewContext.notebookOptions.computeEditorPadding(this.internalMetadata); + const editorPadding = this._viewContext.notebookOptions.computeEditorPadding(this.internalMetadata, this.uri); return this._textEditor.getTopForLineNumber(line) + editorPadding.top; } @@ -482,7 +482,7 @@ export abstract class BaseCellViewModel extends Disposable { return 0; } - const editorPadding = this._viewContext.notebookOptions.computeEditorPadding(this.internalMetadata); + const editorPadding = this._viewContext.notebookOptions.computeEditorPadding(this.internalMetadata, this.uri); return this._textEditor.getTopForPosition(line, column) + editorPadding.top; } diff --git a/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts b/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts index 84a5bbbf7dd..e6a2f0b2403 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewModel/codeCellViewModel.ts @@ -186,7 +186,7 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod newState = CellLayoutState.Estimated; } - const statusbarHeight = this.viewContext.notebookOptions.computeEditorStatusbarHeight(this.internalMetadata); + const statusbarHeight = this.viewContext.notebookOptions.computeEditorStatusbarHeight(this.internalMetadata, this.uri); const indicatorHeight = editorHeight + statusbarHeight + outputTotalHeight + outputShowMoreContainerHeight; const outputContainerOffset = notebookLayoutConfiguration.editorToolbarHeight + notebookLayoutConfiguration.cellTopMargin // CELL_TOP_MARGIN @@ -314,7 +314,7 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod } const verticalScrollbarHeight = hasScrolling ? 12 : 0; // take zoom level into account - const editorPadding = this.viewContext.notebookOptions.computeEditorPadding(this.internalMetadata); + const editorPadding = this.viewContext.notebookOptions.computeEditorPadding(this.internalMetadata, this.uri); return this.lineCount * lineHeight + editorPadding.top + editorPadding.bottom // EDITOR_BOTTOM_PADDING @@ -327,7 +327,7 @@ export class CodeCellViewModel extends BaseCellViewModel implements ICellViewMod return layoutConfiguration.editorToolbarHeight + layoutConfiguration.cellTopMargin + editorHeight - + this.viewContext.notebookOptions.computeEditorStatusbarHeight(this.internalMetadata) + + this.viewContext.notebookOptions.computeEditorStatusbarHeight(this.internalMetadata, this.uri) + outputsTotalHeight + outputShowMoreContainerHeight + bottomToolbarGap diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts index 6ae8727a7ee..446c3f23ccf 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts @@ -5,9 +5,8 @@ import { DisposableStore, dispose, IDisposable } from 'vs/base/common/lifecycle'; import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; -import { ICellViewModel, KERNEL_EXTENSIONS, NOTEBOOK_MISSING_KERNEL_EXTENSION, NOTEBOOK_HAS_OUTPUTS, NOTEBOOK_HAS_RUNNING_CELL, NOTEBOOK_INTERRUPTIBLE_KERNEL, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_KERNEL_SELECTED, NOTEBOOK_USE_CONSOLIDATED_OUTPUT_BUTTON, NOTEBOOK_VIEW_TYPE, INotebookEditorDelegate, NOTEBOOK_CELL_TOOLBAR_LOCATION } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { CellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel'; -import { NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { ICellViewModel, INotebookEditorDelegate, KERNEL_EXTENSIONS, NOTEBOOK_CELL_TOOLBAR_LOCATION, NOTEBOOK_HAS_OUTPUTS, NOTEBOOK_HAS_RUNNING_CELL, NOTEBOOK_INTERRUPTIBLE_KERNEL, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_KERNEL_SELECTED, NOTEBOOK_MISSING_KERNEL_EXTENSION, NOTEBOOK_USE_CONSOLIDATED_OUTPUT_BUTTON, NOTEBOOK_VIEW_TYPE } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { INotebookExecutionStateService } 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'; @@ -25,14 +24,14 @@ export class NotebookEditorContextKeys { private readonly _disposables = new DisposableStore(); private readonly _viewModelDisposables = new DisposableStore(); - private readonly _cellStateListeners: IDisposable[] = []; private readonly _cellOutputsListeners: IDisposable[] = []; constructor( private readonly _editor: INotebookEditorDelegate, @INotebookKernelService private readonly _notebookKernelService: INotebookKernelService, @IContextKeyService contextKeyService: IContextKeyService, - @IExtensionService private readonly _extensionService: IExtensionService + @IExtensionService private readonly _extensionService: IExtensionService, + @INotebookExecutionStateService private readonly _notebookExecutionStateService: INotebookExecutionStateService ) { this._notebookKernelCount = NOTEBOOK_KERNEL_COUNT.bindTo(contextKeyService); this._notebookKernelSelected = NOTEBOOK_KERNEL_SELECTED.bindTo(contextKeyService); @@ -52,6 +51,7 @@ export class NotebookEditorContextKeys { this._disposables.add(_notebookKernelService.onDidChangeSelectedNotebooks(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)); } dispose(): void { @@ -61,8 +61,6 @@ export class NotebookEditorContextKeys { this._interruptibleKernel.reset(); this._someCellRunning.reset(); this._viewType.reset(); - dispose(this._cellStateListeners); - this._cellStateListeners.length = 0; dispose(this._cellOutputsListeners); this._cellOutputsListeners.length = 0; } @@ -73,8 +71,6 @@ export class NotebookEditorContextKeys { this._updateForNotebookOptions(); this._viewModelDisposables.clear(); - dispose(this._cellStateListeners); - this._cellStateListeners.length = 0; dispose(this._cellOutputsListeners); this._cellOutputsListeners.length = 0; @@ -82,22 +78,6 @@ export class NotebookEditorContextKeys { return; } - let executionCount = 0; - - const addCellStateListener = (c: ICellViewModel) => { - return (c as CellViewModel).onDidChangeState(e => { - if (!e.runStateChanged) { - return; - } - if (c.internalMetadata.runState === NotebookCellExecutionState.Pending) { - executionCount++; - } else if (!c.internalMetadata.runState) { - executionCount--; - } - this._someCellRunning.set(executionCount > 0); - }); - }; - const recomputeOutputsExistence = () => { let hasOutputs = false; if (this._editor.hasModel()) { @@ -120,7 +100,6 @@ export class NotebookEditorContextKeys { for (let i = 0; i < this._editor.getLength(); i++) { const cell = this._editor.cellAt(i); - this._cellStateListeners.push(addCellStateListener(cell)); this._cellOutputsListeners.push(addCellOutputsListener(cell)); } @@ -130,15 +109,22 @@ export class NotebookEditorContextKeys { this._viewModelDisposables.add(this._editor.onDidChangeViewCells(e => { e.splices.reverse().forEach(splice => { const [start, deleted, newCells] = splice; - const deletedCellStates = this._cellStateListeners.splice(start, deleted, ...newCells.map(addCellStateListener)); const deletedCellOutputStates = this._cellOutputsListeners.splice(start, deleted, ...newCells.map(addCellOutputsListener)); - dispose(deletedCellStates); dispose(deletedCellOutputStates); }); })); this._viewType.set(this._editor.textModel.viewType); } + private _updateForCellExecution(): void { + if (this._editor.textModel) { + const notebookExe = this._notebookExecutionStateService.getCellExecutionStatesForNotebook(this._editor.textModel.uri); + this._someCellRunning.set(notebookExe.length > 0); + } else { + this._someCellRunning.set(false); + } + } + private async _updateForInstalledExtension(): Promise { if (!this._editor.hasModel()) { return; diff --git a/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts b/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts index c77b02f841a..6bacd5d869b 100644 --- a/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts +++ b/src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts @@ -62,7 +62,6 @@ export class NotebookCellTextModel extends Disposable implements ICell { } set internalMetadata(newInternalMetadata: NotebookCellInternalMetadata) { - const runStateChanged = this._internalMetadata.runState !== newInternalMetadata.runState; const lastRunSuccessChanged = this._internalMetadata.lastRunSuccess !== newInternalMetadata.lastRunSuccess; newInternalMetadata = { ...newInternalMetadata, @@ -70,7 +69,7 @@ export class NotebookCellTextModel extends Disposable implements ICell { }; this._internalMetadata = newInternalMetadata; this._hash = null; - this._onDidChangeInternalMetadata.fire({ runStateChanged, lastRunSuccessChanged }); + this._onDidChangeInternalMetadata.fire({ lastRunSuccessChanged }); } get language() { diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index d61c9cb7ecd..436fa84c992 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -101,7 +101,6 @@ export interface NotebookCellMetadata { export interface NotebookCellInternalMetadata { executionOrder?: number; lastRunSuccess?: boolean; - runState?: NotebookCellExecutionState; runStartTime?: number; runStartTimeAdjustment?: number; runEndTime?: number; @@ -200,7 +199,6 @@ export interface ICellOutput { } export interface CellInternalMetadataChangedEvent { - readonly runStateChanged?: boolean; readonly lastRunSuccessChanged?: boolean; } diff --git a/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts b/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts index 5d8599d067b..a17d4b964a1 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookExecutionStateService.ts @@ -6,6 +6,7 @@ import { Event } from 'vs/base/common/event'; import { URI } from 'vs/base/common/uri'; import { createDecorator } from 'vs/platform/instantiation/common/instantiation'; +import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import { NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { CellExecutionUpdateType, ICellExecuteOutputEdit, ICellExecuteOutputItemEdit } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; @@ -22,13 +23,17 @@ export interface ICellExecutionComplete { lastRunSuccess?: boolean; } -export interface ICellExecutionState { +export interface ICellExecutionEntry { + notebook: URI; + cellHandle: number; state: NotebookCellExecutionState; } -export interface INotebookExecutionEvent { +export interface ICellExecutionStateChangedEvent { notebook: URI; cellHandle: number; + changed?: ICellExecutionEntry; // undefined -> execution was completed + affectsCell(cell: NotebookCellTextModel): boolean; } export const INotebookExecutionStateService = createDecorator('INotebookExecutionStateService'); @@ -36,9 +41,11 @@ export const INotebookExecutionStateService = createDecorator; + onDidChangeCellExecution: Event; - getCellExecutionState(notebook: URI, handle: number): ICellExecutionState | undefined; + getCellExecutionStatesForNotebook(notebook: URI): ICellExecutionEntry[]; + + getCellExecutionState(cellUri: URI): ICellExecutionEntry | undefined; createNotebookCellExecution(notebook: URI, cellHandle: number): void; updateNotebookCellExecution(notebook: URI, cellHandle: number, updates: ICellExecuteUpdate[]): void; diff --git a/src/vs/workbench/contrib/notebook/common/notebookOptions.ts b/src/vs/workbench/contrib/notebook/common/notebookOptions.ts index a6253379f4a..1bf1ebf6c19 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookOptions.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookOptions.ts @@ -5,8 +5,10 @@ import { Emitter } from 'vs/base/common/event'; import { Disposable } from 'vs/base/common/lifecycle'; +import { URI } from 'vs/base/common/uri'; import { IConfigurationChangeEvent, IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { NotebookCellDefaultCollapseConfig, NotebookCellInternalMetadata, NotebookSetting, ShowCellStatusBarType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; const SCROLLABLE_ELEMENT_PADDING_TOP = 18; @@ -109,7 +111,11 @@ export class NotebookOptions extends Disposable { protected readonly _onDidChangeOptions = this._register(new Emitter()); readonly onDidChangeOptions = this._onDidChangeOptions.event; - constructor(private readonly configurationService: IConfigurationService, private readonly overrides?: { cellToolbarInteraction: string, globalToolbar: boolean, defaultCellCollapseConfig?: NotebookCellDefaultCollapseConfig }) { + constructor( + private readonly configurationService: IConfigurationService, + private readonly notebookExecutionStateService: INotebookExecutionStateService, + private readonly overrides?: { cellToolbarInteraction: string, globalToolbar: boolean, defaultCellCollapseConfig?: NotebookCellDefaultCollapseConfig } + ) { super(); const showCellStatusBar = this.configurationService.getValue(NotebookSetting.showCellStatusBar); const globalToolbar = overrides?.globalToolbar ?? this.configurationService.getValue(NotebookSetting.globalToolbar) ?? true; @@ -439,25 +445,26 @@ export class NotebookOptions extends Disposable { return 0; } - computeEditorPadding(internalMetadata: NotebookCellInternalMetadata) { + computeEditorPadding(internalMetadata: NotebookCellInternalMetadata, cellUri: URI) { return { top: getEditorTopPadding(), - bottom: this.statusBarIsVisible(internalMetadata) + bottom: this.statusBarIsVisible(internalMetadata, cellUri) ? this._layoutConfiguration.editorBottomPadding : this._layoutConfiguration.editorBottomPaddingWithoutStatusBar }; } - computeEditorStatusbarHeight(internalMetadata: NotebookCellInternalMetadata) { - return this.statusBarIsVisible(internalMetadata) ? this.computeStatusBarHeight() : 0; + computeEditorStatusbarHeight(internalMetadata: NotebookCellInternalMetadata, cellUri: URI) { + return this.statusBarIsVisible(internalMetadata, cellUri) ? this.computeStatusBarHeight() : 0; } - private statusBarIsVisible(internalMetadata: NotebookCellInternalMetadata): boolean { + private statusBarIsVisible(internalMetadata: NotebookCellInternalMetadata, cellUri: URI): boolean { + const exe = this.notebookExecutionStateService.getCellExecutionState(cellUri); if (this._layoutConfiguration.showCellStatusBar === 'visible') { return true; } else if (this._layoutConfiguration.showCellStatusBar === 'visibleAfterExecute') { - return typeof internalMetadata.lastRunSuccess === 'boolean' || internalMetadata.runState !== undefined; + return typeof internalMetadata.lastRunSuccess === 'boolean' || exe !== undefined; } else { return false; } diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookCellList.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookCellList.test.ts index 6a0fdf7b6da..8cb9c0908bf 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookCellList.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookCellList.test.ts @@ -8,6 +8,7 @@ import { DisposableStore } from 'vs/base/common/lifecycle'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { NotebookOptions } from 'vs/workbench/contrib/notebook/common/notebookOptions'; import { createNotebookCellList, setupInstantiationService, withTestNotebook } from 'vs/workbench/contrib/notebook/test/browser/testNotebookEditor'; @@ -20,7 +21,7 @@ suite('NotebookCellList', () => { suiteSetup(() => { disposables = new DisposableStore(); instantiationService = setupInstantiationService(disposables); - notebookDefaultOptions = new NotebookOptions(instantiationService.get(IConfigurationService)); + notebookDefaultOptions = new NotebookOptions(instantiationService.get(IConfigurationService), instantiationService.get(INotebookExecutionStateService)); topInsertToolbarHeight = notebookDefaultOptions.computeTopInsertToolbarHeight(); }); 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 de512bd4011..bc9e8f02dca 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts @@ -9,7 +9,7 @@ import { Event } from 'vs/base/common/event'; import { DisposableStore } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; import { mock } from 'vs/base/test/common/mock'; -import { PLAINTEXT_MODE_ID } from 'vs/editor/common/modes/modesRegistry'; +import { PLAINTEXT_LANGUAGE_ID } from 'vs/editor/common/modes/modesRegistry'; import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; import { insertCellAtIndex } from 'vs/workbench/contrib/notebook/browser/controller/cellOperations'; @@ -111,6 +111,6 @@ class TestNotebookKernel implements INotebookKernel { } constructor(opts?: { languages: string[] }) { - this.supportedLanguages = opts?.languages ?? [PLAINTEXT_MODE_ID]; + this.supportedLanguages = opts?.languages ?? [PLAINTEXT_LANGUAGE_ID]; } } diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookViewModel.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookViewModel.test.ts index 80bb2747e40..36516ad9703 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookViewModel.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookViewModel.test.ts @@ -26,6 +26,7 @@ import { CellKind, diff } from 'vs/workbench/contrib/notebook/common/notebookCom import { NotebookOptions } from 'vs/workbench/contrib/notebook/common/notebookOptions'; import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange'; import { NotebookEditorTestModel, setupInstantiationService, withTestNotebook } from 'vs/workbench/contrib/notebook/test/browser/testNotebookEditor'; +import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; suite('NotebookViewModel', () => { let disposables: DisposableStore; @@ -54,7 +55,7 @@ suite('NotebookViewModel', () => { test('ctor', function () { const notebook = new NotebookTextModel('notebook', URI.parse('test'), [], {}, { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false }, undoRedoService, modelService, languageService); const model = new NotebookEditorTestModel(notebook); - const viewContext = new ViewContext(new NotebookOptions(instantiationService.get(IConfigurationService)), new NotebookEventDispatcher()); + const viewContext = new ViewContext(new NotebookOptions(instantiationService.get(IConfigurationService), instantiationService.get(INotebookExecutionStateService)), new NotebookEventDispatcher()); const viewModel = new NotebookViewModel('notebook', model.notebook, viewContext, null, { isReadOnly: false }, instantiationService, bulkEditService, undoRedoService, textModelService); assert.strictEqual(viewModel.viewType, 'notebook'); }); diff --git a/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts b/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts index c8c120c36ed..d307a745dcf 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/testNotebookEditor.ts @@ -15,10 +15,10 @@ import { mock } from 'vs/base/test/common/mock'; import { EditorFontLigatures } from 'vs/editor/common/config/editorOptions'; import { FontInfo } from 'vs/editor/common/config/fontInfo'; import { ILanguageConfigurationService } from 'vs/editor/common/modes/languageConfigurationRegistry'; -import { IModelService } from 'vs/editor/common/services/modelService'; -import { ModelServiceImpl } from 'vs/editor/common/services/modelServiceImpl'; import { ILanguageService } from 'vs/editor/common/services/languageService'; import { LanguageService } from 'vs/editor/common/services/languageServiceImpl'; +import { IModelService } from 'vs/editor/common/services/modelService'; +import { ModelServiceImpl } from 'vs/editor/common/services/modelServiceImpl'; import { ITextModelService } from 'vs/editor/common/services/resolverService'; import { TestLanguageConfigurationService } from 'vs/editor/test/common/modes/testLanguageConfigurationService'; import { BrowserClipboardService } from 'vs/platform/clipboard/browser/clipboardService'; @@ -29,6 +29,7 @@ import { TestConfigurationService } from 'vs/platform/configuration/test/common/ import { ContextKeyService } from 'vs/platform/contextkey/browser/contextKeyService'; import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; +import { ILayoutService } from 'vs/platform/layout/browser/layoutService'; import { IListService, ListService } from 'vs/platform/list/browser/listService'; import { ILogService, NullLogService } from 'vs/platform/log/common/log'; import { IStorageService } from 'vs/platform/storage/common/storage'; @@ -49,13 +50,13 @@ import { ViewContext } from 'vs/workbench/contrib/notebook/browser/viewModel/vie import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { CellKind, CellUri, INotebookDiffEditorModel, INotebookEditorModel, IOutputDto, IResolvedNotebookEditorModel, NotebookCellMetadata, SelectionStateType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { ICellExecuteUpdate, ICellExecutionComplete, ICellExecutionEntry, ICellExecutionStateChangedEvent, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { NotebookOptions } from 'vs/workbench/contrib/notebook/common/notebookOptions'; import { ICellRange } from 'vs/workbench/contrib/notebook/common/notebookRange'; import { TextModelResolverService } from 'vs/workbench/services/textmodelResolver/common/textModelResolverService'; import { TestWorkspaceTrustRequestService } from 'vs/workbench/services/workspaces/test/common/testWorkspaceTrustService'; -import { TestStorageService } from 'vs/workbench/test/common/workbenchTestServices'; -import { ILayoutService } from 'vs/platform/layout/browser/layoutService'; import { TestLayoutService } from 'vs/workbench/test/browser/workbenchTestServices'; +import { TestStorageService } from 'vs/workbench/test/common/workbenchTestServices'; export class TestCell extends NotebookCellTextModel { constructor( @@ -174,6 +175,7 @@ export function setupInstantiationService(disposables = new DisposableStore()) { instantiationService.stub(IClipboardService, instantiationService.createInstance(BrowserClipboardService)); instantiationService.stub(IStorageService, new TestStorageService()); instantiationService.stub(IWorkspaceTrustRequestService, new TestWorkspaceTrustRequestService(true)); + instantiationService.stub(INotebookExecutionStateService, new TestNotebookExecutionStateService()); return instantiationService; } @@ -192,7 +194,7 @@ function _createTestNotebookEditor(instantiationService: TestInstantiationServic }), {}, { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false }); const model = new NotebookEditorTestModel(notebook); - const notebookOptions = new NotebookOptions(instantiationService.get(IConfigurationService)); + const notebookOptions = new NotebookOptions(instantiationService.get(IConfigurationService), instantiationService.get(INotebookExecutionStateService)); const viewContext = new ViewContext(notebookOptions, new NotebookEventDispatcher()); const viewModel: NotebookViewModel = instantiationService.createInstance(NotebookViewModel, viewType, model.notebook, viewContext, null, { isReadOnly: false }); @@ -385,7 +387,7 @@ export function createNotebookCellList(instantiationService: TestInstantiationSe 'NotebookCellList', DOM.$('container'), DOM.$('body'), - viewContext ?? new ViewContext(new NotebookOptions(instantiationService.get(IConfigurationService)), new NotebookEventDispatcher()), + viewContext ?? new ViewContext(new NotebookOptions(instantiationService.get(IConfigurationService), instantiationService.get(INotebookExecutionStateService)), new NotebookEventDispatcher()), delegate, [renderer], instantiationService.get(IContextKeyService), @@ -406,3 +408,26 @@ export function createNotebookCellList(instantiationService: TestInstantiationSe export function valueBytesFromString(value: string): VSBuffer { return VSBuffer.fromString(value); } + +class TestNotebookExecutionStateService implements INotebookExecutionStateService { + _serviceBrand: undefined; + + onDidChangeCellExecution = new Emitter().event; + + getCellExecutionStatesForNotebook(notebook: URI): ICellExecutionEntry[] { + return []; + } + + getCellExecutionState(cellUri: URI): ICellExecutionEntry | undefined { + return undefined; + } + + createNotebookCellExecution(notebook: URI, cellHandle: number): void { + } + + updateNotebookCellExecution(notebook: URI, cellHandle: number, updates: ICellExecuteUpdate[]): void { + } + + completeNotebookCellExecution(notebook: URI, cellHandle: number, complete: ICellExecutionComplete): void { + } +}