From 7e34708dcccfbec7837c825ed85cc470ec20df89 Mon Sep 17 00:00:00 2001 From: rebornix Date: Wed, 11 May 2022 11:46:20 -0700 Subject: [PATCH 01/13] prepare for removing proxy kernel. --- .../api/browser/mainThreadNotebookKernels.ts | 8 ++++- .../browser/mainThreadNotebookProxyKernels.ts | 10 +++++- .../workbench/api/common/extHost.api.impl.ts | 1 + .../workbench/api/common/extHost.protocol.ts | 6 ++++ .../api/common/extHostNotebookKernels.ts | 9 +++++ src/vs/workbench/api/common/extHostTypes.ts | 5 +++ .../editorStatusBar/editorStatusBar.ts | 8 ++--- .../notebook/browser/controller/apiActions.ts | 6 ++-- .../browser/view/cellParts/cellExecution.ts | 3 +- .../view/renderers/backLayerWebView.ts | 12 +++---- .../notebookEditorWidgetContextKeys.ts | 4 +-- .../viewParts/notebookKernelActionViewItem.ts | 34 ++++++++----------- .../notebook/common/notebookKernelService.ts | 13 +++++++ ...code.proposed.notebookProxyController.d.ts | 8 +++++ 14 files changed, 89 insertions(+), 38 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts index a8809c58016..fc5501b8162 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts @@ -17,7 +17,7 @@ import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/no import { INotebookCellExecution, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { IResolvedNotebookKernel, INotebookKernelChangeEvent, INotebookKernelService, NotebookKernelType } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { SerializableObjectWithBuffers } from 'vs/workbench/services/extensions/common/proxyIdentifier'; -import { ExtHostContext, ExtHostNotebookKernelsShape, ICellExecuteUpdateDto, ICellExecutionCompleteDto, INotebookKernelDto2, MainContext, MainThreadNotebookKernelsShape } from '../common/extHost.protocol'; +import { ExtHostContext, ExtHostNotebookKernelsShape, ICellExecuteUpdateDto, ICellExecutionCompleteDto, INotebookKernelDto2, MainContext, MainThreadNotebookKernelsShape, NotebookControllerState } from '../common/extHost.protocol'; abstract class MainThreadKernel implements IResolvedNotebookKernel { readonly type: NotebookKernelType.Resolved = NotebookKernelType.Resolved; @@ -35,6 +35,7 @@ abstract class MainThreadKernel implements IResolvedNotebookKernel { description?: string; detail?: string; kind?: string; + state?: NotebookControllerState; supportedLanguages: string[]; implementsExecutionOrder: boolean; localResourceRoot: URI; @@ -57,6 +58,7 @@ abstract class MainThreadKernel implements IResolvedNotebookKernel { this.description = data.description; this.detail = data.detail; this.kind = data.kind; + this.state = data.state; this.supportedLanguages = isNonEmptyArray(data.supportedLanguages) ? data.supportedLanguages : _languageService.getRegisteredLanguageIds(); this.implementsExecutionOrder = data.supportsExecutionOrder ?? false; this.localResourceRoot = URI.revive(data.extensionLocation); @@ -83,6 +85,10 @@ abstract class MainThreadKernel implements IResolvedNotebookKernel { this.kind = data.kind; event.kind = true; } + if (data.state !== undefined) { + this.state = data.state; + event.state = true; + } if (data.supportedLanguages !== undefined) { this.supportedLanguages = isNonEmptyArray(data.supportedLanguages) ? data.supportedLanguages : this._languageService.getRegisteredLanguageIds(); event.supportedLanguages = true; diff --git a/src/vs/workbench/api/browser/mainThreadNotebookProxyKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookProxyKernels.ts index 74e7290161d..e7a83bb8631 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookProxyKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookProxyKernels.ts @@ -7,9 +7,10 @@ import { Emitter, Event } from 'vs/base/common/event'; import { DisposableStore, IDisposable } from 'vs/base/common/lifecycle'; import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; import { extHostNamedCustomer, IExtHostContext } from 'vs/workbench/services/extensions/common/extHostCustomers'; -import { INotebookKernelService, INotebookProxyKernel, INotebookProxyKernelChangeEvent, ProxyKernelState, NotebookKernelType } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernelService, INotebookProxyKernel, INotebookProxyKernelChangeEvent, ProxyKernelState, NotebookKernelType, NotebookControllerState } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { ExtHostContext, ExtHostNotebookProxyKernelsShape, INotebookProxyKernelDto, MainContext, MainThreadNotebookProxyKernelsShape } from '../common/extHost.protocol'; import { onUnexpectedError } from 'vs/base/common/errors'; +import { URI } from 'vs/base/common/uri'; abstract class MainThreadProxyKernel implements INotebookProxyKernel { readonly type: NotebookKernelType.Proxy = NotebookKernelType.Proxy; @@ -19,11 +20,16 @@ abstract class MainThreadProxyKernel implements INotebookProxyKernel { readonly viewType: string; readonly extension: ExtensionIdentifier; readonly preloadProvides: string[] = []; + readonly preloadUris: URI[] = []; label: string; description?: string; detail?: string; kind?: string; supportedLanguages: string[] = []; + localResourceRoot: URI; + state?: NotebookControllerState | undefined; + implementsInterrupt?: boolean | undefined; + implementsExecutionOrder?: boolean | undefined; connectionState: ProxyKernelState; constructor(data: INotebookProxyKernelDto) { @@ -35,10 +41,12 @@ abstract class MainThreadProxyKernel implements INotebookProxyKernel { this.description = data.description; this.detail = data.detail; this.kind = data.kind; + this.localResourceRoot = URI.revive(data.extensionLocation); this.connectionState = ProxyKernelState.Disconnected; } + update(data: Partial) { const event: INotebookProxyKernelChangeEvent = Object.create(null); if (data.label !== undefined) { diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index bba2d04f64e..86cb018e404 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -1320,6 +1320,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I NotebookCellOutputItem: extHostTypes.NotebookCellOutputItem, NotebookCellStatusBarItem: extHostTypes.NotebookCellStatusBarItem, NotebookControllerAffinity: extHostTypes.NotebookControllerAffinity, + NotebookControllerState: extHostTypes.NotebookControllerState, PortAttributes: extHostTypes.PortAttributes, LinkedEditingRanges: extHostTypes.LinkedEditingRanges, TestResultState: extHostTypes.TestResultState, diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 9fa08b4b6c3..25f33cb3f4a 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -970,6 +970,11 @@ export interface MainThreadNotebookDocumentsShape extends IDisposable { $trySaveNotebook(uri: UriComponents): Promise; } +export enum NotebookControllerState { + Idle = 1, + Connecting = 2 +} + export interface INotebookKernelDto2 { id: string; notebookType: string; @@ -979,6 +984,7 @@ export interface INotebookKernelDto2 { detail?: string; description?: string; kind?: string; + state?: NotebookControllerState; supportedLanguages?: string[]; supportsInterrupt?: boolean; supportsExecutionOrder?: boolean; diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index f99ead59ac7..4e4ca9640c5 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -196,6 +196,15 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { get rendererScripts() { return data.preloads ? data.preloads.map(extHostTypeConverters.NotebookRendererScript.to) : []; }, + get state() { + checkProposedApiEnabled(extension, 'notebookProxyController'); + return data.state; + }, + set state(value) { + checkProposedApiEnabled(extension, 'notebookProxyController'); + data.state = value; + _update(); + }, get executeHandler() { return _executeHandler; }, diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index 977f0cdf142..10844646688 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -3369,6 +3369,11 @@ export class NotebookRendererScript { } } +export enum NotebookControllerState { + Idle = 1, + Connecting = 2 +} + //#endregion //#region Timeline diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts b/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts index 920669194f6..22233100d5e 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts @@ -28,7 +28,7 @@ import { NotebookEditorWidget } from 'vs/workbench/contrib/notebook/browser/note import { configureKernelIcon, selectKernelIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { NotebookCellsChangeType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { INotebookKernel, INotebookKernelService, NotebookKernelType } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernel, INotebookKernelService, NotebookControllerState } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle'; import { IPaneCompositePartService } from 'vs/workbench/services/panecomposite/browser/panecomposite'; @@ -205,8 +205,8 @@ registerAction2(class extends Action2 { }); } - if (!all.find(item => item.type === NotebookKernelType.Resolved)) { - // there is no resolved kernel, show the install from marketplace + if (!all.length) { + // there is no kernel, show the install from marketplace quickPickItems.push({ id: 'install', label: nls.localize('installKernels', "Install kernels from the marketplace"), @@ -379,7 +379,7 @@ export class KernelStatus extends Disposable implements IWorkbenchContribution { this._kernelInfoElement.add(this._statusbarService.addEntry( { name: nls.localize('notebook.info', "Notebook Kernel Info"), - text: `$(notebook-kernel-select) ${kernel.label}`, + text: `$(notebook-kernel-select) ${kernel.label}` + (kernel.state === NotebookControllerState.Idle ? '' : ' Connecting...'), ariaLabel: kernel.label, tooltip: isSuggested ? nls.localize('tooltop', "{0} (suggestion)", tooltip) : tooltip, command: SELECT_KERNEL_ID, diff --git a/src/vs/workbench/contrib/notebook/browser/controller/apiActions.ts b/src/vs/workbench/contrib/notebook/browser/controller/apiActions.ts index 68d9d0f8dcc..d0c6379a54b 100644 --- a/src/vs/workbench/contrib/notebook/browser/controller/apiActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/controller/apiActions.ts @@ -7,7 +7,7 @@ import * as glob from 'vs/base/common/glob'; import { URI, UriComponents } from 'vs/base/common/uri'; import { CommandsRegistry } from 'vs/platform/commands/common/commands'; import { isDocumentExcludePattern, TransientCellMetadata, TransientDocumentMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { INotebookKernelService, IResolvedNotebookKernel, NotebookKernelType } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; CommandsRegistry.registerCommand('_resolveNotebookContentProvider', (accessor): { @@ -66,13 +66,13 @@ CommandsRegistry.registerCommand('_resolveNotebookKernels', async (accessor, arg const uri = URI.revive(args.uri as UriComponents); const kernels = notebookKernelService.getMatchingKernel({ uri, viewType: args.viewType }); - return kernels.all.filter(kernel => kernel.type === NotebookKernelType.Resolved).map((provider) => ({ + return kernels.all.map(provider => ({ id: provider.id, label: provider.label, kind: provider.kind, description: provider.description, detail: provider.detail, isPreferred: false, // todo@jrieken,@rebornix - preloads: (provider as IResolvedNotebookKernel).preloadUris, + preloads: provider.preloadUris, })); }); diff --git a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellExecution.ts b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellExecution.ts index 469e7cbef3d..1d9865937ea 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellExecution.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/cellParts/cellExecution.ts @@ -9,7 +9,6 @@ import { ICellViewModel, INotebookEditorDelegate } from 'vs/workbench/contrib/no import { CellViewModelStateChangeEvent } from 'vs/workbench/contrib/notebook/browser/notebookViewEvents'; import { CellPart } from 'vs/workbench/contrib/notebook/browser/view/cellPart'; import { NotebookCellInternalMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { NotebookKernelType } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; export class CellExecutionPart extends CellPart { private kernelDisposables = this._register(new DisposableStore()); @@ -42,7 +41,7 @@ export class CellExecutionPart extends CellPart { } private updateExecutionOrder(internalMetadata: NotebookCellInternalMetadata): void { - if (this._notebookEditor.activeKernel?.type === NotebookKernelType.Resolved && this._notebookEditor.activeKernel?.implementsExecutionOrder) { + if (this._notebookEditor.activeKernel?.implementsExecutionOrder) { const executionOrderLabel = typeof internalMetadata.executionOrder === 'number' ? `[${internalMetadata.executionOrder}]` : '[ ]'; diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index cc2eb8bf93a..1d82b850c6d 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -37,7 +37,7 @@ import { preloadsScriptStr, RendererMetadata } from 'vs/workbench/contrib/notebo import { transformWebviewThemeVars } from 'vs/workbench/contrib/notebook/browser/view/renderers/webviewThemeMapping'; import { MarkupCellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/markupCellViewModel'; import { CellUri, INotebookRendererInfo, NotebookSetting, RendererMessagingSpec } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { INotebookKernel, IResolvedNotebookKernel, NotebookKernelType } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernel } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { IScopedRendererMessaging } from 'vs/workbench/contrib/notebook/common/notebookRendererMessagingService'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { IWebviewElement, IWebviewService, WebviewContentPurpose } from 'vs/workbench/contrib/webview/browser/webview'; @@ -907,7 +907,7 @@ var requirejs = (function() { } this._preloadsCache.clear(); - if (this._currentKernel?.type === NotebookKernelType.Resolved) { + if (this._currentKernel) { this._updatePreloadsFromKernel(this._currentKernel); } @@ -1407,14 +1407,14 @@ var requirejs = (function() { const previousKernel = this._currentKernel; this._currentKernel = kernel; - if (previousKernel?.type === NotebookKernelType.Resolved && previousKernel.preloadUris.length > 0) { + if (previousKernel && previousKernel.preloadUris.length > 0) { this.webview?.reload(); // preloads will be restored after reload - } else if (kernel?.type === NotebookKernelType.Resolved) { + } else if (kernel) { this._updatePreloadsFromKernel(kernel); } } - private _updatePreloadsFromKernel(kernel: IResolvedNotebookKernel) { + private _updatePreloadsFromKernel(kernel: INotebookKernel) { const resources: IControllerPreload[] = []; for (const preload of kernel.preloadUris) { const uri = this.environmentService.isExtensionDevelopment && (preload.scheme === 'http' || preload.scheme === 'https') @@ -1440,7 +1440,7 @@ var requirejs = (function() { const mixedResourceRoots = [ ...(this.localResourceRootsCache || []), - ...(this._currentKernel?.type === NotebookKernelType.Resolved ? [this._currentKernel.localResourceRoot] : []), + ...(this._currentKernel ? [this._currentKernel.localResourceRoot] : []), ]; this.webview.localResourcesRoot = mixedResourceRoots; diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts index b1cf70a4946..a90477db542 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookEditorWidgetContextKeys.ts @@ -8,7 +8,7 @@ import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/c import { ICellViewModel, INotebookEditorDelegate, KERNEL_EXTENSIONS } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { NOTEBOOK_CELL_TOOLBAR_LOCATION, NOTEBOOK_HAS_OUTPUTS, NOTEBOOK_HAS_RUNNING_CELL, NOTEBOOK_INTERRUPTIBLE_KERNEL, NOTEBOOK_KERNEL, NOTEBOOK_KERNEL_COUNT, NOTEBOOK_KERNEL_SELECTED, NOTEBOOK_MISSING_KERNEL_EXTENSION, NOTEBOOK_USE_CONSOLIDATED_OUTPUT_BUTTON, NOTEBOOK_VIEW_TYPE } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; -import { INotebookKernelService, NotebookKernelType } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { IExtensionService } from 'vs/workbench/services/extensions/common/extensions'; export class NotebookEditorContextKeys { @@ -148,7 +148,7 @@ export class NotebookEditorContextKeys { const { selected, all } = this._notebookKernelService.getMatchingKernel(this._editor.textModel); this._notebookKernelCount.set(all.length); - this._interruptibleKernel.set((selected?.type === NotebookKernelType.Resolved && selected.implementsInterrupt) ?? false); + this._interruptibleKernel.set(selected?.implementsInterrupt ?? false); this._notebookKernelSelected.set(Boolean(selected)); this._notebookKernel.set(selected?.id ?? ''); } diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts index e5814ce98ec..69917ca972b 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts @@ -10,7 +10,7 @@ import { DisposableStore } from 'vs/base/common/lifecycle'; import { localize } from 'vs/nls'; import { ThemeIcon } from 'vs/platform/theme/common/themeService'; import { selectKernelIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons'; -import { INotebookKernelMatchResult, INotebookKernelService, NotebookKernelType, ProxyKernelState } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernel, INotebookKernelMatchResult, INotebookKernelService, NotebookControllerState } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { Event } from 'vs/base/common/event'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; @@ -68,32 +68,20 @@ export class NotebooKernelActionViewItem extends ActionViewItem { private _updateActionFromKernelInfo(info: INotebookKernelMatchResult): void { this._kernelDisposable.clear(); this._action.enabled = true; - const selectedOrSuggested = info.selected ?? ((info.suggestions.length === 1 && info.suggestions[0].type === NotebookKernelType.Resolved) ? info.suggestions[0] : undefined); + const selectedOrSuggested = info.selected ?? (info.suggestions.length === 1 ? info.suggestions[0] : undefined); if (selectedOrSuggested) { // selected or suggested kernel - this._action.label = selectedOrSuggested.label; + this._action.label = this._generateKenrelLabel(selectedOrSuggested); this._action.tooltip = selectedOrSuggested.description ?? selectedOrSuggested.detail ?? ''; if (!info.selected) { // special UI for selected kernel? } - if (selectedOrSuggested.type === NotebookKernelType.Proxy) { - if (selectedOrSuggested.connectionState === ProxyKernelState.Initializing) { - this._action.label = localize('initializing', "Initializing..."); - } else { - this._action.label = selectedOrSuggested.label; + this._kernelDisposable.add(selectedOrSuggested.onDidChange(e => { + if (e.state) { + this._action.label = this._generateKenrelLabel(selectedOrSuggested); } - - this._kernelDisposable.add(selectedOrSuggested.onDidChange(e => { - if (e.connectionState) { - if (selectedOrSuggested.connectionState === ProxyKernelState.Initializing) { - this._action.label = localize('initializing', "Initializing..."); - } else { - this._action.label = selectedOrSuggested.label; - } - } - })); - } + })); } else { // many kernels or no kernels this._action.label = localize('select', "Select Kernel"); @@ -101,6 +89,14 @@ export class NotebooKernelActionViewItem extends ActionViewItem { } } + private _generateKenrelLabel(kernel: INotebookKernel) { + if (kernel.state === NotebookControllerState.Connecting) { + return localize('kernelconnecting', "Connecting..."); + } else { + return kernel.label; + } + } + private _resetAction(): void { this._action.enabled = false; this._action.label = ''; diff --git a/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts b/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts index 4f5304600b0..7a23d36ecd5 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts @@ -27,6 +27,7 @@ export interface INotebookKernelChangeEvent { description?: true; detail?: true; kind?: true; + state?: true; supportedLanguages?: true; hasExecutionOrder?: true; } @@ -36,6 +37,11 @@ export const enum NotebookKernelType { Proxy = 1 } +export enum NotebookControllerState { + Idle = 1, + Connecting = 2 +} + export interface IResolvedNotebookKernel { readonly type: NotebookKernelType.Resolved; readonly id: string; @@ -51,6 +57,7 @@ export interface IResolvedNotebookKernel { description?: string; detail?: string; kind?: string; + state?: NotebookControllerState; supportedLanguages: string[]; implementsInterrupt?: boolean; implementsExecutionOrder?: boolean; @@ -74,13 +81,19 @@ export interface INotebookProxyKernel { readonly id: string; readonly viewType: string; readonly extension: ExtensionIdentifier; + readonly localResourceRoot: URI; + readonly preloadUris: URI[]; readonly preloadProvides: string[]; + readonly onDidChange: Event>; label: string; description?: string; detail?: string; kind?: string; + state?: NotebookControllerState; supportedLanguages: string[]; + implementsInterrupt?: boolean; + implementsExecutionOrder?: boolean; connectionState: ProxyKernelState; resolveKernel(uri: URI): Promise; } diff --git a/src/vscode-dts/vscode.proposed.notebookProxyController.d.ts b/src/vscode-dts/vscode.proposed.notebookProxyController.d.ts index 07f8e833f15..3fe7cac5428 100644 --- a/src/vscode-dts/vscode.proposed.notebookProxyController.d.ts +++ b/src/vscode-dts/vscode.proposed.notebookProxyController.d.ts @@ -5,6 +5,14 @@ declare module 'vscode' { + export enum NotebookControllerState { + Idle = 1, + Connecting = 2 + } + + export interface NotebookController { + state?: NotebookControllerState; + } export interface NotebookProxyController { /** From 027b7bf21e056d25e50365fa0fa87f93a47c1d7f Mon Sep 17 00:00:00 2001 From: rebornix Date: Sun, 15 May 2022 18:24:06 -0700 Subject: [PATCH 02/13] Notebook state only. --- .../api/browser/mainThreadNotebookKernels.ts | 9 +++-- .../browser/mainThreadNotebookProxyKernels.ts | 16 +++++++++ .../browser/notebookExecutionServiceImpl.ts | 36 +++++++------------ .../notebook/common/notebookKernelService.ts | 4 +++ 4 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts index fc5501b8162..35ffe7bb7ec 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts @@ -25,7 +25,8 @@ abstract class MainThreadKernel implements IResolvedNotebookKernel { private readonly _onDidChange = new Emitter(); private readonly preloads: { uri: URI; provides: string[] }[]; readonly onDidChange: Event = this._onDidChange.event; - + private readonly _onDispose = new Emitter(); + readonly onDispose = this._onDispose.event; readonly id: string; readonly viewType: string; readonly extension: ExtensionIdentifier; @@ -100,6 +101,10 @@ abstract class MainThreadKernel implements IResolvedNotebookKernel { this._onDidChange.fire(event); } + dispose() { + this._onDispose.fire(); + } + abstract executeNotebookCellsRequest(uri: URI, cellHandles: number[]): Promise; abstract cancelNotebookCellExecution(uri: URI, cellHandles: number[]): Promise; } @@ -225,7 +230,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape }); const registration = this._notebookKernelService.registerKernel(kernel); - this._kernels.set(handle, [kernel, combinedDisposable(listener, registration)]); + this._kernels.set(handle, [kernel, combinedDisposable(kernel, listener, registration)]); } $updateKernel(handle: number, data: Partial): void { diff --git a/src/vs/workbench/api/browser/mainThreadNotebookProxyKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookProxyKernels.ts index e7a83bb8631..8d13aa86f72 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookProxyKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookProxyKernels.ts @@ -16,6 +16,8 @@ abstract class MainThreadProxyKernel implements INotebookProxyKernel { readonly type: NotebookKernelType.Proxy = NotebookKernelType.Proxy; protected readonly _onDidChange = new Emitter(); readonly onDidChange: Event = this._onDidChange.event; + private readonly _onDispose = new Emitter(); + readonly onDispose = this._onDispose.event; readonly id: string; readonly viewType: string; readonly extension: ExtensionIdentifier; @@ -69,7 +71,13 @@ abstract class MainThreadProxyKernel implements INotebookProxyKernel { this._onDidChange.fire(event); } + dispose() { + this._onDispose.fire(); + } + abstract resolveKernel(): Promise; + abstract executeNotebookCellsRequest(uri: URI, cellHandles: number[]): Promise; + abstract cancelNotebookCellExecution(uri: URI, cellHandles: number[]): Promise; } @extHostNamedCustomer(MainContext.MainThreadNotebookProxyKernels) @@ -115,6 +123,8 @@ export class MainThreadNotebookProxyKernels implements MainThreadNotebookProxyKe return null; } } + async executeNotebookCellsRequest(uri: URI, cellHandles: number[]): Promise { } + async cancelNotebookCellExecution(uri: URI, cellHandles: number[]): Promise { } }(data); const registration = this._notebookKernelService.registerKernel(proxyKernel); @@ -135,4 +145,10 @@ export class MainThreadNotebookProxyKernels implements MainThreadNotebookProxyKe this._proxyKernels.delete(handle); } } + + async executeNotebookCellsRequest(uri: URI, cellHandles: number[]): Promise { + } + + async cancelNotebookCellExecution(uri: URI, cellHandles: number[]): Promise { + } } diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts index 2d4fe4cebe0..732b53f22e4 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts @@ -48,29 +48,6 @@ export class NotebookExecutionService implements INotebookExecutionService, IDis return; } - if (kernel.type === NotebookKernelType.Proxy) { - this._activeProxyKernelExecutionToken?.dispose(true); - const tokenSource = this._activeProxyKernelExecutionToken = new CancellationTokenSource(); - const resolved = await kernel.resolveKernel(notebook.uri); - const kernels = this._notebookKernelService.getMatchingKernel(notebook); - const newlyMatchedKernel = kernels.all.find(k => k.id === resolved); - - if (!newlyMatchedKernel) { - return; - } - - kernel = newlyMatchedKernel; - if (tokenSource.token.isCancellationRequested) { - // execution was cancelled but we still need to update the active kernel - this._notebookKernelService.selectKernelForNotebook(kernel, notebook); - return; - } - } - - if (kernel.type === NotebookKernelType.Proxy) { - return; - } - const executeCells: NotebookCellTextModel[] = []; for (const cell of cellsArr) { const cellExe = this._notebookExecutionStateService.getCellExecution(cell.uri); @@ -83,11 +60,24 @@ export class NotebookExecutionService implements INotebookExecutionService, IDis executeCells.push(cell); } + if (kernel.state !== undefined) { + // kernel has connection state management + const kernelId = kernel.id; + kernel.onDispose(() => { + // proxy kernel scenario, kernel disposed, we should now make way for new preferred kernel + const exes = executeCells.map(c => this._notebookExecutionStateService.createCellExecution(kernelId, notebook.uri, c.handle)); + exes.forEach(e => e.complete({})); + + this.executeNotebookCells(notebook, executeCells); + }); + } + if (executeCells.length > 0) { this._notebookKernelService.selectKernelForNotebook(kernel, notebook); const exes = executeCells.map(c => this._notebookExecutionStateService.createCellExecution(kernel!.id, notebook.uri, c.handle)); await kernel.executeNotebookCellsRequest(notebook.uri, executeCells.map(c => c.handle)); + // the connecting state can change before the kernel resolves executeNotebookCellsRequest const unconfirmed = exes.filter(exe => exe.state === NotebookCellExecutionState.Unconfirmed); if (unconfirmed.length) { this._logService.debug(`NotebookExecutionService#executeNotebookCells completing unconfirmed executions ${JSON.stringify(unconfirmed.map(exe => exe.cellHandle))}`); diff --git a/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts b/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts index 7a23d36ecd5..081eab0180e 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts @@ -64,6 +64,7 @@ export interface IResolvedNotebookKernel { executeNotebookCellsRequest(uri: URI, cellHandles: number[]): Promise; cancelNotebookCellExecution(uri: URI, cellHandles: number[]): Promise; + onDispose: Event; } export const enum ProxyKernelState { @@ -96,6 +97,9 @@ export interface INotebookProxyKernel { implementsExecutionOrder?: boolean; connectionState: ProxyKernelState; resolveKernel(uri: URI): Promise; + executeNotebookCellsRequest(uri: URI, cellHandles: number[]): Promise; + cancelNotebookCellExecution(uri: URI, cellHandles: number[]): Promise; + onDispose: Event; } export type INotebookKernel = IResolvedNotebookKernel | INotebookProxyKernel; From bd99c998a14e34dc17a3a745bb60f00a13391564 Mon Sep 17 00:00:00 2001 From: rebornix Date: Sun, 15 May 2022 18:56:22 -0700 Subject: [PATCH 03/13] Remove proxy controller. --- .../api/browser/mainThreadNotebookKernels.ts | 6 +- .../browser/mainThreadNotebookProxyKernels.ts | 154 ----------------- .../workbench/api/common/extHost.api.impl.ts | 6 - .../api/common/extHostNotebookProxyKernels.ts | 157 ------------------ .../browser/notebookExecutionServiceImpl.ts | 8 +- .../notebook/common/notebookKernelService.ts | 35 +--- .../browser/notebookExecutionService.test.ts | 11 +- .../notebookExecutionStateService.test.ts | 6 +- .../browser/notebookKernelService.test.ts | 7 +- ...code.proposed.notebookProxyController.d.ts | 48 ------ 10 files changed, 19 insertions(+), 419 deletions(-) delete mode 100644 src/vs/workbench/api/browser/mainThreadNotebookProxyKernels.ts delete mode 100644 src/vs/workbench/api/common/extHostNotebookProxyKernels.ts diff --git a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts index 35ffe7bb7ec..38c81e4ba0e 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts @@ -15,13 +15,11 @@ import { extHostNamedCustomer, IExtHostContext } from 'vs/workbench/services/ext import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/notebookEditorService'; import { INotebookCellExecution, INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; -import { IResolvedNotebookKernel, INotebookKernelChangeEvent, INotebookKernelService, NotebookKernelType } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +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, INotebookKernelDto2, MainContext, MainThreadNotebookKernelsShape, NotebookControllerState } from '../common/extHost.protocol'; -abstract class MainThreadKernel implements IResolvedNotebookKernel { - readonly type: NotebookKernelType.Resolved = NotebookKernelType.Resolved; - +abstract class MainThreadKernel implements INotebookKernel { private readonly _onDidChange = new Emitter(); private readonly preloads: { uri: URI; provides: string[] }[]; readonly onDidChange: Event = this._onDidChange.event; diff --git a/src/vs/workbench/api/browser/mainThreadNotebookProxyKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookProxyKernels.ts deleted file mode 100644 index 8d13aa86f72..00000000000 --- a/src/vs/workbench/api/browser/mainThreadNotebookProxyKernels.ts +++ /dev/null @@ -1,154 +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 { Emitter, Event } from 'vs/base/common/event'; -import { DisposableStore, IDisposable } from 'vs/base/common/lifecycle'; -import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; -import { extHostNamedCustomer, IExtHostContext } from 'vs/workbench/services/extensions/common/extHostCustomers'; -import { INotebookKernelService, INotebookProxyKernel, INotebookProxyKernelChangeEvent, ProxyKernelState, NotebookKernelType, NotebookControllerState } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; -import { ExtHostContext, ExtHostNotebookProxyKernelsShape, INotebookProxyKernelDto, MainContext, MainThreadNotebookProxyKernelsShape } from '../common/extHost.protocol'; -import { onUnexpectedError } from 'vs/base/common/errors'; -import { URI } from 'vs/base/common/uri'; - -abstract class MainThreadProxyKernel implements INotebookProxyKernel { - readonly type: NotebookKernelType.Proxy = NotebookKernelType.Proxy; - protected readonly _onDidChange = new Emitter(); - readonly onDidChange: Event = this._onDidChange.event; - private readonly _onDispose = new Emitter(); - readonly onDispose = this._onDispose.event; - readonly id: string; - readonly viewType: string; - readonly extension: ExtensionIdentifier; - readonly preloadProvides: string[] = []; - readonly preloadUris: URI[] = []; - label: string; - description?: string; - detail?: string; - kind?: string; - supportedLanguages: string[] = []; - localResourceRoot: URI; - state?: NotebookControllerState | undefined; - implementsInterrupt?: boolean | undefined; - implementsExecutionOrder?: boolean | undefined; - connectionState: ProxyKernelState; - - constructor(data: INotebookProxyKernelDto) { - this.id = data.id; - this.viewType = data.notebookType; - this.extension = data.extensionId; - - this.label = data.label; - this.description = data.description; - this.detail = data.detail; - this.kind = data.kind; - this.localResourceRoot = URI.revive(data.extensionLocation); - - this.connectionState = ProxyKernelState.Disconnected; - } - - - update(data: Partial) { - const event: INotebookProxyKernelChangeEvent = Object.create(null); - if (data.label !== undefined) { - this.label = data.label; - event.label = true; - } - if (data.description !== undefined) { - this.description = data.description; - event.description = true; - } - if (data.detail !== undefined) { - this.detail = data.detail; - event.detail = true; - } - if (data.kind !== undefined) { - this.kind = data.kind; - event.kind = true; - } - - this._onDidChange.fire(event); - } - - dispose() { - this._onDispose.fire(); - } - - abstract resolveKernel(): Promise; - abstract executeNotebookCellsRequest(uri: URI, cellHandles: number[]): Promise; - abstract cancelNotebookCellExecution(uri: URI, cellHandles: number[]): Promise; -} - -@extHostNamedCustomer(MainContext.MainThreadNotebookProxyKernels) -export class MainThreadNotebookProxyKernels implements MainThreadNotebookProxyKernelsShape { - - private readonly _disposables = new DisposableStore(); - - private readonly _proxyKernels = new Map(); - private readonly _proxyKernelProxy: ExtHostNotebookProxyKernelsShape; - - constructor( - extHostContext: IExtHostContext, - @INotebookKernelService private readonly _notebookKernelService: INotebookKernelService, - ) { - this._proxyKernelProxy = extHostContext.getProxy(ExtHostContext.ExtHostNotebookProxyKernels); - } - - dispose(): void { - this._disposables.dispose(); - - for (let [, registration] of this._proxyKernels.values()) { - registration.dispose(); - } - } - - // -- Proxy kernel - - async $addProxyKernel(handle: number, data: INotebookProxyKernelDto): Promise { - const that = this; - const proxyKernel = new class extends MainThreadProxyKernel { - async resolveKernel(): Promise { - try { - this.connectionState = ProxyKernelState.Initializing; - this._onDidChange.fire({ connectionState: true }); - const delegateKernel = await that._proxyKernelProxy.$resolveKernel(handle); - this.connectionState = ProxyKernelState.Connected; - this._onDidChange.fire({ connectionState: true }); - return delegateKernel; - } catch (err) { - onUnexpectedError(err); - this.connectionState = ProxyKernelState.Disconnected; - this._onDidChange.fire({ connectionState: true }); - return null; - } - } - async executeNotebookCellsRequest(uri: URI, cellHandles: number[]): Promise { } - async cancelNotebookCellExecution(uri: URI, cellHandles: number[]): Promise { } - }(data); - - const registration = this._notebookKernelService.registerKernel(proxyKernel); - this._proxyKernels.set(handle, [proxyKernel, registration]); - } - - $updateProxyKernel(handle: number, data: Partial): void { - const tuple = this._proxyKernels.get(handle); - if (tuple) { - tuple[0].update(data); - } - } - - $removeProxyKernel(handle: number): void { - const tuple = this._proxyKernels.get(handle); - if (tuple) { - tuple[1].dispose(); - this._proxyKernels.delete(handle); - } - } - - async executeNotebookCellsRequest(uri: URI, cellHandles: number[]): Promise { - } - - async cancelNotebookCellExecution(uri: URI, cellHandles: number[]): Promise { - } -} diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index 86cb018e404..43e08b3776b 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -93,7 +93,6 @@ import { ExtHostInteractive } from 'vs/workbench/api/common/extHostInteractive'; import { combinedDisposable } from 'vs/base/common/lifecycle'; import { checkProposedApiEnabled, ExtensionIdentifierSet, isProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions'; import { DebugConfigurationProviderTriggerKind } from 'vs/workbench/contrib/debug/common/debug'; -import { ExtHostNotebookProxyKernels } from 'vs/workbench/api/common/extHostNotebookProxyKernels'; export interface IExtensionRegistries { mine: ExtensionDescriptionRegistry; @@ -161,7 +160,6 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I const extHostNotebookDocuments = rpcProtocol.set(ExtHostContext.ExtHostNotebookDocuments, new ExtHostNotebookDocuments(extHostNotebook)); const extHostNotebookEditors = rpcProtocol.set(ExtHostContext.ExtHostNotebookEditors, new ExtHostNotebookEditors(extHostLogService, rpcProtocol, extHostNotebook)); const extHostNotebookKernels = rpcProtocol.set(ExtHostContext.ExtHostNotebookKernels, new ExtHostNotebookKernels(rpcProtocol, initData, extHostNotebook, extHostCommands, extHostLogService)); - const extHostNotebookProxyKernels = rpcProtocol.set(ExtHostContext.ExtHostNotebookProxyKernels, new ExtHostNotebookProxyKernels(rpcProtocol, extHostNotebookKernels, extHostLogService)); const extHostNotebookRenderers = rpcProtocol.set(ExtHostContext.ExtHostNotebookRenderers, new ExtHostNotebookRenderers(rpcProtocol, extHostNotebook)); const extHostEditors = rpcProtocol.set(ExtHostContext.ExtHostEditors, new ExtHostEditors(rpcProtocol, extHostDocumentsAndEditors)); const extHostTreeViews = rpcProtocol.set(ExtHostContext.ExtHostTreeViews, new ExtHostTreeViews(rpcProtocol.getProxy(MainContext.MainThreadTreeViews), extHostCommands, extHostLogService)); @@ -1153,10 +1151,6 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I onDidChangeNotebookCellExecutionState(listener, thisArgs?, disposables?) { checkProposedApiEnabled(extension, 'notebookCellExecutionState'); return extHostNotebookKernels.onDidChangeNotebookCellExecutionState(listener, thisArgs, disposables); - }, - createNotebookProxyController(id: string, notebookType: string, label: string, handler: () => vscode.NotebookController | string | Thenable) { - checkProposedApiEnabled(extension, 'notebookProxyController'); - return extHostNotebookProxyKernels.createNotebookProxyController(extension, id, notebookType, label, handler); } }; diff --git a/src/vs/workbench/api/common/extHostNotebookProxyKernels.ts b/src/vs/workbench/api/common/extHostNotebookProxyKernels.ts deleted file mode 100644 index 786101bf360..00000000000 --- a/src/vs/workbench/api/common/extHostNotebookProxyKernels.ts +++ /dev/null @@ -1,157 +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 { Emitter } from 'vs/base/common/event'; -import { DisposableStore } from 'vs/base/common/lifecycle'; -import { ResourceMap } from 'vs/base/common/map'; -import { ExtensionIdentifier, IExtensionDescription } from 'vs/platform/extensions/common/extensions'; -import { ILogService } from 'vs/platform/log/common/log'; -import { ExtHostNotebookProxyKernelsShape, IMainContext, INotebookProxyKernelDto, MainContext, MainThreadNotebookProxyKernelsShape } from 'vs/workbench/api/common/extHost.protocol'; -import { createKernelId, ExtHostNotebookKernels } from 'vs/workbench/api/common/extHostNotebookKernels'; -import { checkProposedApiEnabled } from 'vs/workbench/services/extensions/common/extensions'; -import * as vscode from 'vscode'; - -interface IProxyKernelData { - extensionId: ExtensionIdentifier; - controller: vscode.NotebookProxyController; - onDidChangeSelection: Emitter<{ selected: boolean; notebook: vscode.NotebookDocument }>; - associatedNotebooks: ResourceMap; -} - -export type SelectKernelReturnArgs = ControllerInfo | { notebookEditorId: string } | ControllerInfo & { notebookEditorId: string } | undefined; -type ControllerInfo = { id: string; extension: string }; - - -export class ExtHostNotebookProxyKernels implements ExtHostNotebookProxyKernelsShape { - - private readonly _proxy: MainThreadNotebookProxyKernelsShape; - - private readonly _proxyKernelData: Map = new Map(); - private _handlePool: number = 0; - - private readonly _onDidChangeCellExecutionState = new Emitter(); - readonly onDidChangeNotebookCellExecutionState = this._onDidChangeCellExecutionState.event; - - constructor( - mainContext: IMainContext, - private readonly extHostNotebook: ExtHostNotebookKernels, - @ILogService private readonly _logService: ILogService - ) { - this._proxy = mainContext.getProxy(MainContext.MainThreadNotebookProxyKernels); - } - - createNotebookProxyController(extension: IExtensionDescription, id: string, viewType: string, label: string, handler: () => vscode.NotebookController | string | Thenable): vscode.NotebookProxyController { - const handle = this._handlePool++; - - let isDisposed = false; - const commandDisposables = new DisposableStore(); - const onDidChangeSelection = new Emitter<{ selected: boolean; notebook: vscode.NotebookDocument }>(); - - const data: INotebookProxyKernelDto = { - id: createKernelId(extension.identifier, id), - notebookType: viewType, - extensionId: extension.identifier, - extensionLocation: extension.extensionLocation, - label: label || extension.identifier.value, - }; - - let _resolveHandler = handler; - - this._proxy.$addProxyKernel(handle, data).catch(err => { - // this can happen when a kernel with that ID is already registered - console.log(err); - isDisposed = true; - }); - - let tokenPool = 0; - const _update = () => { - if (isDisposed) { - return; - } - const myToken = ++tokenPool; - Promise.resolve().then(() => { - if (myToken === tokenPool) { - this._proxy.$updateProxyKernel(handle, data); - } - }); - }; - - // notebook documents that are associated to this controller - const associatedNotebooks = new ResourceMap(); - - const controller: vscode.NotebookProxyController = { - get id() { return id; }, - get notebookType() { return data.notebookType; }, - onDidChangeSelectedNotebooks: onDidChangeSelection.event, - get label() { - return data.label; - }, - set label(value) { - data.label = value ?? extension.displayName ?? extension.name; - _update(); - }, - get detail() { - return data.detail ?? ''; - }, - set detail(value) { - data.detail = value; - _update(); - }, - get description() { - return data.description ?? ''; - }, - set description(value) { - data.description = value; - _update(); - }, - get kind() { - checkProposedApiEnabled(extension, 'notebookControllerKind'); - return data.kind ?? ''; - }, - set kind(value) { - checkProposedApiEnabled(extension, 'notebookControllerKind'); - data.kind = value; - _update(); - }, - get resolveHandler() { - return _resolveHandler; - }, - dispose: () => { - if (!isDisposed) { - this._logService.trace(`NotebookProxyController[${handle}], DISPOSED`); - isDisposed = true; - this._proxyKernelData.delete(handle); - commandDisposables.dispose(); - onDidChangeSelection.dispose(); - this._proxy.$removeProxyKernel(handle); - } - } - }; - - this._proxyKernelData.set(handle, { - extensionId: extension.identifier, - controller, - onDidChangeSelection, - associatedNotebooks - }); - return controller; - } - - async $resolveKernel(handle: number): Promise { - const obj = this._proxyKernelData.get(handle); - if (!obj) { - // extension can dispose kernels in the meantime - return null; - } - - const controller = await obj.controller.resolveHandler(); - if (typeof controller === 'string') { - return controller; - } else { - return this.extHostNotebook.getIdByController(controller); - } - } -} - diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts index 732b53f22e4..c92696d34eb 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts @@ -14,7 +14,7 @@ import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/mode import { CellKind, INotebookTextModel, NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; -import { INotebookKernelService, NotebookKernelType } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; export class NotebookExecutionService implements INotebookExecutionService, IDisposable { declare _serviceBrand: undefined; @@ -91,11 +91,7 @@ export class NotebookExecutionService implements INotebookExecutionService, IDis this._logService.debug(`NotebookExecutionService#cancelNotebookCellHandles ${JSON.stringify(cellsArr)}`); const kernel = this._notebookKernelService.getSelectedOrSuggestedKernel(notebook); if (kernel) { - if (kernel.type === NotebookKernelType.Proxy) { - this._activeProxyKernelExecutionToken?.dispose(true); - } else { - await kernel.cancelNotebookCellExecution(notebook.uri, cellsArr); - } + await kernel.cancelNotebookCellExecution(notebook.uri, cellsArr); } } diff --git a/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts b/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts index 081eab0180e..13fd479ef1a 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts @@ -32,18 +32,12 @@ export interface INotebookKernelChangeEvent { hasExecutionOrder?: true; } -export const enum NotebookKernelType { - Resolved, - Proxy = 1 -} - export enum NotebookControllerState { Idle = 1, Connecting = 2 } -export interface IResolvedNotebookKernel { - readonly type: NotebookKernelType.Resolved; +export interface INotebookKernel { readonly id: string; readonly viewType: string; readonly onDidChange: Event>; @@ -77,33 +71,6 @@ export interface INotebookProxyKernelChangeEvent extends INotebookKernelChangeEv connectionState?: true; } -export interface INotebookProxyKernel { - readonly type: NotebookKernelType.Proxy; - readonly id: string; - readonly viewType: string; - readonly extension: ExtensionIdentifier; - readonly localResourceRoot: URI; - readonly preloadUris: URI[]; - readonly preloadProvides: string[]; - - readonly onDidChange: Event>; - label: string; - description?: string; - detail?: string; - kind?: string; - state?: NotebookControllerState; - supportedLanguages: string[]; - implementsInterrupt?: boolean; - implementsExecutionOrder?: boolean; - connectionState: ProxyKernelState; - resolveKernel(uri: URI): Promise; - executeNotebookCellsRequest(uri: URI, cellHandles: number[]): Promise; - cancelNotebookCellExecution(uri: URI, cellHandles: number[]): Promise; - onDispose: Event; -} - -export type INotebookKernel = IResolvedNotebookKernel | INotebookProxyKernel; - export interface INotebookTextModelLike { uri: URI; viewType: string } export const INotebookKernelService = createDecorator('INotebookKernelService'); diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts index 8d3bff7a83a..9ceec332a7a 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts @@ -20,7 +20,7 @@ import { NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewMod import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { CellKind, IOutputDto, NotebookCellMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; -import { INotebookKernelService, IResolvedNotebookKernel, ISelectedNotebooksChangeEvent, NotebookKernelType } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernel, INotebookKernelService, ISelectedNotebooksChangeEvent, NotebookControllerState } 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'; @@ -166,8 +166,7 @@ suite('NotebookExecutionService', () => { }); }); -class TestNotebookKernel implements IResolvedNotebookKernel { - type: NotebookKernelType.Resolved = NotebookKernelType.Resolved; +class TestNotebookKernel implements INotebookKernel { id: string = 'test'; label: string = ''; viewType = '*'; @@ -179,14 +178,18 @@ class TestNotebookKernel implements IResolvedNotebookKernel { preloadUris: URI[] = []; preloadProvides: string[] = []; supportedLanguages: string[] = []; + onDispose = Event.None; 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_LANGUAGE_ID]; } + kind?: string | undefined; + state?: NotebookControllerState | undefined; + implementsInterrupt?: boolean | undefined; + implementsExecutionOrder?: boolean | 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 index e5b7f84b8bf..4763ee855c6 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts @@ -21,7 +21,7 @@ import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/no import { CellEditType, CellKind, CellUri, IOutputDto, NotebookCellMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; -import { INotebookKernelService, IResolvedNotebookKernel, NotebookKernelType } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +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'; @@ -170,8 +170,7 @@ suite('NotebookExecutionStateService', () => { }); }); -class TestNotebookKernel implements IResolvedNotebookKernel { - type: NotebookKernelType.Resolved = NotebookKernelType.Resolved; +class TestNotebookKernel implements INotebookKernel { id: string = 'test'; label: string = ''; viewType = '*'; @@ -183,6 +182,7 @@ class TestNotebookKernel implements IResolvedNotebookKernel { preloadUris: URI[] = []; preloadProvides: string[] = []; supportedLanguages: string[] = []; + onDispose = Event.None; async executeNotebookCellsRequest(): Promise { } async cancelNotebookCellExecution(): Promise { } diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookKernelService.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookKernelService.test.ts index 62d6afecb8b..54b3b52cb3c 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookKernelService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookKernelService.test.ts @@ -8,7 +8,7 @@ import { URI } from 'vs/base/common/uri'; import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; import { setupInstantiationService, withTestNotebook as _withTestNotebook } from 'vs/workbench/contrib/notebook/test/browser/testNotebookEditor'; import { Emitter, Event } from 'vs/base/common/event'; -import { INotebookKernelService, IResolvedNotebookKernel, NotebookKernelType } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernel, INotebookKernelService, NotebookControllerState } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { NotebookKernelService } from 'vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { mock } from 'vs/base/test/common/mock'; @@ -159,8 +159,7 @@ suite('NotebookKernelService', () => { }); }); -class TestNotebookKernel implements IResolvedNotebookKernel { - type: NotebookKernelType.Resolved = NotebookKernelType.Resolved; +class TestNotebookKernel implements INotebookKernel { id: string = Math.random() + 'kernel'; label: string = 'test-label'; viewType = '*'; @@ -172,6 +171,8 @@ class TestNotebookKernel implements IResolvedNotebookKernel { preloadUris: URI[] = []; preloadProvides: string[] = []; supportedLanguages: string[] = []; + state?: NotebookControllerState | undefined = undefined; + onDispose = Event.None; executeNotebookCellsRequest(): Promise { throw new Error('Method not implemented.'); } diff --git a/src/vscode-dts/vscode.proposed.notebookProxyController.d.ts b/src/vscode-dts/vscode.proposed.notebookProxyController.d.ts index 3fe7cac5428..df429123049 100644 --- a/src/vscode-dts/vscode.proposed.notebookProxyController.d.ts +++ b/src/vscode-dts/vscode.proposed.notebookProxyController.d.ts @@ -13,52 +13,4 @@ declare module 'vscode' { export interface NotebookController { state?: NotebookControllerState; } - - export interface NotebookProxyController { - /** - * The identifier of this notebook controller. - * - * _Note_ that controllers are remembered by their identifier and that extensions should use - * stable identifiers across sessions. - */ - readonly id: string; - - /** - * The notebook type this controller is for. - */ - readonly notebookType: string; - - /** - * The human-readable label of this notebook controller. - */ - label: string; - - /** - * The human-readable description which is rendered less prominent. - */ - description?: string; - - /** - * The human-readable detail which is rendered less prominent. - */ - detail?: string; - - /** - * The human-readable label used to categorise controllers. - */ - kind?: string; - - resolveHandler: () => NotebookController | string | Thenable; - - readonly onDidChangeSelectedNotebooks: Event<{ readonly notebook: NotebookDocument; readonly selected: boolean }>; - - /** - * Dispose and free associated resources. - */ - dispose(): void; - } - - export namespace notebooks { - export function createNotebookProxyController(id: string, notebookType: string, label: string, resolveHandler: () => NotebookController | string | Thenable): NotebookProxyController; - } } From 64030273a62b9075b3369eaf0da6cd68251a3215 Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 16 May 2022 11:24:05 -0700 Subject: [PATCH 04/13] remove main thread proxy kenrel --- .../api/browser/extensionHost.contribution.ts | 1 - src/vs/workbench/api/common/extHost.protocol.ts | 12 ------------ 2 files changed, 13 deletions(-) diff --git a/src/vs/workbench/api/browser/extensionHost.contribution.ts b/src/vs/workbench/api/browser/extensionHost.contribution.ts index 2fa247a959b..37afee52e07 100644 --- a/src/vs/workbench/api/browser/extensionHost.contribution.ts +++ b/src/vs/workbench/api/browser/extensionHost.contribution.ts @@ -64,7 +64,6 @@ import './mainThreadWorkspace'; import './mainThreadComments'; import './mainThreadNotebook'; import './mainThreadNotebookKernels'; -import './mainThreadNotebookProxyKernels'; import './mainThreadNotebookDocumentsAndEditors'; import './mainThreadNotebookRenderers'; import './mainThreadInteractive'; diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index 25f33cb3f4a..607bc5ed01d 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -1035,12 +1035,6 @@ export interface MainThreadNotebookKernelsShape extends IDisposable { $completeExecution(handle: number, data: SerializableObjectWithBuffers): void; } -export interface MainThreadNotebookProxyKernelsShape extends IDisposable { - $addProxyKernel(handle: number, data: INotebookProxyKernelDto): Promise; - $updateProxyKernel(handle: number, data: Partial): void; - $removeProxyKernel(handle: number): void; -} - export interface MainThreadNotebookRenderersShape extends IDisposable { $postMessage(editorId: string | undefined, rendererId: string, message: unknown): Promise; } @@ -2129,10 +2123,6 @@ export interface ExtHostNotebookKernelsShape { $cellExecutionChanged(uri: UriComponents, cellHandle: number, state: notebookCommon.NotebookCellExecutionState | undefined): void; } -export interface ExtHostNotebookProxyKernelsShape { - $resolveKernel(handle: number): Promise; -} - export interface ExtHostInteractiveShape { $willAddInteractiveDocument(uri: UriComponents, eol: string, languageId: string, notebookUri: UriComponents): void; $willRemoveInteractiveDocument(uri: UriComponents, notebookUri: UriComponents): void; @@ -2309,7 +2299,6 @@ export const MainContext = { MainThreadNotebookDocuments: createProxyIdentifier('MainThreadNotebookDocumentsShape'), MainThreadNotebookEditors: createProxyIdentifier('MainThreadNotebookEditorsShape'), MainThreadNotebookKernels: createProxyIdentifier('MainThreadNotebookKernels'), - MainThreadNotebookProxyKernels: createProxyIdentifier('MainThreadNotebookProxyKernels'), MainThreadNotebookRenderers: createProxyIdentifier('MainThreadNotebookRenderers'), MainThreadInteractive: createProxyIdentifier('MainThreadInteractive'), MainThreadTheming: createProxyIdentifier('MainThreadTheming'), @@ -2362,7 +2351,6 @@ export const ExtHostContext = { ExtHostNotebookDocuments: createProxyIdentifier('ExtHostNotebookDocuments'), ExtHostNotebookEditors: createProxyIdentifier('ExtHostNotebookEditors'), ExtHostNotebookKernels: createProxyIdentifier('ExtHostNotebookKernels'), - ExtHostNotebookProxyKernels: createProxyIdentifier('ExtHostNotebookProxyKernels'), ExtHostNotebookRenderers: createProxyIdentifier('ExtHostNotebookRenderers'), ExtHostInteractive: createProxyIdentifier('ExtHostInteractive'), ExtHostTheming: createProxyIdentifier('ExtHostTheming'), From 95b8607540917934d215aa6e2dc8e74cf4db8ad3 Mon Sep 17 00:00:00 2001 From: rebornix Date: Tue, 17 May 2022 21:46:44 -0700 Subject: [PATCH 05/13] load source commands --- src/vs/platform/actions/common/actions.ts | 1 + .../editorStatusBar/editorStatusBar.ts | 181 ++++++++++-------- .../browser/notebookKernelServiceImpl.ts | 50 +++++ .../viewParts/notebookKernelActionViewItem.ts | 21 +- .../notebook/common/notebookKernelService.ts | 6 + .../actions/common/menusExtensionPoint.ts | 5 + 6 files changed, 185 insertions(+), 79 deletions(-) diff --git a/src/vs/platform/actions/common/actions.ts b/src/vs/platform/actions/common/actions.ts index e97c241b95c..93c7c897b13 100644 --- a/src/vs/platform/actions/common/actions.ts +++ b/src/vs/platform/actions/common/actions.ts @@ -140,6 +140,7 @@ export class MenuId { static readonly NotebookDiffCellOutputsTitle = new MenuId('NotebookDiffCellOutputsTitle'); static readonly NotebookOutputToolbar = new MenuId('NotebookOutputToolbar'); static readonly NotebookEditorLayoutConfigure = new MenuId('NotebookEditorLayoutConfigure'); + static readonly NotebookKernelSource = new MenuId('NotebookKernelSource'); static readonly BulkEditTitle = new MenuId('BulkEditTitle'); static readonly BulkEditContext = new MenuId('BulkEditContext'); static readonly TimelineItemContext = new MenuId('TimelineItemContext'); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts b/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts index 22233100d5e..a62de5e8e65 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { IAction } from 'vs/base/common/actions'; import { groupBy } from 'vs/base/common/arrays'; import { Disposable, DisposableStore, IDisposable, MutableDisposable } from 'vs/base/common/lifecycle'; import { Schemas } from 'vs/base/common/network'; @@ -157,86 +158,114 @@ registerAction2(class extends Action2 { } } - if (!newKernel) { - type KernelPick = IQuickPickItem & { kernel: INotebookKernel }; - const configButton: IQuickInputButton = { - iconClass: ThemeIcon.asClassName(configureKernelIcon), - tooltip: nls.localize('notebook.promptKernel.setDefaultTooltip', "Set as default for '{0}' notebooks", editor.textModel.viewType) - }; - function toQuickPick(kernel: INotebookKernel) { - const res = { - kernel, - picked: kernel.id === selected?.id, - label: kernel.label, - description: kernel.description, - detail: kernel.detail, - buttons: [configButton] - }; - if (kernel.id === selected?.id) { - if (!res.description) { - res.description = nls.localize('current1', "Currently Selected"); - } else { - res.description = nls.localize('current2', "{0} - Currently Selected", res.description); - } - } - return res; - } - const quickPickItems: QuickPickInput[] = []; - if (all.length) { - // Always display suggested kernels on the top. - if (suggestions.length) { - quickPickItems.push({ - type: 'separator', - label: nls.localize('suggestedKernels', "Suggested") - }); - quickPickItems.push(...suggestions.map(toQuickPick)); - } - - // Next display all of the kernels grouped by categories or extensions. - // If we don't have a kind, always display those at the bottom. - const picks = all.filter(item => !suggestions.includes(item)).map(toQuickPick); - const kernelsPerCategory = groupBy(picks, (a, b) => compareIgnoreCase(a.kernel.kind || 'z', b.kernel.kind || 'z')); - kernelsPerCategory.forEach(items => { - quickPickItems.push({ - type: 'separator', - label: items[0].kernel.kind || nls.localize('otherKernelKinds', "Other") - }); - quickPickItems.push(...items); - }); - } - - if (!all.length) { - // there is no kernel, show the install from marketplace - quickPickItems.push({ - id: 'install', - label: nls.localize('installKernels', "Install kernels from the marketplace"), - }); - } - - const pick = await quickInputService.pick(quickPickItems, { - placeHolder: selected - ? nls.localize('prompt.placeholder.change', "Change kernel for '{0}'", labelService.getUriLabel(notebook.uri, { relative: true })) - : nls.localize('prompt.placeholder.select', "Select kernel for '{0}'", labelService.getUriLabel(notebook.uri, { relative: true })), - onDidTriggerItemButton: (context) => { - if ('kernel' in context.item) { - notebookKernelService.selectKernelForNotebookType(context.item.kernel, notebook.viewType); - } - } - }); - - if (pick) { - if (pick.id === 'install') { - await this._showKernelExtension(paneCompositeService, notebook.viewType); - } else if ('kernel' in pick) { - newKernel = pick.kernel; - } - } - } - if (newKernel) { notebookKernelService.selectKernelForNotebook(newKernel, notebook); return true; } + + type KernelPick = IQuickPickItem & { kernel: INotebookKernel }; + type SourcePick = IQuickPickItem & { action: IAction }; + + const configButton: IQuickInputButton = { + iconClass: ThemeIcon.asClassName(configureKernelIcon), + tooltip: nls.localize('notebook.promptKernel.setDefaultTooltip', "Set as default for '{0}' notebooks", editor.textModel.viewType) + }; + function toQuickPick(kernel: INotebookKernel) { + const res = { + kernel, + picked: kernel.id === selected?.id, + label: kernel.label, + description: kernel.description, + detail: kernel.detail, + buttons: [configButton] + }; + if (kernel.id === selected?.id) { + if (!res.description) { + res.description = nls.localize('current1', "Currently Selected"); + } else { + res.description = nls.localize('current2', "{0} - Currently Selected", res.description); + } + } + return res; + } + const quickPickItems: QuickPickInput[] = []; + if (all.length) { + // Always display suggested kernels on the top. + if (suggestions.length) { + quickPickItems.push({ + type: 'separator', + label: nls.localize('suggestedKernels', "Suggested") + }); + quickPickItems.push(...suggestions.map(toQuickPick)); + } + + // Next display all of the kernels grouped by categories or extensions. + // If we don't have a kind, always display those at the bottom. + const picks = all.filter(item => !suggestions.includes(item)).map(toQuickPick); + const kernelsPerCategory = groupBy(picks, (a, b) => compareIgnoreCase(a.kernel.kind || 'z', b.kernel.kind || 'z')); + kernelsPerCategory.forEach(items => { + quickPickItems.push({ + type: 'separator', + label: items[0].kernel.kind || nls.localize('otherKernelKinds', "Other") + }); + quickPickItems.push(...items); + }); + } + + if (!all.length) { + // there is no kernel, show the install from marketplace + quickPickItems.push({ + id: 'install', + label: nls.localize('installKernels', "Install kernels from the marketplace"), + }); + } + + const sourceActions = notebookKernelService.getSourceActions(); + if (sourceActions.length) { + quickPickItems.push({ + type: 'separator', + // label: nls.localize('sourceActions', "") + }); + + sourceActions.forEach(action => { + const res = { + action, + picked: false, + label: action.label, + }; + + quickPickItems.push(res); + }); + } + + const pick = await quickInputService.pick(quickPickItems, { + placeHolder: selected + ? nls.localize('prompt.placeholder.change', "Change kernel for '{0}'", labelService.getUriLabel(notebook.uri, { relative: true })) + : nls.localize('prompt.placeholder.select', "Select kernel for '{0}'", labelService.getUriLabel(notebook.uri, { relative: true })), + onDidTriggerItemButton: (context) => { + if ('kernel' in context.item) { + notebookKernelService.selectKernelForNotebookType(context.item.kernel, notebook.viewType); + } + } + }); + + if (pick) { + if ('kernel' in pick) { + newKernel = pick.kernel; + notebookKernelService.selectKernelForNotebook(newKernel, notebook); + return true; + } + + // actions + + if (pick.id === 'install') { + await this._showKernelExtension(paneCompositeService, notebook.viewType); + } else if ('action' in pick) { + // selected explicilty, it should trigger the execution? + notebookKernelService.runSourceAction(pick.action); + } + } + return false; } diff --git a/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts index 3a14f27d0ff..6e0151f222c 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts @@ -12,6 +12,9 @@ import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storag import { URI } from 'vs/base/common/uri'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { runWhenIdle } from 'vs/base/common/async'; +import { IMenu, IMenuService, MenuId } from 'vs/platform/actions/common/actions'; +import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; +import { IAction } from 'vs/base/common/actions'; class KernelInfo { @@ -56,18 +59,25 @@ export class NotebookKernelService extends Disposable implements INotebookKernel private readonly _onDidAddKernel = this._register(new Emitter()); private readonly _onDidRemoveKernel = this._register(new Emitter()); private readonly _onDidChangeNotebookAffinity = this._register(new Emitter()); + private readonly _onDidChangeSourceActions = this._register(new Emitter()); + private readonly _sourceMenu: IMenu; + private _sourceActions: IAction[]; readonly onDidChangeSelectedNotebooks: Event = this._onDidChangeNotebookKernelBinding.event; readonly onDidAddKernel: Event = this._onDidAddKernel.event; readonly onDidRemoveKernel: Event = this._onDidRemoveKernel.event; readonly onDidChangeNotebookAffinity: Event = this._onDidChangeNotebookAffinity.event; + readonly onDidChangeSourceActions: Event = this._onDidChangeSourceActions.event; private static _storageNotebookBinding = 'notebook.controller2NotebookBindings'; private static _storageTypeBinding = 'notebook.controller2TypeBindings'; + constructor( @INotebookService private readonly _notebookService: INotebookService, @IStorageService private readonly _storageService: IStorageService, + @IMenuService readonly _menuService: IMenuService, + @IContextKeyService contextKeyService: IContextKeyService ) { super(); @@ -80,6 +90,10 @@ export class NotebookKernelService extends Disposable implements INotebookKernel this._onDidChangeNotebookKernelBinding.fire({ notebook: notebook.uri, oldKernel: kernelId, newKernel: undefined }); } })); + this._sourceMenu = this._register(this._menuService.createMenu(MenuId.NotebookKernelSource, contextKeyService)); + this._sourceActions = []; + + this._initSourceActions(); // restore from storage try { @@ -96,6 +110,24 @@ export class NotebookKernelService extends Disposable implements INotebookKernel } } + private _initSourceActions() { + const loadActions = (menu: IMenu) => { + const groups = menu.getActions({ shouldForwardArgs: true }); + const actions: IAction[] = []; + groups.forEach(group => { + actions.push(...group[1]); + }); + this._sourceActions = actions; + this._onDidChangeSourceActions.fire(); + }; + + this._register(this._sourceMenu.onDidChange(() => { + loadActions(this._sourceMenu); + })); + + loadActions(this._sourceMenu); + } + override dispose() { this._kernels.clear(); super.dispose(); @@ -255,4 +287,22 @@ export class NotebookKernelService extends Disposable implements INotebookKernel } this._onDidChangeNotebookAffinity.fire(); } + + private _runningAction: IAction | undefined = undefined; + + getRunningSourceAction() { + return this._runningAction; + } + + getSourceActions(): IAction[] { + return this._sourceActions; + } + + async runSourceAction(action: IAction): Promise { + this._runningAction = action; + this._onDidChangeSourceActions.fire(); + await action.run(); + this._runningAction = undefined; + this._onDidChangeSourceActions.fire(); + } } diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts index 69917ca972b..4f23f001c3a 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts @@ -9,7 +9,7 @@ import { Action, IAction } from 'vs/base/common/actions'; import { DisposableStore } from 'vs/base/common/lifecycle'; import { localize } from 'vs/nls'; import { ThemeIcon } from 'vs/platform/theme/common/themeService'; -import { selectKernelIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons'; +import { executingStateIcon, selectKernelIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons'; import { INotebookKernel, INotebookKernelMatchResult, INotebookKernelService, NotebookControllerState } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { Event } from 'vs/base/common/event'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; @@ -33,6 +33,7 @@ export class NotebooKernelActionViewItem extends ActionViewItem { this._register(_editor.onDidChangeModel(this._update, this)); this._register(_notebookKernelService.onDidChangeNotebookAffinity(this._update, this)); this._register(_notebookKernelService.onDidChangeSelectedNotebooks(this._update, this)); + this._register(_notebookKernelService.onDidChangeSourceActions(this._update, this)); this._kernelDisposable = this._register(new DisposableStore()); } @@ -61,8 +62,22 @@ export class NotebooKernelActionViewItem extends ActionViewItem { return; } - const info = this._notebookKernelService.getMatchingKernel(notebook); - this._updateActionFromKernelInfo(info); + const runningAction = this._notebookKernelService.getRunningSourceAction(); + if (runningAction) { + this._updateActionFromSourceAction(runningAction); + return; + } else { + this.action.class = ThemeIcon.asClassName(selectKernelIcon); + const info = this._notebookKernelService.getMatchingKernel(notebook); + this._updateActionFromKernelInfo(info); + } + } + + private _updateActionFromSourceAction(sourceAction: IAction) { + this.action.class = ThemeIcon.asClassName(ThemeIcon.modify(executingStateIcon, 'spin')); + this.updateClass(); + this._action.label = sourceAction.label; + this._action.enabled = true; } private _updateActionFromKernelInfo(info: INotebookKernelMatchResult): void { diff --git a/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts b/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts index 13fd479ef1a..e47d930372f 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts @@ -3,6 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { IAction } from 'vs/base/common/actions'; import { Event } from 'vs/base/common/event'; import { IDisposable } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; @@ -82,6 +83,7 @@ export interface INotebookKernelService { readonly onDidRemoveKernel: Event; readonly onDidChangeSelectedNotebooks: Event; readonly onDidChangeNotebookAffinity: Event; + readonly onDidChangeSourceActions: Event; registerKernel(kernel: INotebookKernel): IDisposable; @@ -115,4 +117,8 @@ export interface INotebookKernelService { */ updateKernelNotebookAffinity(kernel: INotebookKernel, notebook: URI, preference: number | undefined): void; + getSourceActions(): IAction[]; + getRunningSourceAction(): IAction | undefined; + + runSourceAction(action: IAction): Promise; } diff --git a/src/vs/workbench/services/actions/common/menusExtensionPoint.ts b/src/vs/workbench/services/actions/common/menusExtensionPoint.ts index 7e06c120f95..409cfb9924c 100644 --- a/src/vs/workbench/services/actions/common/menusExtensionPoint.ts +++ b/src/vs/workbench/services/actions/common/menusExtensionPoint.ts @@ -171,6 +171,11 @@ const apiMenus: IAPIMenu[] = [ id: MenuId.NotebookToolbar, description: localize('notebook.toolbar', "The contributed notebook toolbar menu") }, + { + key: 'notebook/kernelSource', + id: MenuId.NotebookKernelSource, + description: localize('notebook.kernelSource', "The contributed notebook kernel sources menu") + }, { key: 'notebook/cell/title', id: MenuId.NotebookCellTitle, From 2796b3425ddf413a33937b1167cb96cee544904f Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 20 May 2022 16:28:03 -0700 Subject: [PATCH 06/13] Allow single source action to be executed without picking --- .../browser/notebookExecutionServiceImpl.ts | 21 +++++++++++- .../viewParts/notebookKernelActionViewItem.ts | 32 ++++++++++++++----- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts index c92696d34eb..aa57b01e95a 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts @@ -14,7 +14,7 @@ import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/mode import { CellKind, INotebookTextModel, NotebookCellExecutionState } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookExecutionService } from 'vs/workbench/contrib/notebook/common/notebookExecutionService'; import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; -import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernel, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; export class NotebookExecutionService implements INotebookExecutionService, IDisposable { declare _serviceBrand: undefined; @@ -39,6 +39,10 @@ export class NotebookExecutionService implements INotebookExecutionService, IDis } let kernel = this._notebookKernelService.getSelectedOrSuggestedKernel(notebook); + if (!kernel) { + kernel = await this.resolveSourceActions(notebook); + } + if (!kernel) { await this._commandService.executeCommand(SELECT_KERNEL_ID); kernel = this._notebookKernelService.getSelectedOrSuggestedKernel(notebook); @@ -86,6 +90,21 @@ export class NotebookExecutionService implements INotebookExecutionService, IDis } } + private async resolveSourceActions(notebook: INotebookTextModel) { + let kernel: INotebookKernel | undefined; + const info = this._notebookKernelService.getMatchingKernel(notebook); + if (info.all.length === 0) { + // no kernel at all + const sourceActions = this._notebookKernelService.getSourceActions(); + if (sourceActions.length === 1) { + await this._notebookKernelService.runSourceAction(sourceActions[0]); + kernel = this._notebookKernelService.getSelectedOrSuggestedKernel(notebook); + } + } + + return kernel; + } + async cancelNotebookCellHandles(notebook: INotebookTextModel, cells: Iterable): Promise { const cellsArr = Array.from(cells); this._logService.debug(`NotebookExecutionService#cancelNotebookCellHandles ${JSON.stringify(cellsArr)}`); diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts index 4f23f001c3a..060e17ac37a 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts @@ -64,25 +64,41 @@ export class NotebooKernelActionViewItem extends ActionViewItem { const runningAction = this._notebookKernelService.getRunningSourceAction(); if (runningAction) { - this._updateActionFromSourceAction(runningAction); - return; - } else { - this.action.class = ThemeIcon.asClassName(selectKernelIcon); - const info = this._notebookKernelService.getMatchingKernel(notebook); - this._updateActionFromKernelInfo(info); + return this._updateActionFromSourceAction(runningAction, true); } + + const info = this._notebookKernelService.getMatchingKernel(notebook); + if (info.all.length === 0) { + return this._updateActionsFromSourceActions(); + } + + this._updateActionFromKernelInfo(info); } - private _updateActionFromSourceAction(sourceAction: IAction) { - this.action.class = ThemeIcon.asClassName(ThemeIcon.modify(executingStateIcon, 'spin')); + private _updateActionFromSourceAction(sourceAction: IAction, running: boolean) { + this.action.class = running ? ThemeIcon.asClassName(ThemeIcon.modify(executingStateIcon, 'spin')) : ThemeIcon.asClassName(selectKernelIcon); this.updateClass(); this._action.label = sourceAction.label; this._action.enabled = true; } + private _updateActionsFromSourceActions() { + this._action.enabled = true; + const sourceActions = this._notebookKernelService.getSourceActions(); + if (sourceActions.length === 1) { + // exact one action + this._updateActionFromSourceAction(sourceActions[0], false); + } else { + this._action.class = ThemeIcon.asClassName(selectKernelIcon); + this._action.label = localize('select', "Select Kernel"); + this._action.tooltip = ''; + } + } + private _updateActionFromKernelInfo(info: INotebookKernelMatchResult): void { this._kernelDisposable.clear(); this._action.enabled = true; + this._action.class = ThemeIcon.asClassName(selectKernelIcon); const selectedOrSuggested = info.selected ?? (info.suggestions.length === 1 ? info.suggestions[0] : undefined); if (selectedOrSuggested) { // selected or suggested kernel From 8d503ffaf4ffd70a1ea7329a6cced91661858ff3 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 20 May 2022 16:35:46 -0700 Subject: [PATCH 07/13] connection state in another pr --- .../api/browser/mainThreadNotebookKernels.ts | 16 ++-------------- src/vs/workbench/api/common/extHost.api.impl.ts | 1 - src/vs/workbench/api/common/extHost.protocol.ts | 6 ------ .../api/common/extHostNotebookKernels.ts | 9 --------- src/vs/workbench/api/common/extHostTypes.ts | 4 ---- .../contrib/editorStatusBar/editorStatusBar.ts | 4 ++-- .../browser/notebookExecutionServiceImpl.ts | 12 ------------ .../viewParts/notebookKernelActionViewItem.ts | 8 ++------ .../notebook/common/notebookKernelService.ts | 8 -------- .../browser/notebookExecutionService.test.ts | 4 +--- .../notebookExecutionStateService.test.ts | 1 - .../test/browser/notebookKernelService.test.ts | 4 +--- .../actions/common/menusExtensionPoint.ts | 3 ++- .../vscode.proposed.notebookProxyController.d.ts | 9 --------- 14 files changed, 10 insertions(+), 79 deletions(-) diff --git a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts index 38c81e4ba0e..b6ce5aaa0f8 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts @@ -17,14 +17,12 @@ import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/no import { INotebookCellExecution, 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, INotebookKernelDto2, MainContext, MainThreadNotebookKernelsShape, NotebookControllerState } from '../common/extHost.protocol'; +import { ExtHostContext, ExtHostNotebookKernelsShape, ICellExecuteUpdateDto, ICellExecutionCompleteDto, INotebookKernelDto2, MainContext, MainThreadNotebookKernelsShape } from '../common/extHost.protocol'; abstract class MainThreadKernel implements INotebookKernel { private readonly _onDidChange = new Emitter(); private readonly preloads: { uri: URI; provides: string[] }[]; readonly onDidChange: Event = this._onDidChange.event; - private readonly _onDispose = new Emitter(); - readonly onDispose = this._onDispose.event; readonly id: string; readonly viewType: string; readonly extension: ExtensionIdentifier; @@ -34,7 +32,6 @@ abstract class MainThreadKernel implements INotebookKernel { description?: string; detail?: string; kind?: string; - state?: NotebookControllerState; supportedLanguages: string[]; implementsExecutionOrder: boolean; localResourceRoot: URI; @@ -57,7 +54,6 @@ abstract class MainThreadKernel implements INotebookKernel { this.description = data.description; this.detail = data.detail; this.kind = data.kind; - this.state = data.state; this.supportedLanguages = isNonEmptyArray(data.supportedLanguages) ? data.supportedLanguages : _languageService.getRegisteredLanguageIds(); this.implementsExecutionOrder = data.supportsExecutionOrder ?? false; this.localResourceRoot = URI.revive(data.extensionLocation); @@ -84,10 +80,6 @@ abstract class MainThreadKernel implements INotebookKernel { this.kind = data.kind; event.kind = true; } - if (data.state !== undefined) { - this.state = data.state; - event.state = true; - } if (data.supportedLanguages !== undefined) { this.supportedLanguages = isNonEmptyArray(data.supportedLanguages) ? data.supportedLanguages : this._languageService.getRegisteredLanguageIds(); event.supportedLanguages = true; @@ -99,10 +91,6 @@ abstract class MainThreadKernel implements INotebookKernel { this._onDidChange.fire(event); } - dispose() { - this._onDispose.fire(); - } - abstract executeNotebookCellsRequest(uri: URI, cellHandles: number[]): Promise; abstract cancelNotebookCellExecution(uri: URI, cellHandles: number[]): Promise; } @@ -228,7 +216,7 @@ export class MainThreadNotebookKernels implements MainThreadNotebookKernelsShape }); const registration = this._notebookKernelService.registerKernel(kernel); - this._kernels.set(handle, [kernel, combinedDisposable(kernel, listener, registration)]); + this._kernels.set(handle, [kernel, combinedDisposable(listener, registration)]); } $updateKernel(handle: number, data: Partial): void { diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index 5140d9770d5..e4b23860c6e 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -1315,7 +1315,6 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I NotebookCellOutputItem: extHostTypes.NotebookCellOutputItem, NotebookCellStatusBarItem: extHostTypes.NotebookCellStatusBarItem, NotebookControllerAffinity: extHostTypes.NotebookControllerAffinity, - NotebookControllerState: extHostTypes.NotebookControllerState, NotebookEdit: extHostTypes.NotebookEdit, PortAttributes: extHostTypes.PortAttributes, LinkedEditingRanges: extHostTypes.LinkedEditingRanges, diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index d2e87c6e7c6..cff9c82244a 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -970,11 +970,6 @@ export interface MainThreadNotebookDocumentsShape extends IDisposable { $trySaveNotebook(uri: UriComponents): Promise; } -export enum NotebookControllerState { - Idle = 1, - Connecting = 2 -} - export interface INotebookKernelDto2 { id: string; notebookType: string; @@ -984,7 +979,6 @@ export interface INotebookKernelDto2 { detail?: string; description?: string; kind?: string; - state?: NotebookControllerState; supportedLanguages?: string[]; supportsInterrupt?: boolean; supportsExecutionOrder?: boolean; diff --git a/src/vs/workbench/api/common/extHostNotebookKernels.ts b/src/vs/workbench/api/common/extHostNotebookKernels.ts index 4e4ca9640c5..f99ead59ac7 100644 --- a/src/vs/workbench/api/common/extHostNotebookKernels.ts +++ b/src/vs/workbench/api/common/extHostNotebookKernels.ts @@ -196,15 +196,6 @@ export class ExtHostNotebookKernels implements ExtHostNotebookKernelsShape { get rendererScripts() { return data.preloads ? data.preloads.map(extHostTypeConverters.NotebookRendererScript.to) : []; }, - get state() { - checkProposedApiEnabled(extension, 'notebookProxyController'); - return data.state; - }, - set state(value) { - checkProposedApiEnabled(extension, 'notebookProxyController'); - data.state = value; - _update(); - }, get executeHandler() { return _executeHandler; }, diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index fd1f2558140..039f43fce8b 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -3426,10 +3426,6 @@ export class NotebookRendererScript { } } -export enum NotebookControllerState { - Idle = 1, - Connecting = 2 -} //#endregion diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts b/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts index a62de5e8e65..bb1bd41aeb1 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts @@ -29,7 +29,7 @@ import { NotebookEditorWidget } from 'vs/workbench/contrib/notebook/browser/note import { configureKernelIcon, selectKernelIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { NotebookCellsChangeType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { INotebookKernel, INotebookKernelService, NotebookControllerState } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernel, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle'; import { IPaneCompositePartService } from 'vs/workbench/services/panecomposite/browser/panecomposite'; @@ -408,7 +408,7 @@ export class KernelStatus extends Disposable implements IWorkbenchContribution { this._kernelInfoElement.add(this._statusbarService.addEntry( { name: nls.localize('notebook.info', "Notebook Kernel Info"), - text: `$(notebook-kernel-select) ${kernel.label}` + (kernel.state === NotebookControllerState.Idle ? '' : ' Connecting...'), + text: `$(notebook-kernel-select) ${kernel.label}`, ariaLabel: kernel.label, tooltip: isSuggested ? nls.localize('tooltop', "{0} (suggestion)", tooltip) : tooltip, command: SELECT_KERNEL_ID, diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts index aa57b01e95a..75b6b1a55dd 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts @@ -64,18 +64,6 @@ export class NotebookExecutionService implements INotebookExecutionService, IDis executeCells.push(cell); } - if (kernel.state !== undefined) { - // kernel has connection state management - const kernelId = kernel.id; - kernel.onDispose(() => { - // proxy kernel scenario, kernel disposed, we should now make way for new preferred kernel - const exes = executeCells.map(c => this._notebookExecutionStateService.createCellExecution(kernelId, notebook.uri, c.handle)); - exes.forEach(e => e.complete({})); - - this.executeNotebookCells(notebook, executeCells); - }); - } - if (executeCells.length > 0) { this._notebookKernelService.selectKernelForNotebook(kernel, notebook); diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts index 060e17ac37a..8b8f92277e2 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts @@ -10,7 +10,7 @@ import { DisposableStore } from 'vs/base/common/lifecycle'; import { localize } from 'vs/nls'; import { ThemeIcon } from 'vs/platform/theme/common/themeService'; import { executingStateIcon, selectKernelIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons'; -import { INotebookKernel, INotebookKernelMatchResult, INotebookKernelService, NotebookControllerState } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernel, INotebookKernelMatchResult, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { Event } from 'vs/base/common/event'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; @@ -121,11 +121,7 @@ export class NotebooKernelActionViewItem extends ActionViewItem { } private _generateKenrelLabel(kernel: INotebookKernel) { - if (kernel.state === NotebookControllerState.Connecting) { - return localize('kernelconnecting', "Connecting..."); - } else { - return kernel.label; - } + return kernel.label; } private _resetAction(): void { diff --git a/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts b/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts index e47d930372f..0068bd968e1 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts @@ -28,16 +28,10 @@ export interface INotebookKernelChangeEvent { description?: true; detail?: true; kind?: true; - state?: true; supportedLanguages?: true; hasExecutionOrder?: true; } -export enum NotebookControllerState { - Idle = 1, - Connecting = 2 -} - export interface INotebookKernel { readonly id: string; readonly viewType: string; @@ -52,14 +46,12 @@ export interface INotebookKernel { description?: string; detail?: string; kind?: string; - state?: NotebookControllerState; supportedLanguages: string[]; implementsInterrupt?: boolean; implementsExecutionOrder?: boolean; executeNotebookCellsRequest(uri: URI, cellHandles: number[]): Promise; cancelNotebookCellExecution(uri: URI, cellHandles: number[]): Promise; - onDispose: Event; } export const enum ProxyKernelState { diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts index 9ceec332a7a..b0a288f1e8e 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts @@ -20,7 +20,7 @@ import { NotebookViewModel } from 'vs/workbench/contrib/notebook/browser/viewMod import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { CellKind, IOutputDto, NotebookCellMetadata } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; -import { INotebookKernel, INotebookKernelService, ISelectedNotebooksChangeEvent, NotebookControllerState } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernel, INotebookKernelService, ISelectedNotebooksChangeEvent } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { setupInstantiationService, withTestNotebook as _withTestNotebook } from 'vs/workbench/contrib/notebook/test/browser/testNotebookEditor'; @@ -178,7 +178,6 @@ class TestNotebookKernel implements INotebookKernel { preloadUris: URI[] = []; preloadProvides: string[] = []; supportedLanguages: string[] = []; - onDispose = Event.None; executeNotebookCellsRequest(): Promise { throw new Error('Method not implemented.'); } @@ -189,7 +188,6 @@ class TestNotebookKernel implements INotebookKernel { this.supportedLanguages = opts?.languages ?? [PLAINTEXT_LANGUAGE_ID]; } kind?: string | undefined; - state?: NotebookControllerState | undefined; implementsInterrupt?: boolean | undefined; implementsExecutionOrder?: boolean | 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 index 4763ee855c6..887a4a05443 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts @@ -182,7 +182,6 @@ class TestNotebookKernel implements INotebookKernel { preloadUris: URI[] = []; preloadProvides: string[] = []; supportedLanguages: string[] = []; - onDispose = Event.None; async executeNotebookCellsRequest(): Promise { } async cancelNotebookCellExecution(): Promise { } diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookKernelService.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookKernelService.test.ts index 54b3b52cb3c..b1ebabc1db2 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookKernelService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookKernelService.test.ts @@ -8,7 +8,7 @@ import { URI } from 'vs/base/common/uri'; import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions'; import { setupInstantiationService, withTestNotebook as _withTestNotebook } from 'vs/workbench/contrib/notebook/test/browser/testNotebookEditor'; import { Emitter, Event } from 'vs/base/common/event'; -import { INotebookKernel, INotebookKernelService, NotebookControllerState } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernel, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { NotebookKernelService } from 'vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl'; import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; import { mock } from 'vs/base/test/common/mock'; @@ -171,8 +171,6 @@ class TestNotebookKernel implements INotebookKernel { preloadUris: URI[] = []; preloadProvides: string[] = []; supportedLanguages: string[] = []; - state?: NotebookControllerState | undefined = undefined; - onDispose = Event.None; executeNotebookCellsRequest(): Promise { throw new Error('Method not implemented.'); } diff --git a/src/vs/workbench/services/actions/common/menusExtensionPoint.ts b/src/vs/workbench/services/actions/common/menusExtensionPoint.ts index 409cfb9924c..bceef1e438f 100644 --- a/src/vs/workbench/services/actions/common/menusExtensionPoint.ts +++ b/src/vs/workbench/services/actions/common/menusExtensionPoint.ts @@ -174,7 +174,8 @@ const apiMenus: IAPIMenu[] = [ { key: 'notebook/kernelSource', id: MenuId.NotebookKernelSource, - description: localize('notebook.kernelSource', "The contributed notebook kernel sources menu") + description: localize('notebook.kernelSource', "The contributed notebook kernel sources menu"), + proposed: 'notebookProxyController' }, { key: 'notebook/cell/title', diff --git a/src/vscode-dts/vscode.proposed.notebookProxyController.d.ts b/src/vscode-dts/vscode.proposed.notebookProxyController.d.ts index df429123049..a0f2c9e2df8 100644 --- a/src/vscode-dts/vscode.proposed.notebookProxyController.d.ts +++ b/src/vscode-dts/vscode.proposed.notebookProxyController.d.ts @@ -4,13 +4,4 @@ *--------------------------------------------------------------------------------------------*/ declare module 'vscode' { - - export enum NotebookControllerState { - Idle = 1, - Connecting = 2 - } - - export interface NotebookController { - state?: NotebookControllerState; - } } From 50d802e75ffd879157d631e5a579f6e7b44f38e0 Mon Sep 17 00:00:00 2001 From: rebornix Date: Fri, 20 May 2022 16:40:19 -0700 Subject: [PATCH 08/13] :lipstick: --- src/vs/workbench/api/browser/mainThreadNotebookKernels.ts | 1 + src/vs/workbench/api/common/extHostTypes.ts | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts index b6ce5aaa0f8..6b734982787 100644 --- a/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts +++ b/src/vs/workbench/api/browser/mainThreadNotebookKernels.ts @@ -23,6 +23,7 @@ abstract class MainThreadKernel implements INotebookKernel { private readonly _onDidChange = new Emitter(); private readonly preloads: { uri: URI; provides: string[] }[]; readonly onDidChange: Event = this._onDidChange.event; + readonly id: string; readonly viewType: string; readonly extension: ExtensionIdentifier; diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index 039f43fce8b..dc08ede3d98 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -3426,7 +3426,6 @@ export class NotebookRendererScript { } } - //#endregion //#region Timeline From 17f17b429b44b55e158913159fc06279921a23e4 Mon Sep 17 00:00:00 2001 From: rebornix Date: Sun, 22 May 2022 18:58:42 -0700 Subject: [PATCH 09/13] remove remaining kernel state. --- .../browser/viewParts/notebookKernelActionViewItem.ts | 10 ---------- .../services/actions/common/menusExtensionPoint.ts | 2 +- .../extensions/common/extensionsApiProposals.ts | 2 +- ....d.ts => vscode.proposed.notebookKernelSource.d.ts} | 0 4 files changed, 2 insertions(+), 12 deletions(-) rename src/vscode-dts/{vscode.proposed.notebookProxyController.d.ts => vscode.proposed.notebookKernelSource.d.ts} (100%) diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts index 8b8f92277e2..1e7b6980a23 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts @@ -6,7 +6,6 @@ import 'vs/css!./notebookKernelActionViewItem'; import { ActionViewItem } from 'vs/base/browser/ui/actionbar/actionViewItems'; import { Action, IAction } from 'vs/base/common/actions'; -import { DisposableStore } from 'vs/base/common/lifecycle'; import { localize } from 'vs/nls'; import { ThemeIcon } from 'vs/platform/theme/common/themeService'; import { executingStateIcon, selectKernelIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons'; @@ -18,7 +17,6 @@ import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookB export class NotebooKernelActionViewItem extends ActionViewItem { private _kernelLabel?: HTMLAnchorElement; - private _kernelDisposable: DisposableStore; constructor( actualAction: IAction, @@ -34,7 +32,6 @@ export class NotebooKernelActionViewItem extends ActionViewItem { this._register(_notebookKernelService.onDidChangeNotebookAffinity(this._update, this)); this._register(_notebookKernelService.onDidChangeSelectedNotebooks(this._update, this)); this._register(_notebookKernelService.onDidChangeSourceActions(this._update, this)); - this._kernelDisposable = this._register(new DisposableStore()); } override render(container: HTMLElement): void { @@ -96,7 +93,6 @@ export class NotebooKernelActionViewItem extends ActionViewItem { } private _updateActionFromKernelInfo(info: INotebookKernelMatchResult): void { - this._kernelDisposable.clear(); this._action.enabled = true; this._action.class = ThemeIcon.asClassName(selectKernelIcon); const selectedOrSuggested = info.selected ?? (info.suggestions.length === 1 ? info.suggestions[0] : undefined); @@ -107,12 +103,6 @@ export class NotebooKernelActionViewItem extends ActionViewItem { if (!info.selected) { // special UI for selected kernel? } - - this._kernelDisposable.add(selectedOrSuggested.onDidChange(e => { - if (e.state) { - this._action.label = this._generateKenrelLabel(selectedOrSuggested); - } - })); } else { // many kernels or no kernels this._action.label = localize('select', "Select Kernel"); diff --git a/src/vs/workbench/services/actions/common/menusExtensionPoint.ts b/src/vs/workbench/services/actions/common/menusExtensionPoint.ts index bceef1e438f..adf39476671 100644 --- a/src/vs/workbench/services/actions/common/menusExtensionPoint.ts +++ b/src/vs/workbench/services/actions/common/menusExtensionPoint.ts @@ -175,7 +175,7 @@ const apiMenus: IAPIMenu[] = [ key: 'notebook/kernelSource', id: MenuId.NotebookKernelSource, description: localize('notebook.kernelSource', "The contributed notebook kernel sources menu"), - proposed: 'notebookProxyController' + proposed: 'notebookKernelSource' }, { key: 'notebook/cell/title', diff --git a/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts b/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts index c562da5671f..d5ded005d72 100644 --- a/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts +++ b/src/vs/workbench/services/extensions/common/extensionsApiProposals.ts @@ -38,10 +38,10 @@ export const allApiProposals = Object.freeze({ notebookEditor: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookEditor.d.ts', notebookEditorDecorationType: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookEditorDecorationType.d.ts', notebookEditorEdit: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookEditorEdit.d.ts', + notebookKernelSource: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookKernelSource.d.ts', notebookLiveShare: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookLiveShare.d.ts', notebookMessaging: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookMessaging.d.ts', notebookMime: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookMime.d.ts', - notebookProxyController: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookProxyController.d.ts', notebookWorkspaceEdit: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.notebookWorkspaceEdit.d.ts', portsAttributes: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.portsAttributes.d.ts', quickPickSortByLabel: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.quickPickSortByLabel.d.ts', diff --git a/src/vscode-dts/vscode.proposed.notebookProxyController.d.ts b/src/vscode-dts/vscode.proposed.notebookKernelSource.d.ts similarity index 100% rename from src/vscode-dts/vscode.proposed.notebookProxyController.d.ts rename to src/vscode-dts/vscode.proposed.notebookKernelSource.d.ts From cb30029e0208451c0f7cf0801b4ada76521449d8 Mon Sep 17 00:00:00 2001 From: rebornix Date: Sun, 22 May 2022 19:36:01 -0700 Subject: [PATCH 10/13] show install from marketplace only when there is no kernels and commands at all. --- .../contrib/editorStatusBar/editorStatusBar.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts b/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts index bb1bd41aeb1..fab2077edd9 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts @@ -212,14 +212,6 @@ registerAction2(class extends Action2 { }); } - if (!all.length) { - // there is no kernel, show the install from marketplace - quickPickItems.push({ - id: 'install', - label: nls.localize('installKernels', "Install kernels from the marketplace"), - }); - } - const sourceActions = notebookKernelService.getSourceActions(); if (sourceActions.length) { quickPickItems.push({ @@ -238,6 +230,14 @@ registerAction2(class extends Action2 { }); } + if (!all.length && !sourceActions.length) { + // there is no kernel, show the install from marketplace + quickPickItems.push({ + id: 'install', + label: nls.localize('installKernels', "Install kernels from the marketplace"), + }); + } + const pick = await quickInputService.pick(quickPickItems, { placeHolder: selected ? nls.localize('prompt.placeholder.change', "Change kernel for '{0}'", labelService.getUriLabel(notebook.uri, { relative: true })) From 01e4dbd95c7133e5a955d24ab47c5e60b0e252b1 Mon Sep 17 00:00:00 2001 From: rebornix Date: Sun, 22 May 2022 20:01:02 -0700 Subject: [PATCH 11/13] hide explicit IAction. --- .../editorStatusBar/editorStatusBar.ts | 13 ++-- .../browser/notebookExecutionServiceImpl.ts | 2 +- .../browser/notebookKernelServiceImpl.ts | 65 +++++++++++++------ .../viewParts/notebookKernelActionViewItem.ts | 13 ++-- .../notebook/common/notebookKernelService.ts | 18 +++-- 5 files changed, 71 insertions(+), 40 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts b/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts index fab2077edd9..20c8c594f12 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/editorStatusBar/editorStatusBar.ts @@ -3,7 +3,6 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { IAction } from 'vs/base/common/actions'; import { groupBy } from 'vs/base/common/arrays'; import { Disposable, DisposableStore, IDisposable, MutableDisposable } from 'vs/base/common/lifecycle'; import { Schemas } from 'vs/base/common/network'; @@ -29,7 +28,7 @@ import { NotebookEditorWidget } from 'vs/workbench/contrib/notebook/browser/note import { configureKernelIcon, selectKernelIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { NotebookCellsChangeType } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { INotebookKernel, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernel, INotebookKernelService, ISourceAction } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { LifecyclePhase } from 'vs/workbench/services/lifecycle/common/lifecycle'; import { IPaneCompositePartService } from 'vs/workbench/services/panecomposite/browser/panecomposite'; @@ -164,7 +163,7 @@ registerAction2(class extends Action2 { } type KernelPick = IQuickPickItem & { kernel: INotebookKernel }; - type SourcePick = IQuickPickItem & { action: IAction }; + type SourcePick = IQuickPickItem & { action: ISourceAction }; const configButton: IQuickInputButton = { iconClass: ThemeIcon.asClassName(configureKernelIcon), @@ -219,11 +218,11 @@ registerAction2(class extends Action2 { // label: nls.localize('sourceActions', "") }); - sourceActions.forEach(action => { + sourceActions.forEach(sourceAction => { const res = { - action, + action: sourceAction, picked: false, - label: action.label, + label: sourceAction.action.label, }; quickPickItems.push(res); @@ -262,7 +261,7 @@ registerAction2(class extends Action2 { await this._showKernelExtension(paneCompositeService, notebook.viewType); } else if ('action' in pick) { // selected explicilty, it should trigger the execution? - notebookKernelService.runSourceAction(pick.action); + pick.action.runAction(); } } diff --git a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts index 75b6b1a55dd..0be1459d36a 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookExecutionServiceImpl.ts @@ -85,7 +85,7 @@ export class NotebookExecutionService implements INotebookExecutionService, IDis // no kernel at all const sourceActions = this._notebookKernelService.getSourceActions(); if (sourceActions.length === 1) { - await this._notebookKernelService.runSourceAction(sourceActions[0]); + await sourceActions[0].runAction(); kernel = this._notebookKernelService.getSelectedOrSuggestedKernel(notebook); } } diff --git a/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts index 6e0151f222c..8466386bbdb 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts @@ -6,7 +6,7 @@ import { Event, Emitter } from 'vs/base/common/event'; import { Disposable, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; import { INotebookTextModel } from 'vs/workbench/contrib/notebook/common/notebookCommon'; -import { INotebookKernel, ISelectedNotebooksChangeEvent, INotebookKernelMatchResult, INotebookKernelService, INotebookTextModelLike } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernel, ISelectedNotebooksChangeEvent, INotebookKernelMatchResult, INotebookKernelService, INotebookTextModelLike, ISourceAction } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { LRUCache, ResourceMap } from 'vs/base/common/map'; import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage'; import { URI } from 'vs/base/common/uri'; @@ -46,6 +46,34 @@ class NotebookTextModelLikeId { } } +class SourceAction extends Disposable implements ISourceAction { + execution: Promise | undefined; + private readonly _onDidChangeState = this._register(new Emitter()); + readonly onDidChangeState = this._onDidChangeState.event; + + constructor( + readonly action: IAction, + ) { + super(); + } + + async runAction() { + if (this.execution) { + return this.execution; + } + + this.execution = this._runAction(); + this._onDidChangeState.fire(); + await this.execution; + this.execution = undefined; + this._onDidChangeState.fire(); + } + + private async _runAction(): Promise { + await this.action.run(); + } +} + export class NotebookKernelService extends Disposable implements INotebookKernelService { declare _serviceBrand: undefined; @@ -61,7 +89,7 @@ export class NotebookKernelService extends Disposable implements INotebookKernel private readonly _onDidChangeNotebookAffinity = this._register(new Emitter()); private readonly _onDidChangeSourceActions = this._register(new Emitter()); private readonly _sourceMenu: IMenu; - private _sourceActions: IAction[]; + private _sourceActions: [ISourceAction, IDisposable][]; readonly onDidChangeSelectedNotebooks: Event = this._onDidChangeNotebookKernelBinding.event; readonly onDidAddKernel: Event = this._onDidAddKernel.event; @@ -111,25 +139,32 @@ export class NotebookKernelService extends Disposable implements INotebookKernel } private _initSourceActions() { - const loadActions = (menu: IMenu) => { + const loadActionsFromMenu = (menu: IMenu) => { const groups = menu.getActions({ shouldForwardArgs: true }); const actions: IAction[] = []; groups.forEach(group => { actions.push(...group[1]); }); - this._sourceActions = actions; + this._sourceActions = actions.map(action => { + const sourceAction = new SourceAction(action); + const stateChangeListener = sourceAction.onDidChangeState(() => { + this._onDidChangeSourceActions.fire(); + }); + return [sourceAction, stateChangeListener]; + }); this._onDidChangeSourceActions.fire(); }; this._register(this._sourceMenu.onDidChange(() => { - loadActions(this._sourceMenu); + loadActionsFromMenu(this._sourceMenu); })); - loadActions(this._sourceMenu); + loadActionsFromMenu(this._sourceMenu); } override dispose() { this._kernels.clear(); + this._sourceActions.forEach(a => a[1].dispose()); super.dispose(); } @@ -288,21 +323,11 @@ export class NotebookKernelService extends Disposable implements INotebookKernel this._onDidChangeNotebookAffinity.fire(); } - private _runningAction: IAction | undefined = undefined; - - getRunningSourceAction() { - return this._runningAction; + getRunningSourceActions() { + return this._sourceActions.filter(action => action[0].execution).map(action => action[0]); } - getSourceActions(): IAction[] { - return this._sourceActions; - } - - async runSourceAction(action: IAction): Promise { - this._runningAction = action; - this._onDidChangeSourceActions.fire(); - await action.run(); - this._runningAction = undefined; - this._onDidChangeSourceActions.fire(); + getSourceActions(): ISourceAction[] { + return this._sourceActions.map(a => a[0]); } } diff --git a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts index 1e7b6980a23..958a59df428 100644 --- a/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts +++ b/src/vs/workbench/contrib/notebook/browser/viewParts/notebookKernelActionViewItem.ts @@ -9,7 +9,7 @@ import { Action, IAction } from 'vs/base/common/actions'; import { localize } from 'vs/nls'; import { ThemeIcon } from 'vs/platform/theme/common/themeService'; import { executingStateIcon, selectKernelIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons'; -import { INotebookKernel, INotebookKernelMatchResult, INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { INotebookKernel, INotebookKernelMatchResult, INotebookKernelService, ISourceAction } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { Event } from 'vs/base/common/event'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { INotebookEditor } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; @@ -59,9 +59,9 @@ export class NotebooKernelActionViewItem extends ActionViewItem { return; } - const runningAction = this._notebookKernelService.getRunningSourceAction(); - if (runningAction) { - return this._updateActionFromSourceAction(runningAction, true); + const runningActions = this._notebookKernelService.getRunningSourceActions(); + if (runningActions.length) { + return this._updateActionFromSourceAction(runningActions[0] /** TODO handle multiple actions state */, true); } const info = this._notebookKernelService.getMatchingKernel(notebook); @@ -72,10 +72,11 @@ export class NotebooKernelActionViewItem extends ActionViewItem { this._updateActionFromKernelInfo(info); } - private _updateActionFromSourceAction(sourceAction: IAction, running: boolean) { + private _updateActionFromSourceAction(sourceAction: ISourceAction, running: boolean) { + const action = sourceAction.action; this.action.class = running ? ThemeIcon.asClassName(ThemeIcon.modify(executingStateIcon, 'spin')) : ThemeIcon.asClassName(selectKernelIcon); this.updateClass(); - this._action.label = sourceAction.label; + this._action.label = action.label; this._action.enabled = true; } diff --git a/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts b/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts index 0068bd968e1..2ab657faf5b 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookKernelService.ts @@ -64,6 +64,13 @@ export interface INotebookProxyKernelChangeEvent extends INotebookKernelChangeEv connectionState?: true; } +export interface ISourceAction { + readonly action: IAction; + readonly onDidChangeState: Event; + execution: Promise | undefined; + runAction: () => Promise; +} + export interface INotebookTextModelLike { uri: URI; viewType: string } export const INotebookKernelService = createDecorator('INotebookKernelService'); @@ -75,8 +82,6 @@ export interface INotebookKernelService { readonly onDidRemoveKernel: Event; readonly onDidChangeSelectedNotebooks: Event; readonly onDidChangeNotebookAffinity: Event; - readonly onDidChangeSourceActions: Event; - registerKernel(kernel: INotebookKernel): IDisposable; getMatchingKernel(notebook: INotebookTextModelLike): INotebookKernelMatchResult; @@ -109,8 +114,9 @@ export interface INotebookKernelService { */ updateKernelNotebookAffinity(kernel: INotebookKernel, notebook: URI, preference: number | undefined): void; - getSourceActions(): IAction[]; - getRunningSourceAction(): IAction | undefined; - - runSourceAction(action: IAction): Promise; + //#region Kernel source actions + readonly onDidChangeSourceActions: Event; + getSourceActions(): ISourceAction[]; + getRunningSourceActions(): ISourceAction[]; + //#endregion } From 11d269da26315e9469fa86b77ae18a6f3c70c3c2 Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 23 May 2022 17:27:21 -0700 Subject: [PATCH 12/13] update tests --- .../notebook/browser/notebookKernelServiceImpl.ts | 4 ++-- .../browser/notebookExecutionStateService.test.ts | 11 +++++++++++ 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts b/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts index 8466386bbdb..ab92e2777bd 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookKernelServiceImpl.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Event, Emitter } from 'vs/base/common/event'; -import { Disposable, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; +import { Disposable, dispose, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; import { INotebookTextModel } from 'vs/workbench/contrib/notebook/common/notebookCommon'; import { INotebookKernel, ISelectedNotebooksChangeEvent, INotebookKernelMatchResult, INotebookKernelService, INotebookTextModelLike, ISourceAction } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; import { LRUCache, ResourceMap } from 'vs/base/common/map'; @@ -164,7 +164,7 @@ export class NotebookKernelService extends Disposable implements INotebookKernel override dispose() { this._kernels.clear(); - this._sourceActions.forEach(a => a[1].dispose()); + dispose(this._sourceActions.map(a => a[1])); super.dispose(); } 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 887a4a05443..0b05037e51e 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionStateService.test.ts @@ -10,6 +10,7 @@ import { DisposableStore } from 'vs/base/common/lifecycle'; import { URI } from 'vs/base/common/uri'; import { mock } from 'vs/base/test/common/mock'; import { PLAINTEXT_LANGUAGE_ID } from 'vs/editor/common/languages/modesRegistry'; +import { IMenu, IMenuService } from 'vs/platform/actions/common/actions'; 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'; @@ -47,6 +48,16 @@ suite('NotebookExecutionStateService', () => { } }); + instantiationService.stub(IMenuService, new class extends mock() { + override createMenu() { + return new class extends mock() { + override onDidChange = Event.None; + override getActions() { return []; } + override dispose() { } + }; + } + }); + kernelService = instantiationService.createInstance(NotebookKernelService); instantiationService.set(INotebookKernelService, kernelService); instantiationService.set(INotebookExecutionService, instantiationService.createInstance(NotebookExecutionService)); From 568d036b38b5a8925854f71726de4831d23177b8 Mon Sep 17 00:00:00 2001 From: rebornix Date: Mon, 23 May 2022 17:50:03 -0700 Subject: [PATCH 13/13] shim for menu service in more tests. --- .../test/browser/notebookExecutionService.test.ts | 11 +++++++++++ .../test/browser/notebookKernelService.test.ts | 10 ++++++++++ 2 files changed, 21 insertions(+) diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts index b0a288f1e8e..e249f73080a 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookExecutionService.test.ts @@ -11,6 +11,7 @@ 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/languages/modesRegistry'; +import { IMenu, IMenuService } from 'vs/platform/actions/common/actions'; 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'; @@ -42,6 +43,16 @@ suite('NotebookExecutionService', () => { override getNotebookTextModels() { return []; } }); + instantiationService.stub(IMenuService, new class extends mock() { + override createMenu() { + return new class extends mock() { + override onDidChange = Event.None; + override getActions() { return []; } + override dispose() { } + }; + } + }); + kernelService = instantiationService.createInstance(NotebookKernelService); instantiationService.set(INotebookKernelService, kernelService); diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookKernelService.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookKernelService.test.ts index b1ebabc1db2..1b03a84c623 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookKernelService.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookKernelService.test.ts @@ -16,6 +16,7 @@ import { TestInstantiationService } from 'vs/platform/instantiation/test/common/ import { DisposableStore } from 'vs/base/common/lifecycle'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { PLAINTEXT_LANGUAGE_ID } from 'vs/editor/common/languages/modesRegistry'; +import { IMenu, IMenuService } from 'vs/platform/actions/common/actions'; suite('NotebookKernelService', () => { @@ -37,6 +38,15 @@ suite('NotebookKernelService', () => { override onWillRemoveNotebookDocument = Event.None; override getNotebookTextModels() { return []; } }); + instantiationService.stub(IMenuService, new class extends mock() { + override createMenu() { + return new class extends mock() { + override onDidChange = Event.None; + override getActions() { return []; } + override dispose() { } + }; + } + }); kernelService = instantiationService.createInstance(NotebookKernelService); instantiationService.set(INotebookKernelService, kernelService); });