From 86f66d6c816f30f394a9b1ecf71c8c4e416c52fb Mon Sep 17 00:00:00 2001 From: isidor Date: Mon, 23 Sep 2019 16:26:08 +0200 Subject: [PATCH] Breakpoint supported should live on SessionData in the breakpoint fixes #81303 --- .../contrib/debug/browser/breakpointsView.ts | 28 ++----- .../contrib/debug/browser/debugSession.ts | 14 ++-- .../workbench/contrib/debug/common/debug.ts | 1 + .../contrib/debug/common/debugModel.ts | 73 ++++++++++++++++--- .../debug/test/browser/debugModel.test.ts | 14 +++- 5 files changed, 89 insertions(+), 41 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/breakpointsView.ts b/src/vs/workbench/contrib/debug/browser/breakpointsView.ts index fa909505898..7f8ffb936c3 100644 --- a/src/vs/workbench/contrib/debug/browser/breakpointsView.ts +++ b/src/vs/workbench/contrib/debug/browser/breakpointsView.ts @@ -655,9 +655,8 @@ export function getBreakpointMessageAndClassName(debugService: IDebugService, br }; } - const session = debugService.getViewModel().focusedSession; if (breakpoint instanceof FunctionBreakpoint) { - if (session && !session.capabilities.supportsFunctionBreakpoints) { + if (!breakpoint.supported) { return { className: 'debug-function-breakpoint-unverified', message: nls.localize('functionBreakpointUnsupported', "Function breakpoints not supported by this debug type"), @@ -671,7 +670,7 @@ export function getBreakpointMessageAndClassName(debugService: IDebugService, br } if (breakpoint instanceof DataBreakpoint) { - if (session && !session.capabilities.supportsDataBreakpoints) { + if (!breakpoint.supported) { return { className: 'debug-data-breakpoint-unverified', message: nls.localize('dataBreakpointUnsupported', "Data breakpoints not supported by this debug type"), @@ -686,30 +685,17 @@ export function getBreakpointMessageAndClassName(debugService: IDebugService, br if (breakpoint.logMessage || breakpoint.condition || breakpoint.hitCondition) { const messages: string[] = []; - if (breakpoint.logMessage) { - if (session && !session.capabilities.supportsLogPoints) { - return { - className: 'debug-breakpoint-unsupported', - message: nls.localize('logBreakpointUnsupported', "Logpoints not supported by this debug type"), - }; - } - - messages.push(nls.localize('logMessage', "Log Message: {0}", breakpoint.logMessage)); - } - if (session && breakpoint.condition && !session.capabilities.supportsConditionalBreakpoints) { + if (!breakpoint.supported) { return { className: 'debug-breakpoint-unsupported', - message: nls.localize('conditionalBreakpointUnsupported', "Conditional breakpoints not supported by this debug type"), - }; - } - if (session && breakpoint.hitCondition && !session.capabilities.supportsHitConditionalBreakpoints) { - return { - className: 'debug-breakpoint-unsupported', - message: nls.localize('hitBreakpointUnsupported', "Hit conditional breakpoints not supported by this debug type"), + message: nls.localize('breakpointUnsupported', "Breakpoints of this type are not supported by the debugger"), }; } + if (breakpoint.logMessage) { + messages.push(nls.localize('logMessage', "Log Message: {0}", breakpoint.logMessage)); + } if (breakpoint.condition) { messages.push(nls.localize('expression', "Expression: {0}", breakpoint.condition)); } diff --git a/src/vs/workbench/contrib/debug/browser/debugSession.ts b/src/vs/workbench/contrib/debug/browser/debugSession.ts index 6166571dddc..e50b51b45e0 100644 --- a/src/vs/workbench/contrib/debug/browser/debugSession.ts +++ b/src/vs/workbench/contrib/debug/browser/debugSession.ts @@ -313,7 +313,7 @@ export class DebugSession implements IDebugSession { data.set(breakpointsToSend[i].getId(), response.body.breakpoints[i]); } - this.model.setBreakpointSessionData(this.getId(), data); + this.model.setBreakpointSessionData(this.getId(), this.capabilities, data); } }); } @@ -327,7 +327,7 @@ export class DebugSession implements IDebugSession { for (let i = 0; i < fbpts.length; i++) { data.set(fbpts[i].getId(), response.body.breakpoints[i]); } - this.model.setBreakpointSessionData(this.getId(), data); + this.model.setBreakpointSessionData(this.getId(), this.capabilities, data); } }); } @@ -367,7 +367,7 @@ export class DebugSession implements IDebugSession { for (let i = 0; i < dataBreakpoints.length; i++) { data.set(dataBreakpoints[i].getId(), response.body.breakpoints[i]); } - this.model.setBreakpointSessionData(this.getId(), data); + this.model.setBreakpointSessionData(this.getId(), this.capabilities, data); } }); } @@ -844,7 +844,7 @@ export class DebugSession implements IDebugSession { }], false); if (bps.length === 1) { const data = new Map([[bps[0].getId(), event.body.breakpoint]]); - this.model.setBreakpointSessionData(this.getId(), data); + this.model.setBreakpointSessionData(this.getId(), this.capabilities, data); } } @@ -863,11 +863,11 @@ export class DebugSession implements IDebugSession { event.body.breakpoint.column = undefined; } const data = new Map([[breakpoint.getId(), event.body.breakpoint]]); - this.model.setBreakpointSessionData(this.getId(), data); + this.model.setBreakpointSessionData(this.getId(), this.capabilities, data); } if (functionBreakpoint) { const data = new Map([[functionBreakpoint.getId(), event.body.breakpoint]]); - this.model.setBreakpointSessionData(this.getId(), data); + this.model.setBreakpointSessionData(this.getId(), this.capabilities, data); } } })); @@ -885,7 +885,7 @@ export class DebugSession implements IDebugSession { this.rawListeners.push(this.raw.onDidExitAdapter(event => { this.initialized = true; - this.model.setBreakpointSessionData(this.getId(), undefined); + this.model.setBreakpointSessionData(this.getId(), this.capabilities, undefined); this._onDidEndAdapter.fire(event); })); } diff --git a/src/vs/workbench/contrib/debug/common/debug.ts b/src/vs/workbench/contrib/debug/common/debug.ts index 61238cc36d4..81e4d032dba 100644 --- a/src/vs/workbench/contrib/debug/common/debug.ts +++ b/src/vs/workbench/contrib/debug/common/debug.ts @@ -350,6 +350,7 @@ export interface IBaseBreakpoint extends IEnablement { readonly hitCondition?: string; readonly logMessage?: string; readonly verified: boolean; + readonly supported: boolean; getIdFromAdapter(sessionId: string): number | undefined; } diff --git a/src/vs/workbench/contrib/debug/common/debugModel.ts b/src/vs/workbench/contrib/debug/common/debugModel.ts index 339769792d1..a27bca068fc 100644 --- a/src/vs/workbench/contrib/debug/common/debugModel.ts +++ b/src/vs/workbench/contrib/debug/common/debugModel.ts @@ -23,6 +23,7 @@ import { posix } from 'vs/base/common/path'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles'; import { ITextEditor } from 'vs/workbench/common/editor'; +import { mixin } from 'vs/base/common/objects'; export class ExpressionContainer implements IExpressionContainer { @@ -498,10 +499,28 @@ export class Enablement implements IEnablement { } } -export class BaseBreakpoint extends Enablement implements IBaseBreakpoint { +interface IBreakpointSessionData extends DebugProtocol.Breakpoint { + supportsConditionalBreakpoints: boolean; + supportsHitConditionalBreakpoints: boolean; + supportsLogPoints: boolean; + supportsFunctionBreakpoints: boolean; + supportsDataBreakpoints: boolean; +} + +function toBreakpointSessionData(data: DebugProtocol.Breakpoint, capabilities: DebugProtocol.Capabilities): IBreakpointSessionData { + return mixin({ + supportsConditionalBreakpoints: !!capabilities.supportsConditionalBreakpoints, + supportsHitConditionalBreakpoints: !!capabilities.supportsHitConditionalBreakpoints, + supportsLogPoints: !!capabilities.supportsLogPoints, + supportsFunctionBreakpoints: !!capabilities.supportsFunctionBreakpoints, + supportsDataBreakpoints: !!capabilities.supportsDataBreakpoints + }, data); +} + +export abstract class BaseBreakpoint extends Enablement implements IBaseBreakpoint { - private sessionData = new Map(); - protected data: DebugProtocol.Breakpoint | undefined; + private sessionData = new Map(); + protected data: IBreakpointSessionData | undefined; constructor( enabled: boolean, @@ -516,7 +535,7 @@ export class BaseBreakpoint extends Enablement implements IBaseBreakpoint { } } - setSessionData(sessionId: string, data: DebugProtocol.Breakpoint | undefined): void { + setSessionData(sessionId: string, data: IBreakpointSessionData | undefined): void { if (!data) { this.sessionData.delete(sessionId); } else { @@ -546,6 +565,8 @@ export class BaseBreakpoint extends Enablement implements IBaseBreakpoint { return this.data ? this.data.verified : true; } + abstract get supported(): boolean; + getIdFromAdapter(sessionId: string): number | undefined { const data = this.sessionData.get(sessionId); return data ? data.id : undefined; @@ -623,7 +644,25 @@ export class Breakpoint extends BaseBreakpoint implements IBreakpoint { }; } - setSessionData(sessionId: string, data: DebugProtocol.Breakpoint | undefined): void { + get supported(): boolean { + if (!this.data) { + return true; + } + if (this.logMessage && !this.data.supportsLogPoints) { + return false; + } + if (this.condition && !this.data.supportsConditionalBreakpoints) { + return false; + } + if (this.hitCondition && !this.data.supportsHitConditionalBreakpoints) { + return false; + } + + return true; + } + + + setSessionData(sessionId: string, data: IBreakpointSessionData | undefined): void { super.setSessionData(sessionId, data); if (!this._adapterData) { this._adapterData = this.adapterData; @@ -683,6 +722,14 @@ export class FunctionBreakpoint extends BaseBreakpoint implements IFunctionBreak return result; } + get supported(): boolean { + if (!this.data) { + return true; + } + + return this.data.supportsFunctionBreakpoints; + } + toString(): string { return this.name; } @@ -711,6 +758,14 @@ export class DataBreakpoint extends BaseBreakpoint implements IDataBreakpoint { return result; } + get supported(): boolean { + if (!this.data) { + return true; + } + + return this.data.supportsDataBreakpoints; + } + toString(): string { return this.label; } @@ -971,14 +1026,14 @@ export class DebugModel implements IDebugModel { this._onDidChangeBreakpoints.fire({ changed: updated }); } - setBreakpointSessionData(sessionId: string, data: Map | undefined): void { + setBreakpointSessionData(sessionId: string, capabilites: DebugProtocol.Capabilities, data: Map | undefined): void { this.breakpoints.forEach(bp => { if (!data) { bp.setSessionData(sessionId, undefined); } else { const bpData = data.get(bp.getId()); if (bpData) { - bp.setSessionData(sessionId, bpData); + bp.setSessionData(sessionId, toBreakpointSessionData(bpData, capabilites)); } } }); @@ -988,7 +1043,7 @@ export class DebugModel implements IDebugModel { } else { const fbpData = data.get(fbp.getId()); if (fbpData) { - fbp.setSessionData(sessionId, fbpData); + fbp.setSessionData(sessionId, toBreakpointSessionData(fbpData, capabilites)); } } }); @@ -998,7 +1053,7 @@ export class DebugModel implements IDebugModel { } else { const dbpData = data.get(dbp.getId()); if (dbpData) { - dbp.setSessionData(sessionId, dbpData); + dbp.setSessionData(sessionId, toBreakpointSessionData(dbpData, capabilites)); } } }); diff --git a/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts b/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts index 2ad92873ae4..fc9aa21b00a 100644 --- a/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts +++ b/src/vs/workbench/contrib/debug/test/browser/debugModel.test.ts @@ -112,7 +112,7 @@ suite('Debug - Model', () => { test('breakpoints multiple sessions', () => { const modelUri = uri.file('/myfolder/myfile.js'); - const breakpoints = model.addBreakpoints(modelUri, [{ lineNumber: 5, enabled: true }, { lineNumber: 10, enabled: false }]); + const breakpoints = model.addBreakpoints(modelUri, [{ lineNumber: 5, enabled: true, condition: 'x > 5' }, { lineNumber: 10, enabled: false }]); const session = createMockSession(model); const data = new Map(); @@ -121,7 +121,7 @@ suite('Debug - Model', () => { data.set(breakpoints[0].getId(), { verified: false, line: 10 }); data.set(breakpoints[1].getId(), { verified: true, line: 50 }); - model.setBreakpointSessionData(session.getId(), data); + model.setBreakpointSessionData(session.getId(), {}, data); assert.equal(breakpoints[0].lineNumber, 5); assert.equal(breakpoints[1].lineNumber, 50); @@ -129,17 +129,23 @@ suite('Debug - Model', () => { const data2 = new Map(); data2.set(breakpoints[0].getId(), { verified: true, line: 100 }); data2.set(breakpoints[1].getId(), { verified: true, line: 500 }); - model.setBreakpointSessionData(session2.getId(), data2); + model.setBreakpointSessionData(session2.getId(), {}, data2); // Breakpoint is verified only once, show that line assert.equal(breakpoints[0].lineNumber, 100); // Breakpoint is verified two times, show the original line assert.equal(breakpoints[1].lineNumber, 10); - model.setBreakpointSessionData(session.getId(), undefined); + model.setBreakpointSessionData(session.getId(), {}, undefined); // No more double session verification assert.equal(breakpoints[0].lineNumber, 100); assert.equal(breakpoints[1].lineNumber, 500); + + assert.equal(breakpoints[0].supported, false); + const data3 = new Map(); + data3.set(breakpoints[0].getId(), { verified: true, line: 500 }); + model.setBreakpointSessionData(session2.getId(), { supportsConditionalBreakpoints: true }, data2); + assert.equal(breakpoints[0].supported, true); }); // Threads -- GitLab