From 9da0bcb7bb3ec15597aaa4459442fdf5d807afd1 Mon Sep 17 00:00:00 2001 From: amunger Date: Wed, 2 Oct 2024 15:33:11 -0700 Subject: [PATCH 1/2] enable variables for REPL despite setting --- .../notebookVariables/notebookVariables.ts | 13 +++---------- .../notebookVariables/notebookVariablesView.ts | 18 +++++++++++------- 2 files changed, 14 insertions(+), 17 deletions(-) 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 3280eccce3ac7..004dad8ea515e 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariables.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariables.ts @@ -50,20 +50,13 @@ export class NotebookVariables extends Disposable implements IWorkbenchContribut 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(); - } + this.handleInitEvent(); } } private handleInitEvent(notebook?: URI) { - if (this.configurationService.getValue(NotebookSetting.notebookVariablesView) - && (!!notebook || this.editorService.activeEditorPane?.getId() === 'workbench.editor.notebook')) { - + const enabled = this.editorService.activeEditorPane?.getId() === 'workbench.editor.repl' || this.configurationService.getValue(NotebookSetting.notebookVariablesView); + if (enabled && (!!notebook || this.editorService.activeEditorPane?.getId() === 'workbench.editor.notebook')) { if (this.hasVariableProvider(notebook) && !this.initialized && this.initializeView()) { this.viewEnabled.set(true); this.initialized = true; diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesView.ts b/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesView.ts index dede87d9eeb8e..08870f67629b0 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesView.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesView.ts @@ -71,11 +71,11 @@ export class NotebookVariablesView extends ViewPane { ) { super(options, keybindingService, contextMenuService, configurationService, contextKeyService, viewDescriptorService, instantiationService, openerService, themeService, telemetryService, hoverService); - this._register(this.editorService.onDidActiveEditorChange(this.handleActiveEditorChange.bind(this))); + this._register(this.editorService.onDidActiveEditorChange(() => this.handleActiveEditorChange())); this._register(this.notebookKernelService.onDidNotebookVariablesUpdate(this.handleVariablesChanged.bind(this))); this._register(this.notebookExecutionStateService.onDidChangeExecution(this.handleExecutionStateChange.bind(this))); - this.activeNotebook = this.getActiveNotebook()?.notebookDocument; + this.handleActiveEditorChange(false); this.dataSource = new NotebookVariableDataSource(this.notebookKernelService); this.updateScheduler = new RunOnceScheduler(() => this.tree?.updateChildren(), 100); @@ -143,15 +143,19 @@ export class NotebookVariablesView extends ViewPane { this.tree?.layout(height, width); } - private setActiveNotebook(notebookDocument: NotebookTextModel, editor: IEditorPane) { + private setActiveNotebook(notebookDocument: NotebookTextModel, editor: IEditorPane, doUpdate = true) { this.activeNotebook = notebookDocument; - this.tree?.setInput({ kind: 'root', notebook: notebookDocument }); - this.updateScheduler.schedule(); + if (isCompositeNotebookEditorInput(editor.input)) { this.updateTitle(NotebookVariablesView.REPL_TITLE.value); } else { this.updateTitle(NotebookVariablesView.NOTEBOOK_TITLE.value); } + + if (doUpdate) { + this.tree?.setInput({ kind: 'root', notebook: notebookDocument }); + this.updateScheduler.schedule(); + } } private getActiveNotebook() { @@ -160,10 +164,10 @@ export class NotebookVariablesView extends ViewPane { return notebookDocument && notebookEditor ? { notebookDocument, notebookEditor } : undefined; } - private handleActiveEditorChange() { + private handleActiveEditorChange(doUpdate = true) { const found = this.getActiveNotebook(); if (found && found.notebookDocument !== this.activeNotebook) { - this.setActiveNotebook(found.notebookDocument, found.notebookEditor); + this.setActiveNotebook(found.notebookDocument, found.notebookEditor, doUpdate); } } From bf680433061ee70ca17f24b58e77d7b5ece7783f Mon Sep 17 00:00:00 2001 From: amunger Date: Wed, 2 Oct 2024 15:41:45 -0700 Subject: [PATCH 2/2] handle closing notebook with variables --- .../notebookVariablesDataSource.ts | 10 ++++++++-- .../notebookVariables/notebookVariablesView.ts | 16 ++++++++++++---- 2 files changed, 20 insertions(+), 6 deletions(-) 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 d0af4bc80f9a9..f3474e3035b96 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesDataSource.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesDataSource.ts @@ -9,6 +9,10 @@ import { localize } from '../../../../../../nls.js'; import { NotebookTextModel } from '../../../common/model/notebookTextModel.js'; import { INotebookKernel, INotebookKernelService, VariablesResult, variablePageSize } from '../../../common/notebookKernelService.js'; +export interface IEmptyScope { + kind: 'empty'; +} + export interface INotebookScope { kind: 'root'; readonly notebook: NotebookTextModel; @@ -49,8 +53,10 @@ export class NotebookVariableDataSource implements IAsyncDataSource> { - if (element.kind === 'root') { + async getChildren(element: INotebookScope | INotebookVariableElement | IEmptyScope): Promise> { + if (element.kind === 'empty') { + return []; + } else if (element.kind === 'root') { return this.getRootVariables(element.notebook); } else { return this.getVariables(element); diff --git a/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesView.ts b/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesView.ts index 08870f67629b0..3cc57762731f0 100644 --- a/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesView.ts +++ b/src/vs/workbench/contrib/notebook/browser/contrib/notebookVariables/notebookVariablesView.ts @@ -26,14 +26,14 @@ import { IThemeService } from '../../../../../../platform/theme/common/themeServ import { IViewPaneOptions, ViewPane } from '../../../../../browser/parts/views/viewPane.js'; import { IViewDescriptorService } from '../../../../../common/views.js'; import { CONTEXT_VARIABLE_EXTENSIONID, CONTEXT_VARIABLE_INTERFACES, CONTEXT_VARIABLE_LANGUAGE, CONTEXT_VARIABLE_NAME, CONTEXT_VARIABLE_TYPE, CONTEXT_VARIABLE_VALUE } from '../../../../debug/common/debug.js'; -import { INotebookScope, INotebookVariableElement, NotebookVariableDataSource } from './notebookVariablesDataSource.js'; +import { IEmptyScope, INotebookScope, INotebookVariableElement, NotebookVariableDataSource } from './notebookVariablesDataSource.js'; import { NotebookVariableAccessibilityProvider, NotebookVariableRenderer, NotebookVariablesDelegate } from './notebookVariablesTree.js'; import { getNotebookEditorFromEditorPane } from '../../notebookBrowser.js'; import { NotebookTextModel } from '../../../common/model/notebookTextModel.js'; import { ICellExecutionStateChangedEvent, IExecutionStateChangedEvent, INotebookExecutionStateService } from '../../../common/notebookExecutionStateService.js'; import { INotebookKernelService } from '../../../common/notebookKernelService.js'; import { IEditorService } from '../../../../../services/editor/common/editorService.js'; -import { IEditorPane } from '../../../../../common/editor.js'; +import { IEditorCloseEvent, IEditorPane } from '../../../../../common/editor.js'; import { isCompositeNotebookEditorInput } from '../../../common/notebookEditorInput.js'; export type contextMenuArg = { source: string; name: string; type?: string; value?: string; expression?: string; language?: string; extensionId?: string }; @@ -44,7 +44,7 @@ export class NotebookVariablesView extends ViewPane { static readonly NOTEBOOK_TITLE: ILocalizedString = nls.localize2('notebook.notebookVariables', "Notebook Variables"); static readonly REPL_TITLE: ILocalizedString = nls.localize2('notebook.ReplVariables', "REPL Variables"); - private tree: WorkbenchAsyncDataTree | undefined; + private tree: WorkbenchAsyncDataTree | undefined; private activeNotebook: NotebookTextModel | undefined; private readonly dataSource: NotebookVariableDataSource; @@ -74,6 +74,7 @@ export class NotebookVariablesView extends ViewPane { this._register(this.editorService.onDidActiveEditorChange(() => this.handleActiveEditorChange())); this._register(this.notebookKernelService.onDidNotebookVariablesUpdate(this.handleVariablesChanged.bind(this))); this._register(this.notebookExecutionStateService.onDidChangeExecution(this.handleExecutionStateChange.bind(this))); + this._register(this.editorService.onDidCloseEditor((e) => this.handleCloseEditor(e))); this.handleActiveEditorChange(false); @@ -85,7 +86,7 @@ export class NotebookVariablesView extends ViewPane { super.renderBody(container); this.element.classList.add('debug-pane'); - this.tree = >this.instantiationService.createInstance( + this.tree = >this.instantiationService.createInstance( WorkbenchAsyncDataTree, 'notebookVariablesTree', container, @@ -164,6 +165,13 @@ export class NotebookVariablesView extends ViewPane { return notebookDocument && notebookEditor ? { notebookDocument, notebookEditor } : undefined; } + private handleCloseEditor(e: IEditorCloseEvent) { + if (e.editor.resource && e.editor.resource.toString() === this.activeNotebook?.uri.toString()) { + this.tree?.setInput({ kind: 'empty' }); + this.updateScheduler.schedule(); + } + } + private handleActiveEditorChange(doUpdate = true) { const found = this.getActiveNotebook(); if (found && found.notebookDocument !== this.activeNotebook) {