Skip to content

Commit

Permalink
Fixes to IW debugging with breakpoints
Browse files Browse the repository at this point in the history
  • Loading branch information
@DonJayamanne
DonJayamanne committed Jun 2, 2022
1 parent 3d03651 commit 828c97d
Show file tree
Hide file tree
Showing 6 changed files with 167 additions and 62 deletions.
13 changes: 8 additions & 5 deletions src/interactive-window/debugger/jupyter/debugCellControllers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,20 @@ export class DebugCellController implements IDebuggingDelegate {
public async willSendEvent(_msg: DebugProtocolMessage): Promise<boolean> {
return false;
}
private debugCellDumped?: Promise<void>;
public async willSendRequest(request: DebugProtocol.Request): Promise<void> {
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();
}
}
Expand Down
121 changes: 118 additions & 3 deletions src/interactive-window/debugger/jupyter/kernelDebugAdapter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}
66 changes: 19 additions & 47 deletions src/kernels/debugger/kernelDebugAdapterBase.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import {
shortNameMatchesLongName,
getMessageSourceAndHookIt
} from '../../notebooks/debugger/helper';
import { ResourceMap } from '../../platform/vscode-path/map';
import { IDisposable } from '../../platform/common/types';

/**
Expand All @@ -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<DebugProtocolMessage>();
private readonly endSession = new EventEmitter<DebugSession>();
private readonly configuration: IKernelDebugAdapterConfig;
Expand Down Expand Up @@ -233,9 +228,6 @@ export abstract class KernelDebugAdapterBase implements DebugAdapter, IKernelDeb
);
}
protected abstract dumpCell(index: number): Promise<void>;
public getSourcePath(filePath: Uri) {
return this.cellToFile.get(filePath)?.path;
}

private async debugInfo(): Promise<void> {
const response = await this.session.customRequest('debugInfo');
Expand Down Expand Up @@ -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') {
Expand All @@ -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);
Expand All @@ -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;
}
4 changes: 1 addition & 3 deletions src/kernels/debugger/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ import {
Event,
NotebookCell,
NotebookDocument,
NotebookEditor,
Uri
NotebookEditor
} from 'vscode';

export interface ISourceMapMapping {
Expand Down Expand Up @@ -71,7 +70,6 @@ export interface IKernelDebugAdapter extends DebugAdapter {
onDidEndSession: Event<DebugSession>;
dumpAllCells(): Promise<void>;
getConfiguration(): IKernelDebugAdapterConfig;
getSourcePath(filePath: Uri): string | undefined;
}

export const IDebuggingManager = Symbol('IDebuggingManager');
Expand Down
4 changes: 1 addition & 3 deletions src/notebooks/debugger/helper.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
21 changes: 20 additions & 1 deletion src/notebooks/debugger/kernelDebugAdapter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() {
Expand Down

0 comments on commit 828c97d

Please sign in to comment.