debug: breakpoints should give some properties based on session

src/vs/workbench/parts/debug/browser/debugEditorModelManager.ts

@@ -8,7 +8,7 @@ import { Constants } from 'vs/editor/common/core/uint';
 import { Range } from 'vs/editor/common/core/range';
 import { ITextModel, TrackedRangeStickiness, IModelDeltaDecoration, IModelDecorationOptions } from 'vs/editor/common/model';
 import { IWorkbenchContribution } from 'vs/workbench/common/contributions';
-import { IDebugService, IBreakpoint, State } from 'vs/workbench/parts/debug/common/debug';
+import { IDebugService, IBreakpoint, State, IBreakpointUpdateData } from 'vs/workbench/parts/debug/common/debug';
 import { IModelService } from 'vs/editor/common/services/modelService';
 import { MarkdownString } from 'vs/base/common/htmlContent';
 import { getBreakpointMessageAndClassName } from 'vs/workbench/parts/debug/browser/breakpointsView';
@@ -197,7 +197,7 @@ export class DebugEditorModelManager implements IWorkbenchContribution {
 			return;
 		}
 
-		const data: { [id: string]: DebugProtocol.Breakpoint } = Object.create(null);
+		const data: { [id: string]: IBreakpointUpdateData } = Object.create(null);
 		const breakpoints = this.debugService.getModel().getBreakpoints();
 		const modelUri = modelData.model.uri;
 		for (let i = 0, len = modelData.breakpointDecorations.length; i < len; i++) {
@@ -209,9 +209,8 @@ export class DebugEditorModelManager implements IWorkbenchContribution {
 				// since we know it is collapsed, it cannot grow to multiple lines
 				if (breakpoint) {
 					data[breakpoint.getId()] = {
-						line: decorationRange.startLineNumber,
+						lineNumber: decorationRange.startLineNumber,
 						column: breakpoint.column ? decorationRange.startColumn : undefined,
-						verified: breakpoint.verified
 					};
 				}
 			}

src/vs/workbench/parts/debug/common/debug.ts

@@ -243,10 +243,12 @@ export interface IBreakpointData {
 	readonly hitCondition?: string;
 }
 
-export interface IBreakpointUpdateData extends DebugProtocol.Breakpoint {
+export interface IBreakpointUpdateData {
 	readonly condition?: string;
 	readonly hitCondition?: string;
 	readonly logMessage?: string;
+	readonly lineNumber?: number;
+	readonly column?: number;
 }
 
 export interface IBaseBreakpoint extends IEnablement {

src/vs/workbench/parts/debug/common/debugModel.ts

@@ -726,8 +726,8 @@ export class Enablement implements IEnablement {
 
 export class BaseBreakpoint extends Enablement implements IBaseBreakpoint {
 
-	public verified: boolean;
-	public idFromAdapter: number;
+	private sessionData = new Map<string, DebugProtocol.Breakpoint>();
+	private sessionId: string;
 
 	constructor(
 		enabled: boolean,
@@ -740,29 +740,94 @@ export class BaseBreakpoint extends Enablement implements IBaseBreakpoint {
 		if (enabled === undefined) {
 			this.enabled = true;
 		}
-		this.verified = false;
+	}
+
+	protected getSessionData() {
+		return this.sessionData.get(this.sessionId);
+	}
+
+	public setSessionData(sessionId: string, data: DebugProtocol.Breakpoint): void {
+		this.sessionData.set(sessionId, data);
+	}
+
+	public setSessionId(sessionId: string): void {
+		this.sessionId = sessionId;
+	}
+
+	public get verified(): boolean {
+		const data = this.getSessionData();
+		return !!(data && data.verified);
+	}
+
+	public get idFromAdapter(): number {
+		const data = this.getSessionData();
+		return data ? data.id : undefined;
 	}
 }
 
 export class Breakpoint extends BaseBreakpoint implements IBreakpoint {
 
-	public message: string;
-	public endLineNumber: number;
-	public endColumn: number;
-
 	constructor(
 		public uri: uri,
-		public lineNumber: number,
-		public column: number,
+		private _lineNumber: number,
+		private _column: number,
 		enabled: boolean,
 		condition: string,
 		hitCondition: string,
 		logMessage: string,
-		public adapterData: any,
+		private _adapterData: any,
 		id = generateUuid()
 	) {
 		super(enabled, hitCondition, condition, logMessage, id);
 	}
+
+	public get lineNumber(): number {
+		const data = this.getSessionData();
+		return data && typeof data.line === 'number' ? data.line : this._lineNumber;
+	}
+
+	public get column(): number {
+		const data = this.getSessionData();
+		return data && typeof data.column === 'number' ? data.column : this._column;
+	}
+
+	public get message(): string {
+		const data = this.getSessionData();
+		return data ? data.message : undefined;
+	}
+
+	public get adapterData(): any {
+		const data = this.getSessionData();
+		return data && data.source && data.source.adapterData ? data.source.adapterData : this._adapterData;
+	}
+
+	public get endLineNumber(): number {
+		const data = this.getSessionData();
+		return data ? data.endLine : undefined;
+	}
+
+	public get endColumn(): number {
+		const data = this.getSessionData();
+		return data ? data.endColumn : undefined;
+	}
+
+	public update(data: IBreakpointUpdateData): void {
+		if (!isUndefinedOrNull(data.lineNumber)) {
+			this._lineNumber = data.lineNumber;
+		}
+		if (!isUndefinedOrNull(data.column)) {
+			this._column = data.column;
+		}
+		if (!isUndefinedOrNull(data.condition)) {
+			this.condition = data.condition;
+		}
+		if (!isUndefinedOrNull(data.hitCondition)) {
+			this.hitCondition = data.hitCondition;
+		}
+		if (!isUndefinedOrNull(data.logMessage)) {
+			this.logMessage = data.logMessage;
+		}
+	}
 }
 
 export class FunctionBreakpoint extends BaseBreakpoint implements IFunctionBreakpoint {
@@ -893,7 +958,7 @@ export class Model implements IModel {
 		return thread.fetchCallStack();
 	}
 
-	public getBreakpoints(filter?: { uri?: uri, lineNumber?: number, column?: number, enabledOnly?: boolean }): IBreakpoint[] {
+	public getBreakpoints(filter?: { uri?: uri, lineNumber?: number, column?: number, enabledOnly?: boolean }): Breakpoint[] {
 		if (filter) {
 			const uriStr = filter.uri ? filter.uri.toString() : undefined;
 			return this.breakpoints.filter(bp => {
@@ -917,7 +982,7 @@ export class Model implements IModel {
 		return this.breakpoints;
 	}
 
-	public getFunctionBreakpoints(): IFunctionBreakpoint[] {
+	public getFunctionBreakpoints(): FunctionBreakpoint[] {
 		return this.functionBreakpoints;
 	}
 
@@ -967,28 +1032,7 @@ export class Model implements IModel {
 		this.breakpoints.forEach(bp => {
 			const bpData = data[bp.getId()];
 			if (bpData) {
-				if (!isUndefinedOrNull(bpData.line)) {
-					bp.lineNumber = bpData.line;
-				}
-				bp.endLineNumber = bpData.endLine;
-				bp.column = bpData.column;
-				bp.endColumn = bpData.endColumn;
-				if (!isUndefinedOrNull(bpData.verified)) {
-					bp.verified = bpData.verified;
-				}
-				bp.idFromAdapter = bpData.id;
-				bp.message = bpData.message;
-				bp.adapterData = bpData.source ? bpData.source.adapterData : bp.adapterData;
-
-				if (!isUndefinedOrNull(bpData.condition)) {
-					bp.condition = bpData.condition;
-				}
-				if (!isUndefinedOrNull(bpData.hitCondition)) {
-					bp.hitCondition = bpData.hitCondition;
-				}
-				if (!isUndefinedOrNull(bpData.logMessage)) {
-					bp.logMessage = bpData.logMessage;
-				}
+				bp.update(bpData);
 				updated.push(bp);
 			}
 		});
@@ -996,10 +1040,6 @@ export class Model implements IModel {
 		this._onDidChangeBreakpoints.fire({ changed: updated });
 	}
 
-	public unverifyBreakpoints(): void {
-		this.breakpoints.forEach(bp => bp.verified = false);
-	}
-
 	private sortAndDeDup(): void {
 		this.breakpoints = this.breakpoints.sort((first, second) => {
 			if (first.uri.toString() !== second.uri.toString()) {
@@ -1022,17 +1062,12 @@ export class Model implements IModel {
 			}
 
 			element.enabled = enable;
-			if (element instanceof Breakpoint && !element.enabled) {
-				const breakpoint = <Breakpoint>element;
-				breakpoint.verified = false;
-			}
 
 			this._onDidChangeBreakpoints.fire({ changed: changed });
 		}
 	}
 
 	public enableOrDisableAllBreakpoints(enable: boolean): void {
-
 		const changed: (IBreakpoint | IFunctionBreakpoint)[] = [];
 
 		this.breakpoints.forEach(bp => {
@@ -1040,9 +1075,6 @@ export class Model implements IModel {
 				changed.push(bp);
 			}
 			bp.enabled = enable;
-			if (!enable) {
-				bp.verified = false;
-			}
 		});
 		this.functionBreakpoints.forEach(fbp => {
 			if (fbp.enabled !== enable) {
@@ -1062,23 +1094,12 @@ export class Model implements IModel {
 		return newFunctionBreakpoint;
 	}
 
-	public updateFunctionBreakpoints(data: { [id: string]: { name?: string, verified?: boolean; id?: number; hitCondition?: string } }): void {
-
-		const changed: IFunctionBreakpoint[] = [];
-
-		this.functionBreakpoints.forEach(fbp => {
-			const fbpData = data[fbp.getId()];
-			if (fbpData) {
-				fbp.name = fbpData.name || fbp.name;
-				fbp.verified = fbpData.verified;
-				fbp.idFromAdapter = fbpData.id;
-				fbp.hitCondition = fbpData.hitCondition;
-
-				changed.push(fbp);
-			}
-		});
-
-		this._onDidChangeBreakpoints.fire({ changed: changed });
+	public renameFunctionBreakpoint(id: string, name: string): void {
+		const functionBreakpoint = this.functionBreakpoints.filter(fbp => fbp.getId() === id).pop();
+		if (functionBreakpoint) {
+			functionBreakpoint.name = name;
+			this._onDidChangeBreakpoints.fire({ changed: [functionBreakpoint] });
+		}
 	}
 
 	public removeFunctionBreakpoints(id?: string): void {

src/vs/workbench/parts/debug/electron-browser/breakpointWidget.ts

@@ -173,8 +173,6 @@ export class BreakpointWidget extends ZoneWidget implements IPrivateBreakpointWi
 					[this.breakpoint.getId()]: {
 						condition,
 						hitCondition,
-						verified: this.breakpoint.verified,
-						column: this.breakpoint.column,
 						logMessage
 					}
 				}, false);

src/vs/workbench/parts/debug/electron-browser/debugService.ts

@@ -425,10 +425,10 @@ export class DebugService implements debug.IDebugService {
 					if (!breakpoint.column) {
 						event.body.breakpoint.column = undefined;
 					}
-					this.model.updateBreakpoints({ [breakpoint.getId()]: event.body.breakpoint });
+					breakpoint.setSessionData(session.getId(), event.body.breakpoint);
 				}
 				if (functionBreakpoint) {
-					this.model.updateFunctionBreakpoints({ [functionBreakpoint.getId()]: event.body.breakpoint });
+					functionBreakpoint.setSessionData(session.getId(), event.body.breakpoint);
 				}
 			}
 		}));
@@ -646,7 +646,7 @@ export class DebugService implements debug.IDebugService {
 	}
 
 	public renameFunctionBreakpoint(id: string, newFunctionName: string): TPromise<void> {
-		this.model.updateFunctionBreakpoints({ [id]: { name: newFunctionName } });
+		this.model.renameFunctionBreakpoint(id, newFunctionName);
 		return this.sendFunctionBreakpoints();
 	}
 
@@ -706,7 +706,6 @@ export class DebugService implements debug.IDebugService {
 				if (this.model.getSessions().length === 0) {
 					this.removeReplExpressions();
 					this.allSessions.clear();
-					this.model.unverifyBreakpoints();
 				}
 
 				let config: debug.IConfig, compound: debug.ICompound;
@@ -1184,13 +1183,6 @@ export class DebugService implements debug.IDebugService {
 		this.updateStateAndEmit(raw.getId(), debug.State.Inactive);
 
 		if (this.model.getSessions().length === 0) {
-			// set breakpoints back to unverified since the session ended.
-			const data: { [id: string]: { line: number, verified: boolean, column: number, endLine: number, endColumn: number } } = {};
-			breakpoints.forEach(bp => {
-				data[bp.getId()] = { line: bp.lineNumber, verified: false, column: bp.column, endLine: bp.endLineNumber, endColumn: bp.endColumn };
-			});
-			this.model.updateBreakpoints(data);
-
 			this.inDebugMode.reset();
 			this.debugType.reset();
 			this.viewModel.setMultiSessionView(false);
@@ -1255,16 +1247,9 @@ export class DebugService implements debug.IDebugService {
 					return;
 				}
 
-				const data: { [id: string]: DebugProtocol.Breakpoint } = {};
 				for (let i = 0; i < breakpointsToSend.length; i++) {
-					data[breakpointsToSend[i].getId()] = response.body.breakpoints[i];
-					if (!breakpointsToSend[i].column) {
-						// If there was no column sent ignore the breakpoint column response from the adapter
-						data[breakpointsToSend[i].getId()].column = undefined;
-					}
+					breakpointsToSend[i].setSessionData(raw.getId(), response.body.breakpoints[i]);
 				}
-
-				this.model.updateBreakpoints(data);
 			});
 		};
 
@@ -1284,12 +1269,9 @@ export class DebugService implements debug.IDebugService {
 					return;
 				}
 
-				const data: { [id: string]: { name?: string, verified?: boolean } } = {};
 				for (let i = 0; i < breakpointsToSend.length; i++) {
-					data[breakpointsToSend[i].getId()] = response.body.breakpoints[i];
+					breakpointsToSend[i].setSessionData(raw.getId(), response.body.breakpoints[i]);
 				}
-
-				this.model.updateFunctionBreakpoints(data);
 			});
 		};
 
@@ -1334,6 +1316,7 @@ export class DebugService implements debug.IDebugService {
 	}
 
 	private store(): void {
+		// TODO@Isidor proper storage of breakpoints
 		const breakpoints = this.model.getBreakpoints();
 		if (breakpoints.length) {
 			this.storageService.store(DEBUG_BREAKPOINTS_KEY, JSON.stringify(breakpoints), StorageScope.WORKSPACE);

src/vs/workbench/parts/debug/test/node/debugModel.test.ts

@@ -59,10 +59,9 @@ suite('Debug - Model', () => {
 		assert.equal(model.getBreakpoints().length, 5);
 		const bp = model.getBreakpoints()[0];
 		const update: any = {};
-		update[bp.getId()] = { line: 100, verified: true };
+		update[bp.getId()] = { lineNumber: 100 };
 		model.updateBreakpoints(update);
 		assert.equal(bp.lineNumber, 100);
-		assert.equal(bp.verified, true);
 
 		model.enableOrDisableAllBreakpoints(false);
 		model.getBreakpoints().forEach(bp => {
@@ -73,9 +72,6 @@ suite('Debug - Model', () => {
 
 		model.removeBreakpoints(model.getBreakpoints({ uri: modelUri1 }));
 		assert.equal(model.getBreakpoints().length, 3);
-
-		model.unverifyBreakpoints();
-		model.getBreakpoints().forEach(bp => assert.equal(bp.verified, false));
 	});
 
 	test('breakpoints conditions', () => {
@@ -96,16 +92,12 @@ suite('Debug - Model', () => {
 	test('function brekapoints', () => {
 		model.addFunctionBreakpoint('foo', '1');
 		model.addFunctionBreakpoint('bar', '2');
-		model.updateFunctionBreakpoints({
-			'1': { name: 'fooUpdated', verified: true, hitCondition: '5' },
-			'2': { name: 'barUpdated', verified: false }
-		});
+		model.renameFunctionBreakpoint('1', 'fooUpdated');
+		model.renameFunctionBreakpoint('2', 'barUpdated');
+
 		const functionBps = model.getFunctionBreakpoints();
 		assert.equal(functionBps[0].name, 'fooUpdated');
-		assert.equal(functionBps[0].verified, true);
-		assert.equal(functionBps[0].hitCondition, '5');
 		assert.equal(functionBps[1].name, 'barUpdated');
-		assert.equal(functionBps[1].verified, false);
 
 		model.removeFunctionBreakpoints();
 		assert.equal(model.getFunctionBreakpoints().length, 0);