Rob's comments and update variable view when done debugging

src/client/datascience/jupyter/debuggerVariables.ts

@@ -54,6 +54,7 @@ export class DebuggerVariables extends DebugLocationTracker
         @inject(IVSCodeNotebook) private readonly vscNotebook: IVSCodeNotebook
     ) {
         super(undefined);
+        this.debuggingManager.onDoneDebugging(() => this.refreshEventEmitter.fire(), this);
     }
 
     public get refreshRequired(): Event<void> {
@@ -236,32 +237,8 @@ export class DebuggerVariables extends DebugLocationTracker
         // When the initialize response comes back, indicate we have started.
         if (message.type === 'response' && message.command === 'initialize') {
             this.debuggingStarted = true;
-        } else if (
-            (message as DebugProtocol.StackTraceResponse).command === 'stackTrace' &&
-            this.activeNotebookIsDebugging()
-        ) {
-            const sf = (message as DebugProtocol.StackTraceResponse).body.stackFrames[0];
-            const callScopes = async () => {
-                const doc = this.vscNotebook.activeNotebookEditor?.document;
-                if (doc) {
-                    const session = await this.debuggingManager.getDebugSession(doc);
-                    const mode = this.debuggingManager.getDebugMode(doc);
-                    if (mode === KernelDebugMode.RunByLine) {
-                        const cell = this.debuggingManager.getDebugCell(doc);
-                        // Only call scopes (and variables) if we are stopped on the cell we are executing
-                        if (sf.source && cell && sf.source.path === cell.document.uri.toString()) {
-                            void session?.customRequest('scopes', { frameId: sf.id });
-                        }
-                    } else {
-                        // Only call scopes (and variables) if we are stopped on the notebook we are executing
-                        const docURI = path.basename(doc.uri.toString());
-                        if (sf.source && sf.source.path && sf.source.path.includes(docURI)) {
-                            void session?.customRequest('scopes', { frameId: sf.id });
-                        }
-                    }
-                }
-            };
-            void callScopes();
+        } else if (message.type === 'event' && message.event === 'stopped' && this.activeNotebookIsDebugging()) {
+            void this.handleNotebookVariables(message as DebugProtocol.StoppedEvent);
         } else if (message.type === 'response' && message.command === 'scopes' && message.body && message.body.scopes) {
             const response = message as DebugProtocol.ScopesResponse;
 
@@ -271,16 +248,6 @@ export class DebuggerVariables extends DebugLocationTracker
                 this.currentVariablesReference = newVariablesReference;
                 this.currentSeqNumsForVariables.clear();
             }
-
-            // Catch the scopes response and use its variablesReference to send a variables message
-            const doc = this.vscNotebook.activeNotebookEditor?.document;
-            if (this.activeNotebookIsDebugging() && doc) {
-                const callVariables = async () => {
-                    const session = await this.debuggingManager.getDebugSession(doc);
-                    void session?.customRequest('variables', { variablesReference: newVariablesReference });
-                };
-                void callVariables();
-            }
         } else if (
             message.type === 'response' &&
             message.command === 'variables' &&
@@ -300,10 +267,6 @@ export class DebuggerVariables extends DebugLocationTracker
 
             this.updateVariables(undefined, message as DebugProtocol.VariablesResponse);
             this.monkeyPatchDataViewableVariables(message);
-
-            if (this.activeNotebookIsDebugging()) {
-                this.refreshEventEmitter.fire();
-            }
         } else if (message.type === 'event' && message.event === 'terminated') {
             // When the debugger exits, make sure the variables are cleared
             this.lastKnownVariables = [];
@@ -452,6 +415,65 @@ export class DebuggerVariables extends DebugLocationTracker
         const activeNotebook = this.vscNotebook.activeNotebookEditor;
         return !!activeNotebook && this.debuggingManager.isDebugging(activeNotebook.document);
     }
+
+    // This handles all the debug session calls, variable handling, and refresh calls needed for notebook debugging
+    private async handleNotebookVariables(stoppedMessage: DebugProtocol.StoppedEvent): Promise<void> {
+        const doc = this.vscNotebook.activeNotebookEditor?.document;
+        const threadId = stoppedMessage.body.threadId;
+
+        if (doc) {
+            const session = await this.debuggingManager.getDebugSession(doc);
+            if (session) {
+                // Call stack trace
+                const stResponse = await session.customRequest('stackTrace', {
+                    threadId,
+                    startFrame: 0,
+                    levels: 1
+                });
+
+                //  Call scopes
+                const sf = stResponse.stackFrames[0];
+                const mode = this.debuggingManager.getDebugMode(doc);
+                // eslint-disable-next-line @typescript-eslint/no-explicit-any
+                let scopesResponse: any | undefined;
+
+                if (mode === KernelDebugMode.RunByLine) {
+                    // Only call scopes (and variables) if we are stopped on the cell we are executing
+                    const cell = this.debuggingManager.getDebugCell(doc);
+                    if (sf.source && cell && sf.source.path === cell.document.uri.toString()) {
+                        scopesResponse = await session.customRequest('scopes', { frameId: sf.id });
+                    }
+                } else {
+                    // Only call scopes (and variables) if we are stopped on the notebook we are executing
+                    const docURI = path.basename(doc.uri.toString());
+                    if (sf.source && sf.source.path && sf.source.path.includes(docURI)) {
+                        scopesResponse = await session.customRequest('scopes', { frameId: sf.id });
+                    }
+                }
+
+                // Call variables
+                if (scopesResponse) {
+                    // Keep track of variablesReference because "hover" requests also try to update variables
+                    const newVariablesReference = scopesResponse.scopes[0].variablesReference;
+                    if (newVariablesReference !== this.currentVariablesReference) {
+                        this.currentVariablesReference = newVariablesReference;
+                        this.currentSeqNumsForVariables.clear();
+                    }
+
+                    // Catch the scopes response and use its variablesReference to send a variables message
+                    const varResponse: DebugProtocol.VariablesResponse = await session.customRequest('variables', {
+                        variablesReference: newVariablesReference
+                    });
+
+                    // Refresh variable view
+                    this.updateVariables(undefined, varResponse as DebugProtocol.VariablesResponse);
+                    this.monkeyPatchDataViewableVariables(varResponse);
+
+                    this.refreshEventEmitter.fire();
+                }
+            }
+        }
+    }
 }
 
 export function convertDebugProtocolVariableToIJupyterVariable(variable: DebugProtocol.Variable) {

src/client/debugger/jupyter/debugControllers.ts

@@ -13,9 +13,9 @@ import { Commands } from '../../datascience/constants';
 import { IKernel } from '../../datascience/jupyter/kernels/types';
 import { sendTelemetryEvent } from '../../telemetry';
 import { DebuggingTelemetry } from '../constants';
-import { DebuggingDelegate, IKernelDebugAdapter, KernelDebugMode } from '../types';
+import { IDebuggingDelegate, IKernelDebugAdapter, KernelDebugMode } from '../types';
 
-export class DebugCellController implements DebuggingDelegate {
+export class DebugCellController implements IDebuggingDelegate {
     constructor(
         private readonly debugAdapter: IKernelDebugAdapter,
         public readonly debugCell: NotebookCell,
@@ -25,11 +25,11 @@ export class DebugCellController implements DebuggingDelegate {
         sendTelemetryEvent(DebuggingTelemetry.successfullyStartedRunAndDebugCell);
     }
 
-    public async willSendEvent(_msg: DebugProtocolMessage): Promise<boolean> {
+    public async onWillSendEvent(_msg: DebugProtocolMessage): Promise<boolean> {
         return false;
     }
 
-    public async willSendRequest(request: DebugProtocol.Request): Promise<void> {
+    public async onWillSendRequest(request: DebugProtocol.Request): Promise<void> {
         if (request.command === 'configurationDone') {
             await cellDebugSetup(this.kernel, this.debugAdapter, this.debugCell);
 
@@ -41,7 +41,7 @@ export class DebugCellController implements DebuggingDelegate {
     }
 }
 
-export class RunByLineController implements DebuggingDelegate {
+export class RunByLineController implements IDebuggingDelegate {
     private lastPausedThreadId: number | undefined;
 
     constructor(
@@ -72,7 +72,7 @@ export class RunByLineController implements DebuggingDelegate {
         return config.__mode;
     }
 
-    public async willSendEvent(msg: DebugProtocolMessage): Promise<boolean> {
+    public async onWillSendEvent(msg: DebugProtocolMessage): Promise<boolean> {
         // eslint-disable-next-line @typescript-eslint/no-explicit-any
         const anyMsg = msg as any;
 
@@ -87,7 +87,7 @@ export class RunByLineController implements DebuggingDelegate {
         return false;
     }
 
-    public async willSendRequest(request: DebugProtocol.Request): Promise<void> {
+    public async onWillSendRequest(request: DebugProtocol.Request): Promise<void> {
         if (request.command === 'configurationDone') {
             await this.initializeExecute();
         }

src/client/debugger/jupyter/debugger.ts

@@ -0,0 +1,41 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+'use strict';
+import { debug, NotebookDocument, DebugSession, DebugSessionOptions, DebugConfiguration } from 'vscode';
+
+export class Debugger {
+    private resolveFunc?: (value: DebugSession) => void;
+    private rejectFunc?: (reason?: Error) => void;
+
+    readonly session: Promise<DebugSession>;
+
+    constructor(
+        public readonly document: NotebookDocument,
+        public readonly config: DebugConfiguration,
+        options?: DebugSessionOptions
+    ) {
+        this.session = new Promise<DebugSession>((resolve, reject) => {
+            this.resolveFunc = resolve;
+            this.rejectFunc = reject;
+
+            debug.startDebugging(undefined, config, options).then(undefined, reject);
+        });
+    }
+
+    resolve(session: DebugSession) {
+        if (this.resolveFunc) {
+            this.resolveFunc(session);
+        }
+    }
+
+    reject(reason: Error) {
+        if (this.rejectFunc) {
+            this.rejectFunc(reason);
+        }
+    }
+
+    async stop() {
+        void debug.stopDebugging(await this.session);
+    }
+}

src/client/debugger/jupyter/debuggingManager.ts

@@ -12,9 +12,10 @@ import {
     DebugSession,
     NotebookCell,
     DebugSessionOptions,
-    DebugConfiguration,
     ProgressLocation,
-    DebugAdapterDescriptor
+    DebugAdapterDescriptor,
+    Event,
+    EventEmitter
 } from 'vscode';
 import * as path from 'path';
 import { IKernel, IKernelProvider } from '../../datascience/jupyter/kernels/types';
@@ -38,42 +39,7 @@ import { sendTelemetryEvent } from '../../telemetry';
 import { PythonEnvironment } from '../../pythonEnvironments/info';
 import { DebugCellController, RunByLineController } from './debugControllers';
 import { assertIsDebugConfig } from './helper';
-
-class Debugger {
-    private resolveFunc?: (value: DebugSession) => void;
-    private rejectFunc?: (reason?: Error) => void;
-
-    readonly session: Promise<DebugSession>;
-
-    constructor(
-        public readonly document: NotebookDocument,
-        public readonly config: DebugConfiguration,
-        options?: DebugSessionOptions
-    ) {
-        this.session = new Promise<DebugSession>((resolve, reject) => {
-            this.resolveFunc = resolve;
-            this.rejectFunc = reject;
-
-            debug.startDebugging(undefined, config, options).then(undefined, reject);
-        });
-    }
-
-    resolve(session: DebugSession) {
-        if (this.resolveFunc) {
-            this.resolveFunc(session);
-        }
-    }
-
-    reject(reason: Error) {
-        if (this.rejectFunc) {
-            this.rejectFunc(reason);
-        }
-    }
-
-    async stop() {
-        void debug.stopDebugging(await this.session);
-    }
-}
+import { Debugger } from './debugger';
 
 /**
  * The DebuggingManager maintains the mapping between notebook documents and debug sessions.
@@ -86,6 +52,7 @@ export class DebuggingManager implements IExtensionSingleActivationService, IDeb
     private notebookToRunByLineController = new Map<NotebookDocument, RunByLineController>();
     private cache = new Map<PythonEnvironment, boolean>();
     private readonly disposables: IDisposable[] = [];
+    private _doneDebugging = new EventEmitter<void>();
 
     public constructor(
         @inject(IKernelProvider) private kernelProvider: IKernelProvider,
@@ -223,6 +190,10 @@ export class DebuggingManager implements IExtensionSingleActivationService, IDeb
         );
     }
 
+    public get onDoneDebugging(): Event<void> {
+        return this._doneDebugging.event;
+    }
+
     public dispose() {
         this.disposables.forEach((d) => d.dispose());
     }
@@ -312,6 +283,7 @@ export class DebuggingManager implements IExtensionSingleActivationService, IDeb
     private async endSession(session: DebugSession) {
         void this.updateToolbar(false);
         void this.updateCellToolbar(false);
+        this._doneDebugging.fire();
         for (const [doc, dbg] of this.notebookToDebugger.entries()) {
             if (dbg && session.id === (await dbg.session).id) {
                 this.notebookToDebugger.delete(doc);

src/client/debugger/jupyter/kernelDebugAdapter.ts

@@ -29,9 +29,9 @@ import { IJupyterSession } from '../../datascience/types';
 import { sendTelemetryEvent } from '../../telemetry';
 import { DebuggingTelemetry } from '../constants';
 import {
-    DebuggingDelegate,
-    debugInfoResponse,
-    dumpCellResponse,
+    IDebuggingDelegate,
+    IDebugInfoResponse,
+    IDumpCellResponse,
     IKernelDebugAdapter,
     IKernelDebugAdapterConfig,
     KernelDebugMode
@@ -48,7 +48,7 @@ export class KernelDebugAdapter implements DebugAdapter, IKernelDebugAdapter, ID
     private readonly endSession = new EventEmitter<DebugSession>();
     private readonly configuration: IKernelDebugAdapterConfig;
     private readonly disposables: IDisposable[] = [];
-    private delegate: DebuggingDelegate | undefined;
+    private delegate: IDebuggingDelegate | undefined;
     onDidSendMessage: Event<DebugProtocolMessage> = this.sendMessage.event;
     onDidEndSession: Event<DebugSession> = this.endSession.event;
     public readonly debugCellUri: Uri | undefined;
@@ -77,7 +77,7 @@ export class KernelDebugAdapter implements DebugAdapter, IKernelDebugAdapter, ID
 
                 if (anyMsg.header.msg_type === 'debug_event') {
                     this.trace('event', JSON.stringify(msg));
-                    if (!(await this.delegate?.willSendEvent(anyMsg))) {
+                    if (!(await this.delegate?.onWillSendEvent(anyMsg))) {
                         this.sendMessage.fire(msg.content);
                     }
                 }
@@ -124,7 +124,7 @@ export class KernelDebugAdapter implements DebugAdapter, IKernelDebugAdapter, ID
         );
     }
 
-    public setDebuggingDelegate(delegate: DebuggingDelegate) {
+    public setDebuggingDelegate(delegate: IDebuggingDelegate) {
         this.delegate = delegate;
     }
 
@@ -155,7 +155,7 @@ export class KernelDebugAdapter implements DebugAdapter, IKernelDebugAdapter, ID
         }
 
         if (message.type === 'request') {
-            await this.delegate?.willSendRequest(message as DebugProtocol.Request);
+            await this.delegate?.onWillSendRequest(message as DebugProtocol.Request);
         }
 
         this.sendRequestToJupyterSession(message);
@@ -211,7 +211,7 @@ export class KernelDebugAdapter implements DebugAdapter, IKernelDebugAdapter, ID
         if (cell) {
             try {
                 const response = await this.session.customRequest('dumpCell', { code: cell.document.getText() });
-                const norm = path.normalize((response as dumpCellResponse).sourcePath);
+                const norm = path.normalize((response as IDumpCellResponse).sourcePath);
                 this.fileToCell.set(norm, cell);
                 this.cellToFile.set(cell.document.uri.toString(), norm);
             } catch (err) {
@@ -224,7 +224,7 @@ export class KernelDebugAdapter implements DebugAdapter, IKernelDebugAdapter, ID
         const response = await this.session.customRequest('debugInfo');
 
         // If there's stopped threads at this point, continue them all
-        (response as debugInfoResponse).stoppedThreads.forEach((thread: number) => {
+        (response as IDebugInfoResponse).stoppedThreads.forEach((thread: number) => {
             this.jupyterSession.requestDebug({
                 seq: 0,
                 type: 'request',