debug: proper handling of output objects

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

@@ -57,7 +57,6 @@ export interface ITreeElement {
 }
 
 export interface IExpressionContainer extends ITreeElement {
-	stackFrame: IStackFrame;
 	hasChildren: boolean;
 	getChildren(): TPromise<IExpression[]>;
 }
@@ -65,7 +64,7 @@ export interface IExpressionContainer extends ITreeElement {
 export interface IExpression extends ITreeElement, IExpressionContainer {
 	name: string;
 	value: string;
-	valueChanged: boolean;
+	valueChanged?: boolean;
 	type?: string;
 }
 

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

@@ -49,7 +49,7 @@ export class OutputElement extends AbstractOutputElement {
 	}
 }
 
-export class OutputNameValueElement extends AbstractOutputElement {
+export class OutputNameValueElement extends AbstractOutputElement implements debug.IExpression {
 
 	private static MAX_CHILDREN = 1000; // upper bound of children per value
 
@@ -71,20 +71,25 @@ export class OutputNameValueElement extends AbstractOutputElement {
 		return String(this.valueObj) || '';
 	}
 
-	public getChildren(): debug.ITreeElement[] {
+	public get hasChildren(): boolean {
+		return Array.isArray(this.valueObj) || isObject(this.valueObj);
+	}
+
+	public getChildren(): TPromise<debug.IExpression[]> {
+		let result: debug.IExpression[] = [];
 		if (Array.isArray(this.valueObj)) {
-			return (<any[]>this.valueObj).slice(0, OutputNameValueElement.MAX_CHILDREN)
+			result = (<any[]>this.valueObj).slice(0, OutputNameValueElement.MAX_CHILDREN)
 				.map((v, index) => new OutputNameValueElement(String(index), v));
 		} else if (isObject(this.valueObj)) {
-			return Object.getOwnPropertyNames(this.valueObj).slice(0, OutputNameValueElement.MAX_CHILDREN)
+			result = Object.getOwnPropertyNames(this.valueObj).slice(0, OutputNameValueElement.MAX_CHILDREN)
 				.map(key => new OutputNameValueElement(key, this.valueObj[key]));
 		}
 
-		return [];
+		return TPromise.as(result);
 	}
 }
 
-export abstract class ExpressionContainer implements debug.IExpressionContainer {
+export class ExpressionContainer implements debug.IExpressionContainer {
 
 	public static allValues: { [id: string]: string } = {};
 	// Use chunks to support variable paging #9537
@@ -94,7 +99,7 @@ export abstract class ExpressionContainer implements debug.IExpressionContainer
 	private _value: string;
 
 	constructor(
-		public stackFrame: debug.IStackFrame,
+		protected process: debug.IProcess,
 		public reference: number,
 		private id: string,
 		public namedVariables = 0,
@@ -125,7 +130,7 @@ export abstract class ExpressionContainer implements debug.IExpressionContainer
 				for (let i = 0; i < numberOfChunks; i++) {
 					const start = this.startOfVariables + i * chunkSize;
 					const count = Math.min(chunkSize, this.indexedVariables - i * chunkSize);
-					childrenArray.push(new Variable(this.stackFrame, this, this.reference, `[${start}..${start + count - 1}]`, '', '', null, count, null, true, start));
+					childrenArray.push(new Variable(this.process, this, this.reference, `[${start}..${start + count - 1}]`, '', '', null, count, null, true, start));
 				}
 
 				return childrenArray;
@@ -150,16 +155,16 @@ export abstract class ExpressionContainer implements debug.IExpressionContainer
 	}
 
 	private fetchVariables(start: number, count: number, filter: 'indexed' | 'named'): TPromise<Variable[]> {
-		return this.stackFrame.thread.process.session.variables({
+		return this.process.session.variables({
 			variablesReference: this.reference,
 			start,
 			count,
 			filter
 		}).then(response => {
 			return response && response.body && response.body.variables ? distinct(response.body.variables.filter(v => !!v), v => v.name).map(
-				v => new Variable(this.stackFrame, this, v.variablesReference, v.name, v.evaluateName, v.value, v.namedVariables, v.indexedVariables, v.type)
+				v => new Variable(this.process, this, v.variablesReference, v.name, v.evaluateName, v.value, v.namedVariables, v.indexedVariables, v.type)
 			) : [];
-		}, (e: Error) => [new Variable(this.stackFrame, this, 0, null, e.message, '', 0, 0, null, false)]);
+		}, (e: Error) => [new Variable(this.process, this, 0, null, e.message, '', 0, 0, null, false)]);
 	}
 
 	// The adapter explicitly sents the children count of an expression only if there are lots of children which should be chunked.
@@ -175,12 +180,6 @@ export abstract class ExpressionContainer implements debug.IExpressionContainer
 	}
 }
 
-export class OutputExpressionContainer extends ExpressionContainer {
-	constructor(public name: string, stackFrame: debug.IStackFrame, reference: number, public annotation = null) {
-		super(stackFrame, reference, generateUuid());
-	}
-}
-
 export class Expression extends ExpressionContainer implements debug.IExpression {
 	static DEFAULT_VALUE = nls.localize('notAvailable', "not available");
 
@@ -206,10 +205,7 @@ export class Expression extends ExpressionContainer implements debug.IExpression
 			return TPromise.as(null);
 		}
 
-		// Create a fake stack frame which is just used as a container for the process.
-		// TODO@Isidor revisit if variables should have a reference to the StackFrame or a process after all
-		this.stackFrame = stackFrame || new StackFrame(new Thread(process, undefined, undefined), undefined, undefined, undefined, undefined, undefined);
-
+		this.process = process;
 		return process.session.evaluate({
 			expression: this.name,
 			frameId: stackFrame ? stackFrame.frameId : undefined,
@@ -239,7 +235,7 @@ export class Variable extends ExpressionContainer implements debug.IExpression {
 	private static ARRAY_ELEMENT_SYNTAX = /\[.*\]$/;
 
 	constructor(
-		stackFrame: debug.IStackFrame,
+		process: debug.IProcess,
 		public parent: debug.IExpressionContainer,
 		reference: number,
 		public name: string,
@@ -251,7 +247,7 @@ export class Variable extends ExpressionContainer implements debug.IExpression {
 		public available = true,
 		startOfVariables = 0
 	) {
-		super(stackFrame, reference, `variable:${parent.getId()}:${name}:${reference}`, namedVariables, indexedVariables, startOfVariables);
+		super(process, reference, `variable:${parent.getId()}:${name}:${reference}`, namedVariables, indexedVariables, startOfVariables);
 		this.value = value;
 	}
 
@@ -260,6 +256,7 @@ export class Variable extends ExpressionContainer implements debug.IExpression {
 			return this._evaluateName;
 		}
 
+		// TODO@Isidor get rid of this ugly heuristic
 		let names = [this.name];
 		let v = this.parent;
 		while (v instanceof Variable || v instanceof Expression) {
@@ -272,7 +269,7 @@ export class Variable extends ExpressionContainer implements debug.IExpression {
 		names.forEach(name => {
 			if (!result) {
 				result = name;
-			} else if (Variable.ARRAY_ELEMENT_SYNTAX.test(name) || (this.stackFrame.thread.process.session.configuration.type === 'node' && !Variable.NOT_PROPERTY_SYNTAX.test(name))) {
+			} else if (Variable.ARRAY_ELEMENT_SYNTAX.test(name) || (this.process.session.configuration.type === 'node' && !Variable.NOT_PROPERTY_SYNTAX.test(name))) {
 				// use safe way to access node properties a['property_name']. Also handles array elements.
 				result = name && name.indexOf('[') === 0 ? `${result}${name}` : `${result}['${name}']`;
 			} else {
@@ -284,7 +281,7 @@ export class Variable extends ExpressionContainer implements debug.IExpression {
 	}
 
 	public setVariable(value: string): TPromise<any> {
-		return this.stackFrame.thread.process.session.setVariable({
+		return this.process.session.setVariable({
 			name: this.name,
 			value,
 			variablesReference: (<ExpressionContainer>this.parent).reference
@@ -313,7 +310,7 @@ export class Scope extends ExpressionContainer implements debug.IScope {
 		indexedVariables: number,
 		public range?: IRange
 	) {
-		super(stackFrame, reference, `scope:${stackFrame.getId()}:${name}:${reference}`, namedVariables, indexedVariables);
+		super(stackFrame.thread.process, reference, `scope:${stackFrame.getId()}:${name}:${reference}`, namedVariables, indexedVariables);
 	}
 }
 
@@ -820,7 +817,7 @@ export class Model implements debug.IModel {
 			.then(() => this._onDidChangeREPLElements.fire());
 	}
 
-	public appendReplOutput(output: OutputElement | OutputExpressionContainer | OutputNameValueElement): void {
+	public appendReplOutput(output: OutputElement | debug.IExpression): void {
 		const previousOutput = this.replElements.length && (this.replElements[this.replElements.length - 1] as OutputElement);
 		const groupTogether = output instanceof OutputElement && previousOutput instanceof OutputElement && output.severity === previousOutput.severity;
 		if (groupTogether) {

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

@@ -36,7 +36,7 @@ import { IEditorGroupService } from 'vs/workbench/services/group/common/groupSer
 import { asFileEditorInput } from 'vs/workbench/common/editor';
 import * as debug from 'vs/workbench/parts/debug/common/debug';
 import { RawDebugSession } from 'vs/workbench/parts/debug/electron-browser/rawDebugSession';
-import { Model, ExceptionBreakpoint, FunctionBreakpoint, Breakpoint, Expression, OutputElement, OutputExpressionContainer, OutputNameValueElement } from 'vs/workbench/parts/debug/common/debugModel';
+import { Model, ExceptionBreakpoint, FunctionBreakpoint, Breakpoint, Expression, OutputElement, OutputNameValueElement, ExpressionContainer, Process } from 'vs/workbench/parts/debug/common/debugModel';
 import { DebugStringEditorInput, DebugErrorEditorInput } from 'vs/workbench/parts/debug/browser/debugEditorInputs';
 import { ViewModel } from 'vs/workbench/parts/debug/common/debugViewModel';
 import * as debugactions from 'vs/workbench/parts/debug/browser/debugActions';
@@ -234,7 +234,7 @@ export class DebugService implements debug.IDebugService {
 		}
 	}
 
-	private registerSessionListeners(session: RawDebugSession): void {
+	private registerSessionListeners(process: Process, session: RawDebugSession): void {
 		this.toDisposeOnSessionEnd[session.getId()].push(session);
 		this.toDisposeOnSessionEnd[session.getId()].push(session.onDidInitialize(event => {
 			aria.status(nls.localize('debuggingStarted', "Debugging started."));
@@ -250,7 +250,6 @@ export class DebugService implements debug.IDebugService {
 				}
 			};
 
-			const process = this.model.getProcesses().filter(p => p.getId() === session.getId()).pop();
 			this.sendAllBreakpoints(process).then(sendConfigurationDone, sendConfigurationDone)
 				.done(() => this.fetchThreads(session), errors.onUnexpectedError);
 		}));
@@ -273,7 +272,6 @@ export class DebugService implements debug.IDebugService {
 					allThreadsStopped: event.body.allThreadsStopped
 				});
 
-				const process = this.model.getProcesses().filter(p => p.getId() === session.getId()).pop();
 				const thread = process && process.getThread(threadId);
 				if (thread) {
 					thread.fetchCallStack().then(callStack => {
@@ -305,7 +303,6 @@ export class DebugService implements debug.IDebugService {
 			aria.status(nls.localize('debuggingStopped', "Debugging stopped."));
 			if (session && session.getId() === event.body.sessionId) {
 				if (event.body && typeof event.body.restart === 'boolean' && event.body.restart) {
-					const process = this.model.getProcesses().filter(p => p.getId() === session.getId()).pop();
 					this.restartProcess(process).done(null, err => this.messageService.show(severity.Error, err.message));
 				} else {
 					session.disconnect().done(null, errors.onUnexpectedError);
@@ -325,10 +322,19 @@ export class DebugService implements debug.IDebugService {
 					this.customTelemetryService.publicLog(event.body.output, event.body.data);
 				}
 			} else if (event.body && typeof event.body.output === 'string' && event.body.output.length > 0) {
-				const outputSeverity = event.body.category === 'stderr' ? severity.Error : event.body.category === 'console' ? severity.Warning : severity.Info;
-				const value = event.body.variablesReference ? new OutputExpressionContainer(event.body.output, this.viewModel.focusedStackFrame, event.body.variablesReference)
-					: new OutputElement(event.body.output, outputSeverity);
-				this.model.appendReplOutput(value);
+				if (event.body.variablesReference) {
+					const container = new ExpressionContainer(process, event.body.variablesReference, generateUuid());
+					container.getChildren().then(children => {
+						children.forEach(child => {
+							// Since we can not display multiple trees in a row, we are displaying these variables one after the other (ignoring their names)
+							child.name = null;
+							this.model.appendReplOutput(child);
+						});
+					});
+				} else {
+					const outputSeverity = event.body.category === 'stderr' ? severity.Error : event.body.category === 'console' ? severity.Warning : severity.Info;
+					this.model.appendReplOutput(new OutputElement(event.body.output, outputSeverity));
+				}
 			}
 		}));
 
@@ -638,7 +644,7 @@ export class DebugService implements debug.IDebugService {
 			if (client) {
 				this.toDisposeOnSessionEnd[session.getId()].push(client);
 			}
-			this.registerSessionListeners(session);
+			this.registerSessionListeners(process, session);
 
 			return session.initialize({
 				adapterID: configuration.type,

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

@@ -85,7 +85,7 @@ export function renderVariable(tree: ITree, variable: Variable, data: IVariableT
 	}
 
 	if (variable.value) {
-		data.name.textContent += ':';
+		data.name.textContent += variable.name ? ':' : '';
 		renderExpressionValue(variable, data.value, {
 			showChanged,
 			maxValueLength: MAX_VALUE_RENDER_LENGTH_IN_VIEWLET,

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

@@ -20,7 +20,7 @@ import { IActionProvider } from 'vs/base/parts/tree/browser/actionsRenderer';
 import { ICancelableEvent } from 'vs/base/parts/tree/browser/treeDefaults';
 import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
 import { IExpressionContainer, IExpression, IDebugService } from 'vs/workbench/parts/debug/common/debug';
-import { Model, OutputNameValueElement, Expression, OutputElement, Variable, OutputExpressionContainer } from 'vs/workbench/parts/debug/common/debugModel';
+import { Model, OutputNameValueElement, Expression, OutputElement, Variable } from 'vs/workbench/parts/debug/common/debugModel';
 import { renderVariable, renderExpressionValue, IVariableTemplateData, BaseDebugController } from 'vs/workbench/parts/debug/electron-browser/debugViewer';
 import { AddToWatchExpressionsAction, ClearReplAction } from 'vs/workbench/parts/debug/browser/debugActions';
 import { CopyAction } from 'vs/workbench/parts/debug/electron-browser/electronDebugActions';
@@ -150,7 +150,7 @@ export class ReplExpressionsRenderer implements IRenderer {
 		if (element instanceof OutputElement) {
 			return ReplExpressionsRenderer.VALUE_OUTPUT_TEMPLATE_ID;
 		}
-		if (element instanceof OutputNameValueElement || element instanceof OutputExpressionContainer) {
+		if (element instanceof OutputNameValueElement) {
 			return ReplExpressionsRenderer.NAME_VALUE_OUTPUT_TEMPLATE_ID;
 		}
 
@@ -251,6 +251,30 @@ export class ReplExpressionsRenderer implements IRenderer {
 		templateData.value.className = (output.severity === severity.Warning) ? 'warn' : (output.severity === severity.Error) ? 'error' : 'info';
 	}
 
+	private renderOutputNameValue(tree: ITree, output: OutputNameValueElement, templateData: IKeyValueOutputTemplateData): void {
+		// key
+		if (output.name) {
+			templateData.name.textContent = `${output.name}:`;
+		} else {
+			templateData.name.textContent = '';
+		}
+
+		// value
+		renderExpressionValue(output.value, templateData.value, {
+			showChanged: false,
+			preserveWhitespace: true
+		});
+
+		// annotation if any
+		if (output.annotation) {
+			templateData.annotation.className = 'annotation octicon octicon-info';
+			templateData.annotation.title = output.annotation;
+		} else {
+			templateData.annotation.className = '';
+			templateData.annotation.title = '';
+		}
+	}
+
 	private handleANSIOutput(text: string): HTMLElement | string {
 		let tokensContainer: HTMLSpanElement;
 		let currentToken: HTMLSpanElement;
@@ -401,31 +425,6 @@ export class ReplExpressionsRenderer implements IRenderer {
 		}, event.ctrlKey || event.metaKey).done(null, errors.onUnexpectedError);
 	}
 
-	private renderOutputNameValue(tree: ITree, output: OutputNameValueElement | OutputExpressionContainer, templateData: IKeyValueOutputTemplateData): void {
-
-		// key
-		if (output.name) {
-			templateData.name.textContent = `${output.name}:`;
-		} else {
-			templateData.name.textContent = '';
-		}
-
-		// value
-		renderExpressionValue(output.value, templateData.value, {
-			showChanged: false,
-			preserveWhitespace: true
-		});
-
-		// annotation if any
-		if (output.annotation) {
-			templateData.annotation.className = 'annotation octicon octicon-info';
-			templateData.annotation.title = output.annotation;
-		} else {
-			templateData.annotation.className = '';
-			templateData.annotation.title = '';
-		}
-	}
-
 	public disposeTemplate(tree: ITree, templateId: string, templateData: any): void {
 		// noop
 	}
@@ -503,7 +502,7 @@ export class ReplExpressionsController extends BaseDebugController {
 	protected onLeftClick(tree: ITree, element: any, eventish: ICancelableEvent, origin: string = 'mouse'): boolean {
 		const mouseEvent = <IMouseEvent>eventish;
 		// input and output are one element in the tree => we only expand if the user clicked on the output.
-		if ((element.reference > 0 || (element instanceof OutputNameValueElement && element.getChildren().length > 0)) && mouseEvent.target.className.indexOf('input expression') === -1) {
+		if ((element.reference > 0 || (element instanceof OutputNameValueElement && element.hasChildren)) && mouseEvent.target.className.indexOf('input expression') === -1) {
 			super.onLeftClick(tree, element, eventish, origin);
 			tree.clearFocus();
 			tree.deselect(element);