From 8fa84589eef3538dbc763ff98dc7d5a8a0c56374 Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Thu, 8 Feb 2024 18:57:55 -0800 Subject: [PATCH] show variable view only when applicable (#204789) * only initialize view if there is a variable provider available * use context key to show/hide the view --- .../notebookVariableContextKeys.ts | 8 +++ .../notebookVariables/notebookVariables.ts | 63 +++++++++++++++---- .../notebookVariablesDataSource.ts | 6 +- 3 files changed, 63 insertions(+), 14 deletions(-) create mode 100644 src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariableContextKeys.ts diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariableContextKeys.ts b/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariableContextKeys.ts new file mode 100644 index 0000000000000..b90769ef43cf5 --- /dev/null +++ b/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariableContextKeys.ts @@ -0,0 +1,8 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { RawContextKey } from 'vs/platform/contextkey/common/contextkey'; + +export const NOTEBOOK_VARIABLE_VIEW_ENABLED = new RawContextKey('notebookVariableViewEnabled', false); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariables.ts b/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariables.ts index 962f144cc7061..0da62de50b4c8 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariables.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariables.ts @@ -9,40 +9,75 @@ import { IWorkbenchContribution } from 'vs/workbench/common/contributions'; import { Registry } from 'vs/platform/registry/common/platform'; import { Extensions, IViewContainersRegistry, IViewsRegistry } from 'vs/workbench/common/views'; import { VIEWLET_ID as debugContainerId } from 'vs/workbench/contrib/debug/common/debug'; -import { ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey'; import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors'; import { NotebookVariablesView } from 'vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesView'; -import { NOTEBOOK_KERNEL } from 'vs/workbench/contrib/notebook/common/notebookContextKeys'; import { variablesViewIcon } from 'vs/workbench/contrib/notebook/browser/notebookIcons'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; -import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { IConfigurationChangeEvent, IConfigurationService } from 'vs/platform/configuration/common/configuration'; import { INotebookExecutionStateService } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService'; import { NotebookSetting } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { getNotebookEditorFromEditorPane } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; +import { INotebookKernelService } from 'vs/workbench/contrib/notebook/common/notebookKernelService'; +import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; +import { NOTEBOOK_VARIABLE_VIEW_ENABLED } from 'vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariableContextKeys'; export class NotebookVariables extends Disposable implements IWorkbenchContribution { private listeners: IDisposable[] = []; + private configListener: IDisposable; + private initialized = false; + + private viewEnabled: IContextKey; constructor( + @IContextKeyService contextKeyService: IContextKeyService, + @IConfigurationService private readonly configurationService: IConfigurationService, @IEditorService private readonly editorService: IEditorService, - @IConfigurationService configurationService: IConfigurationService, - @INotebookExecutionStateService private readonly notebookExecutionStateService: INotebookExecutionStateService + @INotebookExecutionStateService private readonly notebookExecutionStateService: INotebookExecutionStateService, + @INotebookKernelService private readonly notebookKernelService: INotebookKernelService ) { super(); - this.listeners.push(this.editorService.onDidEditorsChange(() => this.handleInitEvent(configurationService))); - this.listeners.push(this.notebookExecutionStateService.onDidChangeExecution(() => this.handleInitEvent(configurationService))); + this.viewEnabled = NOTEBOOK_VARIABLE_VIEW_ENABLED.bindTo(contextKeyService); + + this.listeners.push(this.editorService.onDidEditorsChange(() => this.handleInitEvent())); + this.listeners.push(this.notebookExecutionStateService.onDidChangeExecution(() => this.handleInitEvent())); + + this.configListener = configurationService.onDidChangeConfiguration((e) => this.handleConfigChange(e)); + } + + private handleConfigChange(e: IConfigurationChangeEvent) { + if (e.affectsConfiguration(NotebookSetting.notebookVariablesView)) { + if (!this.configurationService.getValue(NotebookSetting.notebookVariablesView)) { + this.viewEnabled.set(false); + } else if (this.initialized) { + this.viewEnabled.set(true); + } else { + this.handleInitEvent(); + } + } } - private handleInitEvent(configurationService: IConfigurationService) { - if (configurationService.getValue(NotebookSetting.notebookVariablesView) + private handleInitEvent() { + if (this.configurationService.getValue(NotebookSetting.notebookVariablesView) && this.editorService.activeEditorPane?.getId() === 'workbench.editor.notebook') { - if (this.initializeView()) { + + if (this.hasVariableProvider()) { + this.viewEnabled.set(true); + } + + if (!this.initialized && this.initializeView()) { + this.initialized = true; this.listeners.forEach(listener => listener.dispose()); } } } + private hasVariableProvider() { + const notebookDocument = getNotebookEditorFromEditorPane(this.editorService.activeEditorPane)?.getViewModel()?.notebookDocument; + return notebookDocument && this.notebookKernelService.getMatchingKernel(notebookDocument).selected?.hasVariableProvider; + } + private initializeView() { const debugViewContainer = Registry.as('workbench.registry.view.containers').get(debugContainerId); @@ -51,7 +86,7 @@ export class NotebookVariables extends Disposable implements IWorkbenchContribut const viewDescriptor = { id: 'NOTEBOOK_VARIABLES', name: nls.localize2('notebookVariables', "Notebook Variables"), containerIcon: variablesViewIcon, ctorDescriptor: new SyncDescriptor(NotebookVariablesView), - order: 50, weight: 5, canToggleVisibility: true, canMoveView: true, collapsed: true, when: ContextKeyExpr.notEquals(NOTEBOOK_KERNEL.key, ''), + order: 50, weight: 5, canToggleVisibility: true, canMoveView: true, collapsed: true, when: NOTEBOOK_VARIABLE_VIEW_ENABLED, }; viewsRegistry.registerViews([viewDescriptor], debugViewContainer); @@ -61,4 +96,10 @@ export class NotebookVariables extends Disposable implements IWorkbenchContribut return false; } + override dispose(): void { + super.dispose(); + this.listeners.forEach(listener => listener.dispose()); + this.configListener.dispose(); + } + } diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesDataSource.ts b/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesDataSource.ts index 035e5b0ee7f9e..2ca03e7798e59 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesDataSource.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesDataSource.ts @@ -53,7 +53,7 @@ export class NotebookVariableDataSource implements IAsyncDataSource { + private async getVariables(parent: INotebookVariableElement): Promise { const selectedKernel = this.notebookKernelService.getMatchingKernel(parent.notebook).selected; if (selectedKernel && selectedKernel.hasVariableProvider) { @@ -75,7 +75,7 @@ export class NotebookVariableDataSource implements IAsyncDataSource variablePageSize) { @@ -128,7 +128,7 @@ export class NotebookVariableDataSource implements IAsyncDataSource { + private async getRootVariables(notebook: NotebookTextModel): Promise { const selectedKernel = this.notebookKernelService.getMatchingKernel(notebook).selected; if (selectedKernel && selectedKernel.hasVariableProvider) { const variables = selectedKernel.provideVariables(notebook.uri, undefined, 'named', 0, this.cancellationTokenSource.token);