From 828c97d5843aef0532e55537ad49db93285aec55 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 3 Jun 2022 01:39:08 +1000 Subject: [PATCH] Fixes to IW debugging with breakpoints --- .../debugger/jupyter/debugCellControllers.ts | 13 +- .../debugger/jupyter/kernelDebugAdapter.ts | 121 +++++++++++++++++- .../debugger/kernelDebugAdapterBase.ts | 66 +++------- src/kernels/debugger/types.ts | 4 +- src/notebooks/debugger/helper.ts | 4 +- src/notebooks/debugger/kernelDebugAdapter.ts | 21 ++- 6 files changed, 167 insertions(+), 62 deletions(-) diff --git a/src/interactive-window/debugger/jupyter/debugCellControllers.ts b/src/interactive-window/debugger/jupyter/debugCellControllers.ts index 86278fb255d7..c8482b130d6c 100644 --- a/src/interactive-window/debugger/jupyter/debugCellControllers.ts +++ b/src/interactive-window/debugger/jupyter/debugCellControllers.ts @@ -26,17 +26,20 @@ export class DebugCellController implements IDebuggingDelegate { public async willSendEvent(_msg: DebugProtocolMessage): Promise { return false; } + private debugCellDumped?: Promise; public async willSendRequest(request: DebugProtocol.Request): Promise { const metadata = getInteractiveCellMetadata(this.debugCell); if (request.command === 'setBreakpoints' && metadata && metadata.generatedCode && !this.cellDumpInvoked) { - this.cellDumpInvoked = true; - await cellDebugSetup(this.kernel, this.debugAdapter); + if (!this.debugCellDumped) { + this.debugCellDumped = cellDebugSetup(this.kernel, this.debugAdapter); + } + await this.debugCellDumped; } if (request.command === 'configurationDone' && metadata && metadata.generatedCode) { - if (!this.cellDumpInvoked) { - this.cellDumpInvoked = true; - await cellDebugSetup(this.kernel, this.debugAdapter); + if (!this.debugCellDumped) { + this.debugCellDumped = cellDebugSetup(this.kernel, this.debugAdapter); } + await this.debugCellDumped; this._ready.resolve(); } } diff --git a/src/interactive-window/debugger/jupyter/kernelDebugAdapter.ts b/src/interactive-window/debugger/jupyter/kernelDebugAdapter.ts index 84be8d727ca8..302103a54fe0 100644 --- a/src/interactive-window/debugger/jupyter/kernelDebugAdapter.ts +++ b/src/interactive-window/debugger/jupyter/kernelDebugAdapter.ts @@ -10,12 +10,20 @@ import { DebugProtocol } from 'vscode-debugprotocol'; import { IJupyterSession, IKernel } from '../../../kernels/types'; import { IPlatformService } from '../../../platform/common/platform/types'; import { IDumpCellResponse, IDebugLocationTrackerFactory } from '../../../kernels/debugger/types'; -import { traceError, traceInfoIfCI } from '../../../platform/logging'; +import { traceError, traceInfo, traceInfoIfCI } from '../../../platform/logging'; import { getInteractiveCellMetadata } from '../../../interactive-window/helpers'; import { KernelDebugAdapterBase } from '../../../kernels/debugger/kernelDebugAdapterBase'; +import { InteractiveCellMetadata } from '../../editor-integration/types'; export class KernelDebugAdapter extends KernelDebugAdapterBase { private readonly debugLocationTracker?: DebugAdapterTracker; + private readonly cellToDebugFileSortedInReverseOrderByLineNumber: { + debugFilePath: string; + interactiveWindow: Uri; + lineOffset: number; + metadata: InteractiveCellMetadata; + }[] = []; + constructor( session: DebugSession, notebookDocument: NotebookDocument, @@ -82,14 +90,121 @@ export class KernelDebugAdapter extends KernelDebugAdapterBase { metadata.interactive.lineIndex + (metadata.generatedCode?.lineOffsetRelativeToIndexOfFirstLineInCell || 0) }); - this.cellToFile.set(Uri.parse(metadata.interactive.uristring), { - path: norm, + this.cellToDebugFileSortedInReverseOrderByLineNumber.push({ + debugFilePath: norm, + interactiveWindow: Uri.parse(metadata.interactive.uristring), + metadata, lineOffset: metadata.interactive.lineIndex + (metadata.generatedCode?.lineOffsetRelativeToIndexOfFirstLineInCell || 0) }); + // Order cells in reverse order. + this.cellToDebugFileSortedInReverseOrderByLineNumber.sort( + (a, b) => b.metadata.interactive.lineIndex - a.metadata.interactive.lineIndex + ); } catch (err) { traceError(`Failed to dump cell for ${cell.index} with code ${metadata.interactive.originalSource}`, err); } } + protected override translateDebuggerFileToRealFile( + source: DebugProtocol.Source | undefined, + lines?: { line?: number; endLine?: number; lines?: number[] } + ) { + if (!source || !source.path || !lines || (typeof lines.line !== 'number' && !Array.isArray(lines.lines))) { + return; + } + // Find the cell that matches this line in the IW file. + const cell = this.cellToDebugFileSortedInReverseOrderByLineNumber.find( + (item) => item.debugFilePath === source.path + ); + if (!cell) { + return; + } + source.name = path.basename(cell.interactiveWindow.path); + source.path = cell.interactiveWindow.toString(); + if (typeof lines?.endLine === 'number') { + lines.endLine = lines.endLine + (cell.lineOffset || 0); + } + if (typeof lines?.line === 'number') { + lines.line = lines.line + (cell.lineOffset || 0); + } + if (lines?.lines && Array.isArray(lines?.lines)) { + lines.lines = lines?.lines.map((line) => line + (cell.lineOffset || 0)); + } + } + protected override translateRealFileToDebuggerFile( + source: DebugProtocol.Source | undefined, + lines?: { line?: number; endLine?: number; lines?: number[] } + ) { + if (!source || !source.path || !lines || (typeof lines.line !== 'number' && !Array.isArray(lines.lines))) { + return; + } + const startLine = lines.line || lines.lines![0]; + // Find the cell that matches this line in the IW file. + const cell = this.cellToDebugFileSortedInReverseOrderByLineNumber.find( + (item) => startLine >= item.metadata.interactive.lineIndex + 1 + ); + if (!cell) { + return; + } + source.path = cell.debugFilePath; + if (typeof lines?.endLine === 'number') { + lines.endLine = lines.endLine - (cell.lineOffset || 0); + } + if (typeof lines?.line === 'number') { + lines.line = lines.line - (cell.lineOffset || 0); + } + if (lines?.lines && Array.isArray(lines?.lines)) { + lines.lines = lines?.lines.map((line) => line - (cell.lineOffset || 0)); + } + } + + protected override async sendRequestToJupyterSession(message: DebugProtocol.ProtocolMessage) { + if (this.jupyterSession.disposed || this.jupyterSession.status === 'dead') { + traceInfo(`Skipping sending message ${message.type} because session is disposed`); + return; + } + + const request = message as unknown as DebugProtocol.SetBreakpointsRequest; + if (request.type === 'request' && request.command === 'setBreakpoints') { + const sortedLines = (request.arguments.lines || []).concat( + (request.arguments.breakpoints || []).map((bp) => bp.line) + ); + const startLine = sortedLines.length ? sortedLines[0] : undefined; + // Find the cell that matches this line in the IW file. + const cell = startLine + ? this.cellToDebugFileSortedInReverseOrderByLineNumber.find( + (item) => startLine >= item.metadata.interactive.lineIndex + 1 + ) + : undefined; + if (cell) { + const clonedRequest: typeof request = JSON.parse(JSON.stringify(request)); + if (request.arguments.lines) { + request.arguments.lines = request.arguments.lines.filter( + (line) => line <= cell.metadata.generatedCode!.endLine + ); + } + if (request.arguments.breakpoints) { + request.arguments.breakpoints = request.arguments.breakpoints.filter( + (bp) => bp.line <= cell.metadata.generatedCode!.endLine + ); + } + if (sortedLines.filter((line) => line > cell.metadata.generatedCode!.endLine).length) { + // Find all the lines that don't belong to this cell & add breakpoints for those as well + // However do that separately as they belong to different files. + await this.setBreakpoints({ + source: clonedRequest.arguments.source, + breakpoints: clonedRequest.arguments.breakpoints?.filter( + (bp) => bp.line > cell.metadata.generatedCode!.endLine + ), + lines: clonedRequest.arguments.lines?.filter( + (line) => line > cell.metadata.generatedCode!.endLine + ) + }); + } + } + } + + return super.sendRequestToJupyterSession(message); + } } diff --git a/src/kernels/debugger/kernelDebugAdapterBase.ts b/src/kernels/debugger/kernelDebugAdapterBase.ts index e07cb305dda8..1fc374144721 100644 --- a/src/kernels/debugger/kernelDebugAdapterBase.ts +++ b/src/kernels/debugger/kernelDebugAdapterBase.ts @@ -41,7 +41,6 @@ import { shortNameMatchesLongName, getMessageSourceAndHookIt } from '../../notebooks/debugger/helper'; -import { ResourceMap } from '../../platform/vscode-path/map'; import { IDisposable } from '../../platform/common/types'; /** @@ -57,10 +56,6 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb lineOffset?: number; } >(); - protected readonly cellToFile = new ResourceMap<{ - path: string; - lineOffset?: number; - }>(); private readonly sendMessage = new EventEmitter(); private readonly endSession = new EventEmitter(); private readonly configuration: IKernelDebugAdapterConfig; @@ -233,9 +228,6 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb ); } protected abstract dumpCell(index: number): Promise; - public getSourcePath(filePath: Uri) { - return this.cellToFile.get(filePath)?.path; - } private async debugInfo(): Promise { const response = await this.session.customRequest('debugInfo'); @@ -268,29 +260,13 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb return undefined; } - private async sendRequestToJupyterSession(message: DebugProtocol.ProtocolMessage) { + protected async sendRequestToJupyterSession(message: DebugProtocol.ProtocolMessage) { if (this.jupyterSession.disposed || this.jupyterSession.status === 'dead') { traceInfo(`Skipping sending message ${message.type} because session is disposed`); return; } // map Source paths from VS Code to Ipykernel temp files - getMessageSourceAndHookIt(message, (source, lines?: { line?: number; endLine?: number; lines?: number[] }) => { - if (source && source.path) { - const mapping = this.cellToFile.get(Uri.file(source.path)); - if (mapping) { - source.path = mapping.path; - if (typeof lines?.endLine === 'number') { - lines.endLine = lines.endLine - (mapping.lineOffset || 0); - } - if (typeof lines?.line === 'number') { - lines.line = lines.line - (mapping.lineOffset || 0); - } - if (lines?.lines && Array.isArray(lines?.lines)) { - lines.lines = lines?.lines.map((line) => line - (mapping.lineOffset || 0)); - } - } - } - }); + getMessageSourceAndHookIt(message, this.translateRealFileToDebuggerFile.bind(this)); this.trace('to kernel', JSON.stringify(message)); if (message.type === 'request') { @@ -307,27 +283,7 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb control.onReply = (msg) => { const message = msg.content as DebugProtocol.ProtocolMessage; - getMessageSourceAndHookIt( - message, - (source, lines?: { line?: number; endLine?: number; lines?: number[] }) => { - if (source && source.path) { - const mapping = this.fileToCell.get(source.path) ?? this.lookupCellByLongName(source.path); - if (mapping) { - source.name = path.basename(mapping.uri.path); - source.path = mapping.uri.toString(); - if (typeof lines?.endLine === 'number') { - lines.endLine = lines.endLine + (mapping.lineOffset || 0); - } - if (typeof lines?.line === 'number') { - lines.line = lines.line + (mapping.lineOffset || 0); - } - if (lines?.lines && Array.isArray(lines?.lines)) { - lines.lines = lines?.lines.map((line) => line + (mapping.lineOffset || 0)); - } - } - } - } - ); + getMessageSourceAndHookIt(message, this.translateDebuggerFileToRealFile.bind(this)); this.trace('response', JSON.stringify(message)); this.sendMessage.fire(message); @@ -350,4 +306,20 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb traceError(`Unknown message type to send ${message.type}`); } } + protected translateDebuggerFileToRealFile( + source: DebugProtocol.Source | undefined, + _lines?: { line?: number; endLine?: number; lines?: number[] } + ) { + if (source && source.path) { + const mapping = this.fileToCell.get(source.path) ?? this.lookupCellByLongName(source.path); + if (mapping) { + source.name = path.basename(mapping.uri.path); + source.path = mapping.uri.toString(); + } + } + } + protected abstract translateRealFileToDebuggerFile( + source: DebugProtocol.Source | undefined, + _lines?: { line?: number; endLine?: number; lines?: number[] } + ): void; } diff --git a/src/kernels/debugger/types.ts b/src/kernels/debugger/types.ts index 5e9ec22099f9..f68c83f469ea 100644 --- a/src/kernels/debugger/types.ts +++ b/src/kernels/debugger/types.ts @@ -13,8 +13,7 @@ import { Event, NotebookCell, NotebookDocument, - NotebookEditor, - Uri + NotebookEditor } from 'vscode'; export interface ISourceMapMapping { @@ -71,7 +70,6 @@ export interface IKernelDebugAdapter extends DebugAdapter { onDidEndSession: Event; dumpAllCells(): Promise; getConfiguration(): IKernelDebugAdapterConfig; - getSourcePath(filePath: Uri): string | undefined; } export const IDebuggingManager = Symbol('IDebuggingManager'); diff --git a/src/notebooks/debugger/helper.ts b/src/notebooks/debugger/helper.ts index 53bac4097e07..4493f549fe3e 100644 --- a/src/notebooks/debugger/helper.ts +++ b/src/notebooks/debugger/helper.ts @@ -78,9 +78,7 @@ export function getMessageSourceAndHookIt( case 'setBreakpoints': // Keep track of the original source to be passed for other hooks. const originalSource = { ...(request.arguments as DebugProtocol.SetBreakpointsArguments).source }; - sourceHook((request.arguments as DebugProtocol.SetBreakpointsArguments).source); - // We pass a copy of the original source, as only the original object as the unaltered source. - sourceHook({ ...originalSource }, request.arguments); + sourceHook((request.arguments as DebugProtocol.SetBreakpointsArguments).source, request.arguments); const breakpoints = (request.arguments as DebugProtocol.SetBreakpointsArguments).breakpoints; if (breakpoints && Array.isArray(breakpoints)) { breakpoints.forEach((bk) => { diff --git a/src/notebooks/debugger/kernelDebugAdapter.ts b/src/notebooks/debugger/kernelDebugAdapter.ts index 2ccfc9e5c3a6..b6e9182a78c0 100644 --- a/src/notebooks/debugger/kernelDebugAdapter.ts +++ b/src/notebooks/debugger/kernelDebugAdapter.ts @@ -8,8 +8,16 @@ import { IDumpCellResponse } from '../../kernels/debugger/types'; import { traceError } from '../../platform/logging'; import { KernelDebugAdapterBase } from '../../kernels/debugger/kernelDebugAdapterBase'; import { executeSilently } from '../../kernels/helpers'; +import { DebugProtocol } from 'vscode-debugprotocol'; export class KernelDebugAdapter extends KernelDebugAdapterBase { + protected readonly cellToFile = new Map< + string, + { + path: string; + lineOffset?: number; + } + >(); public override dispose() { super.dispose(); // On dispose, delete our temp cell files @@ -29,13 +37,24 @@ export class KernelDebugAdapter extends KernelDebugAdapterBase { this.fileToCell.set(norm, { uri: cell.document.uri }); - this.cellToFile.set(cell.document.uri, { + this.cellToFile.set(cell.document.uri.toString(), { path: norm }); } catch (err) { traceError(err); } } + protected translateRealFileToDebuggerFile( + source: DebugProtocol.Source | undefined, + _lines?: { line?: number; endLine?: number; lines?: number[] } + ) { + if (source && source.path) { + const mapping = this.cellToFile.get(source.path); + if (mapping) { + source.path = mapping.path; + } + } + } // Use our jupyter session to delete all the cells private async deleteDumpCells() {