Skip to content

Commit fba4697

Browse files
committed
[LogDebugger] Do not proceed to first breakpoint when set
Setting the breakpoints had a unexpected side effect: when line == 1 it would proceed to the first breakpoint. This seems kinda unexpected, so I removed the behavior.
1 parent 64038ff commit fba4697

File tree

3 files changed

+21
-26
lines changed

3 files changed

+21
-26
lines changed

editors/code/src/debugAdapter.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ export class DebugSession extends LoggingDebugSession {
135135

136136
const source = args.source.path as string;
137137
const bps = args.breakpoints || [];
138-
const breakpoints = this._logDebugger.setBreakPoint(source, bps);
138+
const breakpoints = this._logDebugger.setBreakpoints(source, bps);
139139

140140
response.body = {
141141
breakpoints: breakpoints
@@ -210,15 +210,15 @@ export class DebugSession extends LoggingDebugSession {
210210
protected continueRequest(response: DebugProtocol.ContinueResponse, args: DebugProtocol.ContinueArguments): void {
211211
console.log(`continueRequest ${JSON.stringify(args)}`);
212212

213-
this._logDebugger.gotoNextBreakpoint();
213+
this._logDebugger.gotoBreakpoint();
214214
this.sendEvent(new StoppedEvent('breakpoint', DebugSession._threadID));
215215
this.sendResponse(response);
216216
}
217217

218218
protected reverseContinueRequest(response: DebugProtocol.ReverseContinueResponse, args: DebugProtocol.ReverseContinueArguments): void {
219219
console.log(`reverseContinueRequest ${JSON.stringify(args)}`);
220220

221-
this._logDebugger.gotoNextBreakpoint(true);
221+
this._logDebugger.gotoBreakpoint(true);
222222
this.sendEvent(new StoppedEvent('breakpoint', DebugSession._threadID));
223223
this.sendResponse(response);
224224
}

editors/code/src/logDebugger.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,11 @@ export class LogDebugger {
2222
this._logLines = logLines;
2323
}
2424

25-
setBreakPoint(source: string, breakpoints: DebugProtocol.SourceBreakpoint[]): DebugProtocol.Breakpoint[] {
25+
setBreakpoints(source: string, breakpoints: DebugProtocol.SourceBreakpoint[]): DebugProtocol.Breakpoint[] {
2626
const bps = new Array<DebugProtocol.Breakpoint>();
2727
const sourcePath = path.resolve(source);
2828
this._breakPoints.set(sourcePath, bps);
2929
breakpoints.forEach((breakpoint) => {
30-
if (this._line === 1) {
31-
this._line = breakpoint.line;
32-
}
3330
const verified = breakpoint.line > 0 && breakpoint.line < this._logLines;
3431
bps.push({ line: breakpoint.line, verified: verified });
3532
});
@@ -53,7 +50,7 @@ export class LogDebugger {
5350
this._line = Math.max(1, this._line - 1);
5451
}
5552

56-
gotoNextBreakpoint(reverse = false): void {
53+
gotoBreakpoint(reverse = false): void {
5754
this._line = this.findNextLineToStop(reverse);
5855
}
5956

editors/code/src/test/suite/logDebugger.test.ts

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -52,49 +52,47 @@ suite('LogDebugger Test Suite', () => {
5252

5353
test('hasBreakpoints is true after breakpoints set', () => {
5454
logDebugger.setToLog(logPath, 100);
55-
logDebugger.setBreakPoint(logPath, [{ line: 10 }]);
55+
logDebugger.setBreakpoints(logPath, [{ line: 10 }]);
5656
assert.ok(logDebugger.hasBreakpoints());
5757
});
5858

5959
test('hasBreakpoints is false after empty breakpoints set', () => {
6060
logDebugger.setToLog(logPath, 100);
61-
logDebugger.setBreakPoint(logPath, []);
61+
logDebugger.setBreakpoints(logPath, []);
6262
assert.strictEqual(logDebugger.hasBreakpoints(), false);
6363
});
6464

6565
test('gotoNextBreakpoint advances to next breakpoint', () => {
6666
logDebugger.setToLog(logPath, 100);
67-
// setBreakPoint moves _line to the first breakpoint (10) when starting at line 1
68-
logDebugger.setBreakPoint(logPath, [{ line: 10 }, { line: 20 }, { line: 30 }]);
67+
logDebugger.setBreakpoints(logPath, [{ line: 10 }, { line: 20 }, { line: 30 }]);
68+
assert.strictEqual(logDebugger.linenum(), 1);
69+
logDebugger.gotoBreakpoint();
6970
assert.strictEqual(logDebugger.linenum(), 10);
70-
logDebugger.gotoNextBreakpoint();
71+
logDebugger.gotoBreakpoint();
7172
assert.strictEqual(logDebugger.linenum(), 20);
72-
logDebugger.gotoNextBreakpoint();
73-
assert.strictEqual(logDebugger.linenum(), 30);
7473
});
7574

7675
test('gotoNextBreakpoint reverse retreats to previous breakpoint', () => {
7776
logDebugger.setToLog(logPath, 100);
78-
// setBreakPoint moves _line to 10; navigate forward to 30, then reverse to 20
79-
logDebugger.setBreakPoint(logPath, [{ line: 10 }, { line: 20 }, { line: 30 }]);
80-
logDebugger.gotoNextBreakpoint(); // 10 → 20
81-
logDebugger.gotoNextBreakpoint(); // 20 → 30
82-
logDebugger.gotoNextBreakpoint(true); // 30 → 20
83-
assert.strictEqual(logDebugger.linenum(), 20);
77+
logDebugger.setBreakpoints(logPath, [{ line: 10 }, { line: 20 }, { line: 30 }]);
78+
logDebugger.gotoBreakpoint(); // 1 → 10
79+
logDebugger.gotoBreakpoint(); // 10 → 20
80+
logDebugger.gotoBreakpoint(true); // 20 → 10
81+
assert.strictEqual(logDebugger.linenum(), 10);
8482
});
8583

8684
test('gotoNextBreakpoint past last clamps to logLines', () => {
8785
logDebugger.setToLog(logPath, 100);
88-
logDebugger.setBreakPoint(logPath, [{ line: 10 }]);
89-
logDebugger.gotoNextBreakpoint();
90-
logDebugger.gotoNextBreakpoint();
86+
logDebugger.setBreakpoints(logPath, [{ line: 10 }]);
87+
logDebugger.gotoBreakpoint();
88+
logDebugger.gotoBreakpoint();
9189
assert.strictEqual(logDebugger.linenum(), 100);
9290
});
9391

9492
test('gotoNextBreakpoint before first in reverse clamps to line 1', () => {
9593
logDebugger.setToLog(logPath, 100);
96-
logDebugger.setBreakPoint(logPath, [{ line: 10 }]);
97-
logDebugger.gotoNextBreakpoint(true);
94+
logDebugger.setBreakpoints(logPath, [{ line: 10 }]);
95+
logDebugger.gotoBreakpoint(true);
9896
assert.strictEqual(logDebugger.linenum(), 1);
9997
});
10098
});

0 commit comments

Comments
 (0)