From 0e17c435addec4957ed81dcb5e4b51335eb86f72 Mon Sep 17 00:00:00 2001 From: isidor Date: Tue, 22 Nov 2016 15:46:15 +0100 Subject: [PATCH] replViewer: renaming and polishing --- src/vs/workbench/parts/debug/common/debug.ts | 2 +- .../parts/debug/common/debugModel.ts | 28 ++++----- .../debug/electron-browser/debugHover.ts | 2 +- .../parts/debug/electron-browser/repl.ts | 2 +- .../debug/electron-browser/replViewer.ts | 62 +++++++++---------- .../parts/debug/test/node/debugModel.test.ts | 6 +- 6 files changed, 49 insertions(+), 53 deletions(-) diff --git a/src/vs/workbench/parts/debug/common/debug.ts b/src/vs/workbench/parts/debug/common/debug.ts index 752ce17beb1..9efd4ae564c 100644 --- a/src/vs/workbench/parts/debug/common/debug.ts +++ b/src/vs/workbench/parts/debug/common/debug.ts @@ -59,7 +59,7 @@ export interface ITreeElement { export interface IExpressionContainer extends ITreeElement { stackFrame: IStackFrame; hasChildren: boolean; - getChildren(debugService: IDebugService): TPromise; + getChildren(): TPromise; } export interface IExpression extends ITreeElement, IExpressionContainer { diff --git a/src/vs/workbench/parts/debug/common/debugModel.ts b/src/vs/workbench/parts/debug/common/debugModel.ts index 29d29838739..e918971763d 100644 --- a/src/vs/workbench/parts/debug/common/debugModel.ts +++ b/src/vs/workbench/parts/debug/common/debugModel.ts @@ -24,10 +24,10 @@ import { Source } from 'vs/workbench/parts/debug/common/debugSource'; const MAX_REPL_LENGTH = 10000; const UNKNOWN_SOURCE_LABEL = nls.localize('unknownSource', "Unknown Source"); -export class OutputElement implements debug.ITreeElement { +export abstract class AbstractOutputElement implements debug.ITreeElement { private static ID_COUNTER = 0; - constructor(private id = OutputElement.ID_COUNTER++) { + constructor(private id = AbstractOutputElement.ID_COUNTER++) { // noop } @@ -36,7 +36,7 @@ export class OutputElement implements debug.ITreeElement { } } -export class ValueOutputElement extends OutputElement { +export class ValueOutputElement extends AbstractOutputElement { constructor( public value: string, @@ -47,11 +47,11 @@ export class ValueOutputElement extends OutputElement { } } -export class KeyValueOutputElement extends OutputElement { +export class NameValueOutputElement extends AbstractOutputElement { private static MAX_CHILDREN = 1000; // upper bound of children per value - constructor(public key: string, public valueObj: any, public annotation?: string) { + constructor(public name: string, public valueObj: any, public annotation?: string) { super(); } @@ -71,11 +71,11 @@ export class KeyValueOutputElement extends OutputElement { public getChildren(): debug.ITreeElement[] { if (Array.isArray(this.valueObj)) { - return (this.valueObj).slice(0, KeyValueOutputElement.MAX_CHILDREN) - .map((v, index) => new KeyValueOutputElement(String(index), v)); + return (this.valueObj).slice(0, NameValueOutputElement.MAX_CHILDREN) + .map((v, index) => new NameValueOutputElement(String(index), v)); } else if (isObject(this.valueObj)) { - return Object.getOwnPropertyNames(this.valueObj).slice(0, KeyValueOutputElement.MAX_CHILDREN) - .map(key => new KeyValueOutputElement(key, this.valueObj[key])); + return Object.getOwnPropertyNames(this.valueObj).slice(0, NameValueOutputElement.MAX_CHILDREN) + .map(key => new NameValueOutputElement(key, this.valueObj[key])); } return []; @@ -95,14 +95,13 @@ export abstract class ExpressionContainer implements debug.IExpressionContainer public stackFrame: debug.IStackFrame, public reference: number, private id: string, - public namedVariables: number, - public indexedVariables: number, + public namedVariables = 0, + public indexedVariables = 0, private startOfVariables = 0 ) { } public getChildren(): TPromise { - // only variables with reference > 0 have children. - if (this.reference <= 0) { + if (!this.hasChildren) { return TPromise.as([]); } @@ -144,6 +143,7 @@ export abstract class ExpressionContainer implements debug.IExpressionContainer } public get hasChildren(): boolean { + // only variables with reference > 0 have children. return this.reference > 0; } @@ -180,7 +180,7 @@ export class Expression extends ExpressionContainer implements debug.IExpression public type: string; constructor(public name: string, id = generateUuid()) { - super(null, 0, id, 0, 0); + super(null, 0, id); this.available = false; // name is not set if the expression is just being added // in that case do not set default value to prevent flashing #14499 diff --git a/src/vs/workbench/parts/debug/electron-browser/debugHover.ts b/src/vs/workbench/parts/debug/electron-browser/debugHover.ts index 7ab16ecf3c4..a8f0b932d57 100644 --- a/src/vs/workbench/parts/debug/electron-browser/debugHover.ts +++ b/src/vs/workbench/parts/debug/electron-browser/debugHover.ts @@ -190,7 +190,7 @@ export class DebugHoverWidget implements IContentWidget { } private doFindExpression(container: IExpressionContainer, namesToFind: string[]): TPromise { - return container.getChildren(this.debugService).then(children => { + return container.getChildren().then(children => { // look for our variable in the list. First find the parents of the hovered variable if there are any. // some languages pass the type as part of the name, so need to check if the last word of the name matches. const filtered = children.filter(v => typeof v.name === 'string' && (namesToFind[0] === v.name || namesToFind[0] === v.name.substr(v.name.lastIndexOf(' ') + 1))); diff --git a/src/vs/workbench/parts/debug/electron-browser/repl.ts b/src/vs/workbench/parts/debug/electron-browser/repl.ts index b5993dadc7b..6854bb4b0b1 100644 --- a/src/vs/workbench/parts/debug/electron-browser/repl.ts +++ b/src/vs/workbench/parts/debug/electron-browser/repl.ts @@ -140,7 +140,7 @@ export class Repl extends Panel implements IPrivateReplService { this.renderer = this.instantiationService.createInstance(ReplExpressionsRenderer); this.tree = new Tree(this.treeContainer, { - dataSource: new ReplExpressionsDataSource(this.debugService), + dataSource: new ReplExpressionsDataSource(), renderer: this.renderer, accessibilityProvider: new ReplExpressionsAccessibilityProvider(), controller: new ReplExpressionsController(this.debugService, this.contextMenuService, new ReplExpressionsActionProvider(this.instantiationService), this.replInput, false) diff --git a/src/vs/workbench/parts/debug/electron-browser/replViewer.ts b/src/vs/workbench/parts/debug/electron-browser/replViewer.ts index 8027cc846a3..ddf65b2358c 100644 --- a/src/vs/workbench/parts/debug/electron-browser/replViewer.ts +++ b/src/vs/workbench/parts/debug/electron-browser/replViewer.ts @@ -19,8 +19,8 @@ import { ITree, IAccessibilityProvider, IDataSource, IRenderer } from 'vs/base/p 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 * as debug from 'vs/workbench/parts/debug/common/debug'; -import { Model, KeyValueOutputElement, Expression, ValueOutputElement, Variable } from 'vs/workbench/parts/debug/common/debugModel'; +import { IExpressionContainer, IExpression, IDebugService } from 'vs/workbench/parts/debug/common/debug'; +import { Model, NameValueOutputElement, Expression, ValueOutputElement, 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'; @@ -33,30 +33,26 @@ const $ = dom.$; export class ReplExpressionsDataSource implements IDataSource { - constructor(private debugService: debug.IDebugService) { - // noop - } - public getId(tree: ITree, element: any): string { return element.getId(); } public hasChildren(tree: ITree, element: any): boolean { - return element instanceof Model || element.reference > 0 || (element instanceof KeyValueOutputElement && element.getChildren().length > 0); + return element instanceof Model || (element).hasChildren || element instanceof NameValueOutputElement; } public getChildren(tree: ITree, element: any): TPromise { if (element instanceof Model) { return TPromise.as(element.getReplElements()); } - if (element instanceof KeyValueOutputElement) { + if (element instanceof NameValueOutputElement) { return TPromise.as(element.getChildren()); } if (element instanceof ValueOutputElement) { return TPromise.as(null); } - return (element).getChildren(this.debugService); + return (element).getChildren(); } public getParent(tree: ITree, element: any): TPromise { @@ -64,7 +60,7 @@ export class ReplExpressionsDataSource implements IDataSource { } } -interface IInputOutputPairTemplateData { +interface IExpressionTemplateData { input: HTMLElement; output: HTMLElement; value: HTMLElement; @@ -80,7 +76,7 @@ interface IValueOutputTemplateData { interface IKeyValueOutputTemplateData { container: HTMLElement; expression: HTMLElement; - key: HTMLElement; + name: HTMLElement; value: HTMLElement; annotation: HTMLElement; } @@ -88,9 +84,9 @@ interface IKeyValueOutputTemplateData { export class ReplExpressionsRenderer implements IRenderer { private static VARIABLE_TEMPLATE_ID = 'variable'; - private static INPUT_OUTPUT_PAIR_TEMPLATE_ID = 'inputOutputPair'; + private static EXPRESSION_TEMPLATE_ID = 'inputOutputPair'; private static VALUE_OUTPUT_TEMPLATE_ID = 'outputValue'; - private static KEY_VALUE_OUTPUT_TEMPLATE_ID = 'outputKeyValue'; + private static NAME_VALUE_OUTPUT_TEMPLATE_ID = 'outputKeyValue'; private static FILE_LOCATION_PATTERNS: RegExp[] = [ // group 0: the full thing :) @@ -145,13 +141,13 @@ export class ReplExpressionsRenderer implements IRenderer { return ReplExpressionsRenderer.VARIABLE_TEMPLATE_ID; } if (element instanceof Expression) { - return ReplExpressionsRenderer.INPUT_OUTPUT_PAIR_TEMPLATE_ID; + return ReplExpressionsRenderer.EXPRESSION_TEMPLATE_ID; } if (element instanceof ValueOutputElement) { return ReplExpressionsRenderer.VALUE_OUTPUT_TEMPLATE_ID; } - if (element instanceof KeyValueOutputElement) { - return ReplExpressionsRenderer.KEY_VALUE_OUTPUT_TEMPLATE_ID; + if (element instanceof NameValueOutputElement) { + return ReplExpressionsRenderer.NAME_VALUE_OUTPUT_TEMPLATE_ID; } return null; @@ -167,8 +163,8 @@ export class ReplExpressionsRenderer implements IRenderer { return data; } - if (templateId === ReplExpressionsRenderer.INPUT_OUTPUT_PAIR_TEMPLATE_ID) { - let data: IInputOutputPairTemplateData = Object.create(null); + if (templateId === ReplExpressionsRenderer.EXPRESSION_TEMPLATE_ID) { + let data: IExpressionTemplateData = Object.create(null); dom.addClass(container, 'input-output-pair'); data.input = dom.append(container, $('.input.expression')); data.output = dom.append(container, $('.output.expression')); @@ -190,13 +186,13 @@ export class ReplExpressionsRenderer implements IRenderer { return data; } - if (templateId === ReplExpressionsRenderer.KEY_VALUE_OUTPUT_TEMPLATE_ID) { + if (templateId === ReplExpressionsRenderer.NAME_VALUE_OUTPUT_TEMPLATE_ID) { let data: IKeyValueOutputTemplateData = Object.create(null); dom.addClass(container, 'output'); data.container = container; data.expression = dom.append(container, $('.output.expression')); - data.key = dom.append(data.expression, $('span.name')); + data.name = dom.append(data.expression, $('span.name')); data.value = dom.append(data.expression, $('span.value')); data.annotation = dom.append(data.expression, $('span')); @@ -207,16 +203,16 @@ export class ReplExpressionsRenderer implements IRenderer { public renderElement(tree: ITree, element: any, templateId: string, templateData: any): void { if (templateId === ReplExpressionsRenderer.VARIABLE_TEMPLATE_ID) { renderVariable(tree, element, templateData, false); - } else if (templateId === ReplExpressionsRenderer.INPUT_OUTPUT_PAIR_TEMPLATE_ID) { - this.renderInputOutputPair(tree, element, templateData); + } else if (templateId === ReplExpressionsRenderer.EXPRESSION_TEMPLATE_ID) { + this.renderExpression(tree, element, templateData); } else if (templateId === ReplExpressionsRenderer.VALUE_OUTPUT_TEMPLATE_ID) { this.renderOutputValue(element, templateData); - } else if (templateId === ReplExpressionsRenderer.KEY_VALUE_OUTPUT_TEMPLATE_ID) { - this.renderOutputKeyValue(tree, element, templateData); + } else if (templateId === ReplExpressionsRenderer.NAME_VALUE_OUTPUT_TEMPLATE_ID) { + this.renderOutputNameValue(tree, element, templateData); } } - private renderInputOutputPair(tree: ITree, expression: debug.IExpression, templateData: IInputOutputPairTemplateData): void { + private renderExpression(tree: ITree, expression: IExpression, templateData: IExpressionTemplateData): void { templateData.input.textContent = expression.name; renderExpressionValue(expression, templateData.value, { showChanged: false, @@ -401,13 +397,13 @@ export class ReplExpressionsRenderer implements IRenderer { }, event.ctrlKey || event.metaKey).done(null, errors.onUnexpectedError); } - private renderOutputKeyValue(tree: ITree, output: KeyValueOutputElement, templateData: IKeyValueOutputTemplateData): void { + private renderOutputNameValue(tree: ITree, output: NameValueOutputElement, templateData: IKeyValueOutputTemplateData): void { // key - if (output.key) { - templateData.key.textContent = `${output.key}:`; + if (output.name) { + templateData.name.textContent = `${output.name}:`; } else { - templateData.key.textContent = ''; + templateData.name.textContent = ''; } // value @@ -443,8 +439,8 @@ export class ReplExpressionsAccessibilityProvider implements IAccessibilityProvi if (element instanceof ValueOutputElement) { return nls.localize('replValueOutputAriaLabel', "{0}, read eval print loop, debug", (element).value); } - if (element instanceof KeyValueOutputElement) { - return nls.localize('replKeyValueOutputAriaLabel', "Output variable {0} has value {1}, read eval print loop, debug", (element).key, (element).value); + if (element instanceof NameValueOutputElement) { + return nls.localize('replKeyValueOutputAriaLabel', "Output variable {0} has value {1}, read eval print loop, debug", (element).name, (element).value); } return null; @@ -491,7 +487,7 @@ export class ReplExpressionsController extends BaseDebugController { private lastSelectedString: string = null; constructor( - debugService: debug.IDebugService, + debugService: IDebugService, contextMenuService: IContextMenuService, actionProvider: IActionProvider, private replInput: ICodeEditor, @@ -503,7 +499,7 @@ export class ReplExpressionsController extends BaseDebugController { protected onLeftClick(tree: ITree, element: any, eventish: ICancelableEvent, origin: string = 'mouse'): boolean { const mouseEvent = 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 KeyValueOutputElement && element.getChildren().length > 0)) && mouseEvent.target.className.indexOf('input expression') === -1) { + if ((element.reference > 0 || (element instanceof NameValueOutputElement && element.getChildren().length > 0)) && mouseEvent.target.className.indexOf('input expression') === -1) { super.onLeftClick(tree, element, eventish, origin); tree.clearFocus(); tree.deselect(element); diff --git a/src/vs/workbench/parts/debug/test/node/debugModel.test.ts b/src/vs/workbench/parts/debug/test/node/debugModel.test.ts index 1a2db11ee55..4852f1498cd 100644 --- a/src/vs/workbench/parts/debug/test/node/debugModel.test.ts +++ b/src/vs/workbench/parts/debug/test/node/debugModel.test.ts @@ -354,13 +354,13 @@ suite('Debug - Model', () => { assert.equal(elements[3].severity, severity.Warning); const keyValueObject = { 'key1': 2, 'key2': 'value' }; - model.appendReplOutput(keyValueObject); - const element = model.getReplElements()[4]; + model.appendReplOutput(keyValueObject, severity.Info); + const element = model.getReplElements()[4]; assert.equal(element.value, 'Object'); assert.deepEqual(element.valueObj, keyValueObject); const multiLineContent = 'multi line \n string \n last line'; - model.appendReplOutput(multiLineContent); + model.appendReplOutput(multiLineContent, severity.Info); const multiLineElement = model.getReplElements()[5]; assert.equal(multiLineElement.value, multiLineContent); assert.equal(model.getReplElements().length, 6); -- GitLab