Skip to content

Commit 7e56de5

Browse files
martin-fleck-atmartin-fleck-at
committed
Incorporate PR feedback
- Properly extract API proposal into it's own file - Remove data breakpoint info convenience method - Provide proposal for different data breakpoint sources (discussion) - Ensure that breakpoint mode is properly updated and passed through
1 parent ede5a2a commit 7e56de5

File tree

11 files changed

+223
-179
lines changed

11 files changed

+223
-179
lines changed

‎extensions/vscode-api-tests/src/singlefolder-tests/debug.test.ts

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,21 +104,48 @@ suite('vscode API - debug', function () {
104104
assert.strictEqual(addressDbp.accessType, 'readWrite');
105105
});
106106

107-
test('data breakpoint - dynamic variable', async function () {
108-
debug.addBreakpoints([new DataBreakpoint({ type: 'dynamicVariable', name: 'i', variablesReference: 1000 }, 'readWrite', false, 'data', false, 'condition', 'hitCondition', 'logMessage')]);
107+
test('data breakpoint - expression', async function () {
108+
debug.addBreakpoints([new DataBreakpoint({ type: 'expression', expression: 'i' }, 'readWrite', false, 'data', false, 'condition', 'hitCondition', 'logMessage')]);
109109
const dynamicVariableDbp = debug.breakpoints[debug.breakpoints.length - 1] as DataBreakpoint;
110110
assert.strictEqual(dynamicVariableDbp.condition, 'condition');
111111
assert.strictEqual(dynamicVariableDbp.hitCondition, 'hitCondition');
112112
assert.strictEqual(dynamicVariableDbp.logMessage, 'logMessage');
113113
assert.strictEqual(dynamicVariableDbp.enabled, false);
114114
assert.strictEqual(dynamicVariableDbp.label, 'data');
115-
assert.strictEqual(dynamicVariableDbp.source.type, 'dynamicVariable');
116-
assert.strictEqual(dynamicVariableDbp.source.name, 'i');
117-
assert.strictEqual(dynamicVariableDbp.source.variablesReference, 1000);
115+
assert.strictEqual(dynamicVariableDbp.source.type, 'expression');
116+
assert.strictEqual(dynamicVariableDbp.source.expression, 'i');
118117
assert.strictEqual(dynamicVariableDbp.canPersist, false);
119118
assert.strictEqual(dynamicVariableDbp.accessType, 'readWrite');
120119
});
121120

121+
test('data breakpoint - scoped', async function () {
122+
debug.addBreakpoints([new DataBreakpoint({ type: 'scoped', expression: 'exp()', frameId: 1 }, 'readWrite', false, 'data', false, 'condition', 'hitCondition', 'logMessage')]);
123+
const scopedExpression = debug.breakpoints[debug.breakpoints.length - 1] as DataBreakpoint;
124+
assert.strictEqual(scopedExpression.condition, 'condition');
125+
assert.strictEqual(scopedExpression.hitCondition, 'hitCondition');
126+
assert.strictEqual(scopedExpression.logMessage, 'logMessage');
127+
assert.strictEqual(scopedExpression.enabled, false);
128+
assert.strictEqual(scopedExpression.label, 'data');
129+
assert.strictEqual(scopedExpression.source.type, 'scoped');
130+
assert.strictEqual(scopedExpression.source.frameId, 1);
131+
assert.strictEqual(scopedExpression.source.expression, 'exp()');
132+
assert.strictEqual(scopedExpression.canPersist, false);
133+
assert.strictEqual(scopedExpression.accessType, 'readWrite');
134+
135+
debug.addBreakpoints([new DataBreakpoint({ type: 'scoped', variable: 'var', variablesReference: 1 }, 'readWrite', false, 'data', false, 'condition', 'hitCondition', 'logMessage')]);
136+
const scopedVariable = debug.breakpoints[debug.breakpoints.length - 1] as DataBreakpoint;
137+
assert.strictEqual(scopedVariable.condition, 'condition');
138+
assert.strictEqual(scopedVariable.hitCondition, 'hitCondition');
139+
assert.strictEqual(scopedVariable.logMessage, 'logMessage');
140+
assert.strictEqual(scopedVariable.enabled, false);
141+
assert.strictEqual(scopedVariable.label, 'data');
142+
assert.strictEqual(scopedVariable.source.type, 'scoped');
143+
assert.strictEqual(scopedVariable.source.variablesReference, 1);
144+
assert.strictEqual(scopedVariable.source.variable, 'var');
145+
assert.strictEqual(scopedVariable.canPersist, false);
146+
assert.strictEqual(scopedVariable.accessType, 'readWrite');
147+
});
148+
122149
test('start debugging', async function () {
123150
let stoppedEvents = 0;
124151
let variablesReceived: () => void;

‎src/vs/platform/extensions/common/extensionsApiProposals.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,9 @@ const _allApiProposals = {
154154
customEditorMove: {
155155
proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.customEditorMove.d.ts',
156156
},
157+
debugDataBreakpoints: {
158+
proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.debugDataBreakpoints.d.ts',
159+
},
157160
debugVisualization: {
158161
proposal: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.debugVisualization.d.ts',
159162
},

‎src/vs/workbench/api/browser/mainThreadDebugService.ts

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,11 @@ export class MainThreadDebugService implements MainThreadDebugServiceShape, IDeb
236236
this.debugService.addDataBreakpoint({
237237
description: dto.label,
238238
src: dto.source.type === 'variable' ? { type: DataBreakpointSetType.Variable, dataId: dto.source.dataId }
239-
: dto.source.type === 'dynamicVariable' ? { type: DataBreakpointSetType.DynamicVariable, name: dto.source.name, variablesReference: dto.source.variablesReference }
240-
: { type: DataBreakpointSetType.Address, address: dto.source.address, bytes: dto.source.bytes },
239+
: dto.source.type === 'address' ? { type: DataBreakpointSetType.Address, address: dto.source.address, bytes: dto.source.bytes }
240+
: dto.source.type === 'expression' ? { type: DataBreakpointSetType.Expression, expression: dto.source.expression }
241+
: dto.source.frameId ? { type: DataBreakpointSetType.Scoped, expression: dto.source.expression, frameId: dto.source.frameId }
242+
: dto.source.variablesReference ? { type: DataBreakpointSetType.Scoped, variable: dto.source.variable, variablesReference: dto.source.variablesReference }
243+
: { type: DataBreakpointSetType.Variable, dataId: '' }, // should not happen
241244
condition: dto.condition,
242245
enabled: dto.enabled,
243246
hitCondition: dto.hitCondition,
@@ -376,22 +379,6 @@ export class MainThreadDebugService implements MainThreadDebugServiceShape, IDeb
376379
return Promise.reject(new ErrorNoTelemetry('debug session not found'));
377380
}
378381

379-
public $getDataBreakpointInfo(sessionId: DebugSessionUUID, name: string, variablesReference?: number): Promise<IDataBreakpointInfo | undefined> {
380-
const session = this.debugService.getModel().getSession(sessionId, true);
381-
if (session) {
382-
return Promise.resolve(session.dataBreakpointInfo(name, variablesReference));
383-
}
384-
return Promise.reject(new ErrorNoTelemetry('debug session not found'));
385-
}
386-
387-
public $getDataBytesBreakpointInfo(sessionId: DebugSessionUUID, address: string, bytes?: number): Promise<IDataBreakpointInfo | undefined> {
388-
const session = this.debugService.getModel().getSession(sessionId, true);
389-
if (session) {
390-
return Promise.resolve(session.dataBytesBreakpointInfo(address, bytes));
391-
}
392-
return Promise.reject(new ErrorNoTelemetry('debug session not found'));
393-
}
394-
395382
public $stopDebugging(sessionId: DebugSessionUUID | undefined): Promise<void> {
396383
if (sessionId) {
397384
const session = this.debugService.getModel().getSession(sessionId, true);
@@ -473,23 +460,28 @@ export class MainThreadDebugService implements MainThreadDebugServiceShape, IDeb
473460
condition: fbp.condition,
474461
hitCondition: fbp.hitCondition,
475462
logMessage: fbp.logMessage,
476-
functionName: fbp.name
463+
functionName: fbp.name,
464+
mode: fbp.mode
477465
} satisfies IFunctionBreakpointDto;
478466
} else if ('src' in bp) {
479467
const dbp: IDataBreakpoint = bp;
480468
return {
481469
type: 'data',
482470
id: dbp.getId(),
483471
source: dbp.src.type === DataBreakpointSetType.Variable ? { type: 'variable', dataId: dbp.src.dataId }
484-
: dbp.src.type === DataBreakpointSetType.DynamicVariable ? { type: 'dynamicVariable', name: dbp.src.name, variablesReference: dbp.src.variablesReference }
485-
: { type: 'address', address: dbp.src.address, bytes: dbp.src.bytes },
472+
: dbp.src.type === DataBreakpointSetType.Address ? { type: 'address', address: dbp.src.address, bytes: dbp.src.bytes }
473+
: dbp.src.type === DataBreakpointSetType.Expression ? { type: 'expression', expression: dbp.src.expression }
474+
: dbp.src.frameId ? { type: 'scoped', expression: dbp.src.expression, frameId: dbp.src.frameId }
475+
: dbp.src.variablesReference ? { type: 'scoped', variable: dbp.src.variable, variablesReference: dbp.src.variablesReference }
476+
: { type: 'variable', dataId: '' }, // should not happen
486477
enabled: dbp.enabled,
487478
condition: dbp.condition,
488479
hitCondition: dbp.hitCondition,
489480
logMessage: dbp.logMessage,
490481
accessType: dbp.accessType,
491482
label: dbp.description,
492-
canPersist: dbp.canPersist
483+
canPersist: dbp.canPersist,
484+
mode: dbp.mode
493485
} satisfies IDataBreakpointDto;
494486
} else if ('uri' in bp) {
495487
const sbp: IBreakpoint = bp;
@@ -503,6 +495,7 @@ export class MainThreadDebugService implements MainThreadDebugServiceShape, IDeb
503495
uri: sbp.uri,
504496
line: sbp.lineNumber > 0 ? sbp.lineNumber - 1 : 0,
505497
character: (typeof sbp.column === 'number' && sbp.column > 0) ? sbp.column - 1 : 0,
498+
mode: sbp.mode
506499
} satisfies ISourceBreakpointDto;
507500
} else {
508501
return undefined;

‎src/vs/workbench/api/common/extHost.protocol.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1632,8 +1632,6 @@ export interface MainThreadDebugServiceShape extends IDisposable {
16321632
$setDebugSessionName(id: DebugSessionUUID, name: string): void;
16331633
$customDebugAdapterRequest(id: DebugSessionUUID, command: string, args: any): Promise<any>;
16341634
$getDebugProtocolBreakpoint(id: DebugSessionUUID, breakpoinId: string): Promise<DebugProtocol.Breakpoint | undefined>;
1635-
$getDataBreakpointInfo(id: DebugSessionUUID, name: string, variablesReference?: number): Promise<IDataBreakpointInfo | undefined>;
1636-
$getDataBytesBreakpointInfo(id: DebugSessionUUID, address: string, bytes?: number): Promise<IDataBreakpointInfo | undefined>;
16371635
$appendDebugConsole(value: string): void;
16381636
$registerBreakpoints(breakpoints: Array<ISourceMultiBreakpointDto | IFunctionBreakpointDto | IDataBreakpointDto>): Promise<void>;
16391637
$unregisterBreakpoints(breakpointIds: string[], functionBreakpointIds: string[], dataBreakpointIds: string[]): Promise<void>;
@@ -2394,8 +2392,6 @@ export interface IFunctionBreakpointDto extends IBreakpointDto {
23942392
mode?: string;
23952393
}
23962394

2397-
export type IDataBreakpointInfo = DebugProtocol.DataBreakpointInfoResponse['body'];
2398-
23992395
export interface IDataBreakpointDto extends IBreakpointDto {
24002396
type: 'data';
24012397
source: vscode.DataBreakpointSource;

‎src/vs/workbench/api/common/extHostDebugService.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { IExtHostCommands } from './extHostCommands.js';
3131
import * as Convert from './extHostTypeConverters.js';
3232
import { coalesce } from '../../../base/common/arrays.js';
3333
import { IExtHostTesting } from './extHostTesting.js';
34+
import { Mutable } from '../../../base/common/types.js';
3435

3536
export const IExtHostDebugService = createDecorator<IExtHostDebugService>('IExtHostDebugService');
3637

@@ -756,28 +757,31 @@ export abstract class ExtHostDebugServiceBase extends DisposableCls implements I
756757
const bp = this._breakpoints.get(bpd.id);
757758
if (bp) {
758759
if (bp instanceof FunctionBreakpoint && bpd.type === 'function') {
759-
const fbp = <any>bp;
760+
const fbp = <Mutable<FunctionBreakpoint>>bp;
760761
fbp.enabled = bpd.enabled;
761762
fbp.condition = bpd.condition;
762763
fbp.hitCondition = bpd.hitCondition;
763764
fbp.logMessage = bpd.logMessage;
764765
fbp.functionName = bpd.functionName;
766+
fbp.mode = bpd.mode;
765767
} else if (bp instanceof SourceBreakpoint && bpd.type === 'source') {
766-
const sbp = <any>bp;
768+
const sbp = <Mutable<SourceBreakpoint>>bp;
767769
sbp.enabled = bpd.enabled;
768770
sbp.condition = bpd.condition;
769771
sbp.hitCondition = bpd.hitCondition;
770772
sbp.logMessage = bpd.logMessage;
771773
sbp.location = new Location(URI.revive(bpd.uri), new Position(bpd.line, bpd.character));
774+
sbp.mode = bpd.mode;
772775
} else if (bp instanceof DataBreakpoint && bpd.type === 'data') {
773-
const dbp = <any>bp;
776+
const dbp = <Mutable<DataBreakpoint>>bp;
774777
dbp.enabled = bpd.enabled;
775778
dbp.condition = bpd.condition;
776779
dbp.hitCondition = bpd.hitCondition;
777780
dbp.logMessage = bpd.logMessage;
778781
dbp.label = bpd.label;
779782
dbp.source = bpd.source;
780783
dbp.canPersist = bpd.canPersist;
784+
dbp.mode = bpd.mode;
781785
dbp.accessType = bpd.accessType;
782786
}
783787
c.push(bp);
@@ -1143,12 +1147,6 @@ export class ExtHostDebugSession {
11431147
},
11441148
getDebugProtocolBreakpoint(breakpoint: vscode.Breakpoint): Promise<vscode.DebugProtocolBreakpoint | undefined> {
11451149
return that._debugServiceProxy.$getDebugProtocolBreakpoint(that._id, breakpoint.id);
1146-
},
1147-
getDataBreakpointInfo(name: string, variablesReference?: number): Promise<vscode.DataBreakpointInfo | undefined> {
1148-
return that._debugServiceProxy.$getDataBreakpointInfo(that._id, name, variablesReference);
1149-
},
1150-
getDataBytesBreakpointInfo(address: string, bytes?: number): Promise<vscode.DataBreakpointInfo | undefined> {
1151-
return that._debugServiceProxy.$getDataBytesBreakpointInfo(that._id, address, bytes);
11521150
}
11531151
});
11541152
}

‎src/vs/workbench/api/common/extHostTypes.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3088,7 +3088,10 @@ export class DataBreakpoint extends Breakpoint {
30883088
this.label = label ? label
30893089
: this.source.type === 'variable' ? `DataId '${this.source.dataId}'`
30903090
: this.source.type === 'address' ? `Address '${this.source.address}${this.source.bytes ? `,${this.source.bytes}'` : ''}`
3091-
: `Variable '${this.source.name}${this.source.variablesReference ? `,${this.source.variablesReference}` : ''}'`;
3091+
: this.source.type === 'expression' ? `Expression '${this.source.expression}'`
3092+
: this.source.frameId ? `Scoped '${this.source.expression}@${this.source.frameId}'`
3093+
: this.source.variablesReference ? `Scoped '${this.source.variable}@${this.source.variablesReference}'`
3094+
: `Unknown data breakpoint`;
30923095
}
30933096
}
30943097

@@ -4129,7 +4132,7 @@ export function validateTestCoverageCount(cc?: vscode.TestCoverageCount) {
41294132
}
41304133

41314134
if (cc.covered > cc.total) {
4132-
throw new Error(`The total number of covered items (${cc.covered}) cannot be greater than the total(${cc.total})`);
4135+
throw new Error(`The total number of covered items (${cc.covered}) cannot be greater than the total (${cc.total})`);
41334136
}
41344137

41354138
if (cc.total < 0) {

‎src/vs/workbench/contrib/debug/browser/debugSession.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -553,8 +553,8 @@ export class DebugSession implements IDebugSession, IDisposable {
553553
return this._dataBreakpointInfo({ name: address, bytes, asAddress: true });
554554
}
555555

556-
dataBreakpointInfo(name: string, variablesReference?: number): Promise<IDataBreakpointInfoResponse | undefined> {
557-
return this._dataBreakpointInfo({ name, variablesReference });
556+
dataBreakpointInfo(name: string, variablesReference?: number, frameId?: number): Promise<IDataBreakpointInfoResponse | undefined> {
557+
return this._dataBreakpointInfo({ name, variablesReference, frameId });
558558
}
559559

560560
private async _dataBreakpointInfo(args: DebugProtocol.DataBreakpointInfoArguments): Promise<IDataBreakpointInfoResponse | undefined> {

‎src/vs/workbench/contrib/debug/common/debug.ts

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -432,7 +432,7 @@ export interface IDebugSession extends ITreeElement {
432432

433433
sendBreakpoints(modelUri: uri, bpts: IBreakpoint[], sourceModified: boolean): Promise<void>;
434434
sendFunctionBreakpoints(fbps: IFunctionBreakpoint[]): Promise<void>;
435-
dataBreakpointInfo(name: string, variablesReference?: number): Promise<IDataBreakpointInfoResponse | undefined>;
435+
dataBreakpointInfo(name: string, variablesReference?: number, frameId?: number): Promise<IDataBreakpointInfoResponse | undefined>;
436436
dataBytesBreakpointInfo(address: string, bytes?: number): Promise<IDataBreakpointInfoResponse | undefined>;
437437
sendDataBreakpoints(dbps: IDataBreakpoint[]): Promise<void>;
438438
sendInstructionBreakpoints(dbps: IInstructionBreakpoint[]): Promise<void>;
@@ -646,7 +646,8 @@ export interface IExceptionBreakpoint extends IBaseBreakpoint {
646646
export const enum DataBreakpointSetType {
647647
Variable,
648648
Address,
649-
DynamicVariable
649+
Expression,
650+
Scoped
650651
}
651652

652653
/**
@@ -661,22 +662,39 @@ export type DataBreakpointSource =
661662
/** An identifier for the data. If it was retrieved using a `variablesReference` it may only be valid in the current suspended state, otherwise it's valid indefinitely. */
662663
dataId: string;
663664
}
664-
| {
665-
/** The source type for address-based data breakpoints. This only works on sessions that have the `supportsDataBreakpointBytes` capability. */
666-
type: DataBreakpointSetType.DynamicVariable;
667-
/** The name of the variable's child to obtain data breakpoint information for. If `variablesReference` isn't specified, this can be an expression. */
668-
name: string;
669-
/** Reference to the variable container if the data breakpoint is requested for a child of the container. */
670-
variablesReference?: number;
671-
}
672665
| {
673666
/** The source type for address-based data breakpoints. This only works on sessions that have the `supportsDataBreakpointBytes` capability. */
674667
type: DataBreakpointSetType.Address;
675668
/** A memory address as a decimal value, or hex value if it is prefixed with `0x`. */
676669
address: string;
677670
/** If specified, returns information for the range of memory extending `bytes` number of bytes from the address. */
678671
bytes?: number;
679-
};
672+
}
673+
| {
674+
/** The source type for address-based data breakpoints. This only works on sessions that have the `supportsDataBreakpointBytes` capability. */
675+
type: DataBreakpointSetType.Expression;
676+
/** A global expression that is first evaluated when the breakpoint is activated. */
677+
expression: string;
678+
}
679+
| {
680+
/** The source type for address-based data breakpoints. This only works on sessions that have the `supportsDataBreakpointBytes` capability. */
681+
type: DataBreakpointSetType.Scoped;
682+
} & (
683+
| {
684+
/** The name of the variable that is used for resolution. */
685+
variable: string;
686+
/** Reference to the variable container that has the variable named `variable`. */
687+
variablesReference: number;
688+
frameId?: never;
689+
}
690+
| {
691+
/** The name of the expression that is used for resolution. */
692+
expression: string;
693+
/** Reference to the stack frame to which the expression is scoped. */
694+
frameId: number;
695+
variablesReference?: never;
696+
}
697+
);
680698

681699
export interface IDataBreakpoint extends IBaseBreakpoint {
682700
readonly description: string;

0 commit comments

Comments
 (0)