refactor: openVariablePanels command and split out a PanelController

package.json

@@ -136,6 +136,10 @@
         "title": "Deephaven: Open in Browser",
         "icon": "$(globe)"
       },
+      {
+        "command": "vscode-deephaven.openVariablePanels",
+        "title": "Deephaven: Open Variable Panels"
+      },
       {
         "command": "vscode-deephaven.refreshServerTree",
         "title": "Deephaven: Refresh Server Tree",
@@ -187,6 +191,10 @@
           "command": "vscode-deephaven.disconnectFromServer",
           "when": "false"
         },
+        {
+          "command": "vscode-deephaven.openVariablePanels",
+          "when": "false"
+        },
         {
           "command": "vscode-deephaven.refreshServerTree",
           "when": "false"

src/common/commands.ts

@@ -6,6 +6,7 @@ export const DISCONNECT_EDITOR_CMD = `${EXTENSION_ID}.disconnectEditor`;
 export const DISCONNECT_FROM_SERVER_CMD = `${EXTENSION_ID}.disconnectFromServer`;
 export const DOWNLOAD_LOGS_CMD = `${EXTENSION_ID}.downloadLogs`;
 export const OPEN_IN_BROWSER_CMD = `${EXTENSION_ID}.openInBrowser`;
+export const OPEN_VARIABLE_PANELS_CMD = `${EXTENSION_ID}.openVariablePanels`;
 export const REFRESH_SERVER_TREE_CMD = `${EXTENSION_ID}.refreshServerTree`;
 export const REFRESH_SERVER_CONNECTION_TREE_CMD = `${EXTENSION_ID}.refreshServerConnectionTree`;
 export const RUN_CODE_COMMAND = `${EXTENSION_ID}.runCode`;

src/common/constants.ts

@@ -1,5 +1,5 @@
 import * as path from 'node:path';
-import type { ConsoleType, Port } from '../types';
+import type { ConsoleType, Port, VariableType } from '../types';
 
 export const EXTENSION_ID = 'vscode-deephaven' as const;
 
@@ -63,6 +63,15 @@ export const ICON_ID = {
   serverStopped: 'circle-slash',
 } as const;
 
+/* eslint-disable @typescript-eslint/naming-convention */
+export const VARIABLE_UNICODE_ICONS = {
+  'deephaven.plot.express.DeephavenFigure': '📈',
+  'deephaven.ui.Element': '✨',
+  Figure: '📈',
+  Table: '⬜',
+} as const satisfies Record<VariableType, string>;
+/* eslint-enable @typescript-eslint/naming-convention */
+
 export const CONNECTION_TREE_ITEM_CONTEXT = {
   isConnection: 'isConnection',
   isUri: 'isUri',

src/controllers/ExtensionController.ts

@@ -7,6 +7,7 @@ import {
   DISCONNECT_FROM_SERVER_CMD,
   DOWNLOAD_LOGS_CMD,
   OPEN_IN_BROWSER_CMD,
+  OPEN_VARIABLE_PANELS_CMD,
   REFRESH_SERVER_CONNECTION_TREE_CMD,
   REFRESH_SERVER_TREE_CMD,
   RUN_CODE_COMMAND,
@@ -43,10 +44,12 @@ import type {
   ServerConnectionTreeView,
   ServerState,
   ServerTreeView,
+  VariableDefintion,
 } from '../types';
 import { ServerConnectionTreeDragAndDropController } from './ServerConnectionTreeDragAndDropController';
 import { ConnectionController } from './ConnectionController';
 import { PipServerController } from './PipServerController';
+import { PanelController } from './PanelController';
 
 const logger = new Logger('ExtensionController');
 
@@ -63,6 +66,7 @@ export class ExtensionController implements Disposable {
     this.initializeServerManager();
     this.initializeTempDirectory();
     this.initializeConnectionController();
+    this.initializePanelController();
     this.initializePipServerController();
     this.initializeCommands();
     this.initializeWebViews();
@@ -80,6 +84,7 @@ export class ExtensionController implements Disposable {
   readonly _config: IConfigService;
 
   private _connectionController: ConnectionController | null = null;
+  private _panelController: PanelController | null = null;
   private _panelService: IPanelService | null = null;
   private _pipServerController: PipServerController | null = null;
   private _dhcServiceFactory: IDhServiceFactory | null = null;
@@ -140,6 +145,21 @@ export class ExtensionController implements Disposable {
     this._context.subscriptions.push(this._connectionController);
   };
 
+  /**
+   * Initialize panel controller.
+   */
+  initializePanelController = (): void => {
+    assertDefined(this._panelService, 'panelService');
+    assertDefined(this._serverManager, 'serverManager');
+
+    this._panelController = new PanelController(
+      this._serverManager,
+      this._panelService
+    );
+
+    this._context.subscriptions.push(this._panelController);
+  };
+
   /**
    * Initialize pip server controller.
    */
@@ -219,7 +239,6 @@ export class ExtensionController implements Disposable {
     this._context.subscriptions.push(this._panelService);
 
     this._dhcServiceFactory = new DhcServiceFactory(
-      this._panelService,
       this._pythonDiagnostics,
       this._outputChannel,
       this._toaster
@@ -295,6 +314,9 @@ export class ExtensionController implements Disposable {
     /** Open server in browser */
     this.registerCommand(OPEN_IN_BROWSER_CMD, this.onOpenInBrowser);
 
+    /** Open variable panel */
+    this.registerCommand(OPEN_VARIABLE_PANELS_CMD, this.onOpenVariablePanels);
+
     /** Run all code in active editor */
     this.registerCommand(RUN_CODE_COMMAND, this.onRunCode);
 
@@ -468,6 +490,15 @@ export class ExtensionController implements Disposable {
     );
   };
 
+  /**
+   * Open panels for given url and variables.
+   * @param url Connection url to open panels for.
+   * @param variables Variables to open panels for.
+   */
+  onOpenVariablePanels = (url: URL, variables: VariableDefintion[]): void => {
+    this._panelController?.openPanels(url, variables);
+  };
+
   onRefreshServerStatus = async (): Promise<void> => {
     await this._pipServerController?.syncManagedServers();
     await this._serverManager?.updateStatus();

src/controllers/PanelController.ts

@@ -0,0 +1,87 @@
+import * as vscode from 'vscode';
+import type {
+  Disposable,
+  IPanelService,
+  IServerManager,
+  VariableDefintion,
+} from '../types';
+import { getEmbedWidgetUrl } from '../dh/dhc';
+import { assertDefined, getDHThemeKey, getPanelHtml } from '../util';
+import { DhcService } from '../services';
+
+export class PanelController implements Disposable {
+  constructor(serverManager: IServerManager, panelService: IPanelService) {
+    this._panelService = panelService;
+    this._serverManager = serverManager;
+  }
+
+  private readonly _panelService: IPanelService;
+  private readonly _serverManager: IServerManager;
+
+  dispose = async (): Promise<void> => {};
+
+  openPanels = async (
+    serverUrl: URL,
+    variables: VariableDefintion[]
+  ): Promise<void> => {
+    let lastPanel: vscode.WebviewPanel | null = null;
+
+    for (const { id, title } of variables) {
+      if (!this._panelService.hasPanel(serverUrl, id)) {
+        const panel = vscode.window.createWebviewPanel(
+          'dhPanel', // Identifies the type of the webview. Used internally
+          title,
+          { viewColumn: vscode.ViewColumn.Two, preserveFocus: true },
+          {
+            enableScripts: true,
+            retainContextWhenHidden: true,
+          }
+        );
+
+        this._panelService.setPanel(serverUrl, id, panel);
+
+        // If panel gets disposed, remove it from the caches
+        panel.onDidDispose(() => {
+          this._panelService.deletePanel(serverUrl, id);
+        });
+
+        // See @deprecated comment in PanelFocusManager.onDidChangeViewState
+        // Ensure focus is not stolen when panel is loaded
+        // panel.onDidChangeViewState(
+        //   this.panelFocusManager.handleOnDidChangeViewState(panel)
+        // );
+      }
+
+      const panel = this._panelService.getPanelOrThrow(serverUrl, id);
+      lastPanel = panel;
+
+      // See @deprecated comment in PanelFocusManager.onDidChangeViewState
+      // Ensure focus is not stolen when panel is loaded
+      // this.panelFocusManager.initialize(panel);
+
+      const connection = this._serverManager.getConnection(serverUrl);
+      assertDefined(connection, 'connection');
+
+      const iframeUrl = getEmbedWidgetUrl(
+        serverUrl,
+        title,
+        getDHThemeKey(),
+        connection instanceof DhcService ? connection.getPsk() : undefined
+      );
+
+      panel.webview.html = getPanelHtml(iframeUrl, title);
+
+      // TODO: The postMessage apis will be needed for auth in DHE (vscode-deephaven/issues/76).
+      // Leaving this here commented out for reference, but it will need some
+      // re-working. Namely this seems to subscribe multiple times. Should see
+      // if can move it inside of the panel creation block or unsubscribe older
+      // subscriptions whenever we subscribe.
+      // panel.webview.onDidReceiveMessage(({ data }) => {
+      //   const postMessage = panel.webview.postMessage.bind(panel.webview);
+      //   this.handlePanelMessage(data, postMessage);
+      // });
+    }
+
+    lastPanel?.reveal();
+  };
+}

src/controllers/index.ts

@@ -1,4 +1,5 @@
 export * from './ConnectionController';
 export * from './ExtensionController';
+export * from './PanelController';
 export * from './PipServerController';
 export * from './ServerConnectionTreeDragAndDropController';

src/services/DhService.ts

@@ -5,43 +5,31 @@ import type {
   ConnectionAndSession,
   ConsoleType,
   IDhService,
-  IPanelService,
   IToastService,
+  VariableDefintion,
 } from '../types';
 import {
-  assertIsVariableID,
   formatTimestamp,
   getCombinedSelectedLinesText,
   isAggregateError,
   Logger,
   NoConsoleTypesError,
   parseServerError,
 } from '../util';
+import { OPEN_VARIABLE_PANELS_CMD, VARIABLE_UNICODE_ICONS } from '../common';
 
 const logger = new Logger('DhService');
 
-/* eslint-disable @typescript-eslint/naming-convention */
-const icons = {
-  Figure: '📈',
-  'deephaven.plot.express.DeephavenFigure': '📈',
-  Table: '⬜',
-  'deephaven.ui.Element': '✨',
-} as const;
-type IconType = keyof typeof icons;
-/* eslint-enable @typescript-eslint/naming-convention */
-
 export abstract class DhService<TDH = unknown, TClient = unknown>
   implements IDhService<TDH, TClient>
 {
   constructor(
     serverUrl: URL,
-    panelService: IPanelService,
     diagnosticsCollection: vscode.DiagnosticCollection,
     outputChannel: vscode.OutputChannel,
     toaster: IToastService
   ) {
     this.serverUrl = serverUrl;
-    this.panelService = panelService;
     this.diagnosticsCollection = diagnosticsCollection;
     this.outputChannel = outputChannel;
     this.toaster = toaster;
@@ -55,7 +43,6 @@ export abstract class DhService<TDH = unknown, TClient = unknown>
 
   protected readonly outputChannel: vscode.OutputChannel;
   protected readonly toaster: IToastService;
-  private readonly panelService: IPanelService;
   private readonly diagnosticsCollection: vscode.DiagnosticCollection;
   private cachedCreateClient: Promise<TClient> | null = null;
   private cachedCreateSession: Promise<
@@ -74,14 +61,6 @@ export abstract class DhService<TDH = unknown, TClient = unknown>
     dh: TDH,
     client: TClient
   ): Promise<ConnectionAndSession<DhcType.IdeConnection, DhcType.IdeSession>>;
-  protected abstract getPanelHtml(title: string): string;
-  protected abstract handlePanelMessage(
-    message: {
-      id: string;
-      message: string;
-    },
-    postResponseMessage: (response: unknown) => void
-  ): Promise<void>;
 
   private clearCaches(): void {
     this.cachedCreateClient = null;
@@ -103,6 +82,7 @@ export abstract class DhService<TDH = unknown, TClient = unknown>
 
   public async dispose(): Promise<void> {
     this.clearCaches();
+    this._onDidDisconnect.dispose();
   }
 
   protected getToastErrorMessage(
@@ -303,69 +283,23 @@ export abstract class DhService<TDH = unknown, TClient = unknown>
       return;
     }
 
-    const changed = [...result!.changes.created, ...result!.changes.updated];
-
-    // Have to set this with type assertion since TypeScript can't figure out
-    // assignments inside of the `forEach` and will treat `lastPanel` as `null`.
-    let lastPanel = null as vscode.WebviewPanel | null;
-
-    changed.forEach(({ id, title = 'Unknown', type }) => {
-      assertIsVariableID(id, 'id');
+    const changed = [
+      ...result!.changes.created,
+      ...result!.changes.updated,
+    ] as VariableDefintion[];
 
-      const icon = icons[type as IconType] ?? type;
+    changed.forEach(({ title = 'Unknown', type }) => {
+      const icon = VARIABLE_UNICODE_ICONS[type] ?? type;
       this.outputChannel.appendLine(`${icon} ${title}`);
-
-      // Don't show panels for variables starting with '_'
-      if (title.startsWith('_')) {
-        return;
-      }
-
-      if (!this.panelService.hasPanel(this.serverUrl, id)) {
-        const panel = vscode.window.createWebviewPanel(
-          'dhPanel', // Identifies the type of the webview. Used internally
-          title,
-          { viewColumn: vscode.ViewColumn.Two, preserveFocus: true },
-          {
-            enableScripts: true,
-            retainContextWhenHidden: true,
-          }
-        );
-
-        this.panelService.setPanel(this.serverUrl, id, panel);
-
-        // If panel gets disposed, remove it from the caches
-        panel.onDidDispose(() => {
-          this.panelService.deletePanel(this.serverUrl, id);
-        });
-
-        // See @deprecated comment in PanelFocusManager.onDidChangeViewState
-        // Ensure focus is not stolen when panel is loaded
-        // panel.onDidChangeViewState(
-        //   this.panelFocusManager.handleOnDidChangeViewState(panel)
-        // );
-      }
-
-      const panel = this.panelService.getPanelOrThrow(this.serverUrl, id);
-      lastPanel = panel;
-
-      // See @deprecated comment in PanelFocusManager.onDidChangeViewState
-      // Ensure focus is not stolen when panel is loaded
-      // this.panelFocusManager.initialize(panel);
-
-      panel.webview.html = this.getPanelHtml(title);
-
-      // TODO: The postMessage apis will be needed for auth in DHE (vscode-deephaven/issues/76).
-      // Leaving this here commented out for reference, but it will need some
-      // re-working. Namely this seems to subscribe multiple times. Should see
-      // if can move it inside of the panel creation block or unsubscribe older
-      // subscriptions whenever we subscribe.
-      // panel.webview.onDidReceiveMessage(({ data }) => {
-      //   const postMessage = panel.webview.postMessage.bind(panel.webview);
-      //   this.handlePanelMessage(data, postMessage);
-      // });
     });
 
-    lastPanel?.reveal();
+    const showVariables = changed.filter(v => !v.title.startsWith('_'));
+
+    vscode.commands.executeCommand(
+      OPEN_VARIABLE_PANELS_CMD,
+      this.serverUrl,
+      showVariables
+    );
   }
 
   getConsoleTypes = async (): Promise<Set<ConsoleType>> => {