Merge pull request #80422 from dgozman/fix-79196

Separate REPL evaluation from it's result; fixes #79196
This commit is contained in:
Isidor Nikolic 2019-09-10 11:56:51 +02:00 committed by GitHub
commit 4e3b4b6356
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
9 changed files with 207 additions and 148 deletions

View file

@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/
import * as dom from 'vs/base/browser/dom';
import { IExpression, IDebugService } from 'vs/workbench/contrib/debug/common/debug';
import { IExpression, IDebugService, IExpressionContainer } from 'vs/workbench/contrib/debug/common/debug';
import { Expression, Variable, ExpressionContainer } from 'vs/workbench/contrib/debug/common/debugModel';
import { IContextViewService } from 'vs/platform/contextview/browser/contextView';
import { IInputValidationOptions, InputBox } from 'vs/base/browser/ui/inputbox/inputBox';
@ -50,7 +50,7 @@ export function replaceWhitespace(value: string): string {
return value.replace(/[\n\r\t]/g, char => map[char]);
}
export function renderExpressionValue(expressionOrValue: IExpression | string, container: HTMLElement, options: IRenderValueOptions): void {
export function renderExpressionValue(expressionOrValue: IExpressionContainer | string, container: HTMLElement, options: IRenderValueOptions): void {
let value = typeof expressionOrValue === 'string' ? expressionOrValue : expressionOrValue.value;
// remove stale classes

View file

@ -921,7 +921,9 @@ export class DebugSession implements IDebugSession {
}
async addReplExpression(stackFrame: IStackFrame | undefined, name: string): Promise<void> {
await this.repl.addReplExpression(stackFrame, name);
const expressionEvaluated = this.repl.addReplExpression(stackFrame, name);
this._onDidChangeREPLElements.fire();
await expressionEvaluated;
this._onDidChangeREPLElements.fire();
// Evaluate all watch expressions and fetch variables again since repl evaluation might have changed some.
variableSetEmitter.fire();

View file

@ -48,11 +48,13 @@
cursor: text;
}
.repl .repl-tree .output.expression > .value {
.repl .repl-tree .output.expression > .value,
.repl .repl-tree .evaluation-result.expression > .value {
margin-left: 0px;
}
.repl .repl-tree .output.expression > .annotation {
.repl .repl-tree .output.expression > .annotation,
.repl .repl-tree .evaluation-result.expression > .annotation {
font-size: inherit;
padding-left: 6px;
}
@ -67,7 +69,7 @@
}
/* Only show 'stale expansion' info when the element gets expanded. */
.repl .repl-tree .input-output-pair > .output > .annotation::before {
.repl .repl-tree .evaluation-result > .annotation::before {
content: '';
}
@ -134,7 +136,8 @@
.monaco-workbench .repl .repl-tree .output.expression .code-underline { text-decoration: underline; }
/* Links */
.monaco-workbench .repl .repl-tree .output.expression a {
.monaco-workbench .repl .repl-tree .output.expression a,
.monaco-workbench .repl .repl-tree .evaluation-result.expression a {
text-decoration: underline;
cursor: pointer;
}

View file

@ -44,7 +44,8 @@ import { CompletionContext, CompletionList, CompletionProviderRegistry } from 'v
import { first } from 'vs/base/common/arrays';
import { IPanelService } from 'vs/workbench/services/panel/common/panelService';
import { IAccessibilityProvider } from 'vs/base/browser/ui/list/listWidget';
import { Variable, Expression, SimpleReplElement, RawObjectReplElement } from 'vs/workbench/contrib/debug/common/debugModel';
import { Variable } from 'vs/workbench/contrib/debug/common/debugModel';
import { SimpleReplElement, RawObjectReplElement, ReplEvaluationInput, ReplEvaluationResult } from 'vs/workbench/contrib/debug/common/replModel';
import { IListVirtualDelegate } from 'vs/base/browser/ui/list/list';
import { ITreeRenderer, ITreeNode, ITreeContextMenuEvent, IAsyncDataSource } from 'vs/base/browser/ui/tree/tree';
import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
@ -412,7 +413,8 @@ export class Repl extends Panel implements IPrivateReplService, IHistoryNavigati
[
this.instantiationService.createInstance(VariablesRenderer),
this.instantiationService.createInstance(ReplSimpleElementsRenderer),
new ReplExpressionsRenderer(),
new ReplEvaluationInputsRenderer(),
new ReplEvaluationResultsRenderer(),
new ReplRawObjectsRenderer()
],
// https://github.com/microsoft/TypeScript/issues/32526
@ -568,12 +570,13 @@ export class Repl extends Panel implements IPrivateReplService, IHistoryNavigati
// Repl tree
interface IExpressionTemplateData {
input: HTMLElement;
output: HTMLElement;
interface IReplEvaluationInputTemplateData {
label: HighlightedLabel;
}
interface IReplEvaluationResultTemplateData {
value: HTMLElement;
annotation: HTMLElement;
label: HighlightedLabel;
}
interface ISimpleReplElementTemplateData {
@ -593,27 +596,46 @@ interface IRawObjectReplTemplateData {
label: HighlightedLabel;
}
class ReplExpressionsRenderer implements ITreeRenderer<Expression, FuzzyScore, IExpressionTemplateData> {
static readonly ID = 'expressionRepl';
class ReplEvaluationInputsRenderer implements ITreeRenderer<ReplEvaluationInput, FuzzyScore, IReplEvaluationInputTemplateData> {
static readonly ID = 'replEvaluationInput';
get templateId(): string {
return ReplExpressionsRenderer.ID;
return ReplEvaluationInputsRenderer.ID;
}
renderTemplate(container: HTMLElement): IExpressionTemplateData {
dom.addClass(container, 'input-output-pair');
const input = dom.append(container, $('.input.expression'));
renderTemplate(container: HTMLElement): IReplEvaluationInputTemplateData {
const input = dom.append(container, $('.expression'));
const label = new HighlightedLabel(input, false);
const output = dom.append(container, $('.output.expression'));
return { label };
}
renderElement(element: ITreeNode<ReplEvaluationInput, FuzzyScore>, index: number, templateData: IReplEvaluationInputTemplateData): void {
const evaluation = element.element;
templateData.label.set(evaluation.value, createMatches(element.filterData));
}
disposeTemplate(templateData: IReplEvaluationInputTemplateData): void {
// noop
}
}
class ReplEvaluationResultsRenderer implements ITreeRenderer<ReplEvaluationResult, FuzzyScore, IReplEvaluationResultTemplateData> {
static readonly ID = 'replEvaluationResult';
get templateId(): string {
return ReplEvaluationResultsRenderer.ID;
}
renderTemplate(container: HTMLElement): IReplEvaluationResultTemplateData {
const output = dom.append(container, $('.evaluation-result.expression'));
const value = dom.append(output, $('span.value'));
const annotation = dom.append(output, $('span'));
return { input, label, output, value, annotation };
return { value, annotation };
}
renderElement(element: ITreeNode<Expression, FuzzyScore>, index: number, templateData: IExpressionTemplateData): void {
renderElement(element: ITreeNode<ReplEvaluationResult, FuzzyScore>, index: number, templateData: IReplEvaluationResultTemplateData): void {
const expression = element.element;
templateData.label.set(expression.name, createMatches(element.filterData));
renderExpressionValue(expression, templateData.value, {
preserveWhitespace: !expression.hasChildren,
showHover: false,
@ -625,7 +647,7 @@ class ReplExpressionsRenderer implements ITreeRenderer<Expression, FuzzyScore, I
}
}
disposeTemplate(templateData: IExpressionTemplateData): void {
disposeTemplate(templateData: IReplEvaluationResultTemplateData): void {
// noop
}
}
@ -757,24 +779,23 @@ class ReplDelegate implements IListVirtualDelegate<IReplElement> {
const rowHeight = Math.ceil(1.4 * fontSize);
const wordWrap = config.console.wordWrap;
if (!wordWrap) {
return element instanceof Expression ? 2 * rowHeight : rowHeight;
return rowHeight;
}
// In order to keep scroll position we need to give a good approximation to the tree
// For every 150 characters increase the number of lines needed
if (element instanceof Expression) {
let { name, value } = element;
let nameRows = countNumberOfLines(name) + Math.floor(name.length / 150);
if (element instanceof ReplEvaluationResult) {
let value = element.value;
if (element.hasChildren) {
return (nameRows + 1) * rowHeight;
return rowHeight;
}
let valueRows = value ? (countNumberOfLines(value) + Math.floor(value.length / 150)) : 0;
return rowHeight * (nameRows + valueRows);
return rowHeight * valueRows;
}
if (element instanceof SimpleReplElement) {
if (element instanceof SimpleReplElement || element instanceof ReplEvaluationInput) {
let value = element.value;
let valueRows = countNumberOfLines(value) + Math.floor(value.length / 150);
@ -788,8 +809,11 @@ class ReplDelegate implements IListVirtualDelegate<IReplElement> {
if (element instanceof Variable && element.name) {
return VariablesRenderer.ID;
}
if (element instanceof Expression) {
return ReplExpressionsRenderer.ID;
if (element instanceof ReplEvaluationResult) {
return ReplEvaluationResultsRenderer.ID;
}
if (element instanceof ReplEvaluationInput) {
return ReplEvaluationInputsRenderer.ID;
}
if (element instanceof SimpleReplElement || (element instanceof Variable && !element.name)) {
// Variable with no name is a top level variable which should be rendered like a repl element #17404
@ -836,10 +860,7 @@ class ReplAccessibilityProvider implements IAccessibilityProvider<IReplElement>
if (element instanceof Variable) {
return nls.localize('replVariableAriaLabel', "Variable {0} has value {1}, read eval print loop, debug", element.name, element.value);
}
if (element instanceof Expression) {
return nls.localize('replExpressionAriaLabel', "Expression {0} has value {1}, read eval print loop, debug", element.name, element.value);
}
if (element instanceof SimpleReplElement) {
if (element instanceof SimpleReplElement || element instanceof ReplEvaluationInput || element instanceof ReplEvaluationResult) {
return nls.localize('replValueOutputAriaLabel', "{0}, read eval print loop, debug", element.value);
}
if (element instanceof RawObjectReplElement) {

View file

@ -104,13 +104,13 @@ export interface IExpressionContainer extends ITreeElement {
readonly hasChildren: boolean;
getChildren(): Promise<IExpression[]>;
readonly reference?: number;
readonly value: string;
readonly type?: string;
valueChanged?: boolean;
}
export interface IExpression extends IReplElement, IExpressionContainer {
export interface IExpression extends IExpressionContainer {
name: string;
readonly value: string;
valueChanged?: boolean;
readonly type?: string;
}
export interface IDebugger {

View file

@ -10,13 +10,12 @@ import * as lifecycle from 'vs/base/common/lifecycle';
import { Event, Emitter } from 'vs/base/common/event';
import { generateUuid } from 'vs/base/common/uuid';
import { RunOnceScheduler } from 'vs/base/common/async';
import severity from 'vs/base/common/severity';
import { isObject, isString, isUndefinedOrNull } from 'vs/base/common/types';
import { isString, isUndefinedOrNull } from 'vs/base/common/types';
import { distinct, lastIndex } from 'vs/base/common/arrays';
import { Range, IRange } from 'vs/editor/common/core/range';
import {
ITreeElement, IExpression, IExpressionContainer, IDebugSession, IStackFrame, IExceptionBreakpoint, IBreakpoint, IFunctionBreakpoint, IDebugModel, IReplElementSource,
IThread, IRawModelUpdate, IScope, IRawStoppedDetails, IEnablement, IBreakpointData, IExceptionInfo, IReplElement, IBreakpointsChangeEvent, IBreakpointUpdateData, IBaseBreakpoint, State, IDataBreakpoint
ITreeElement, IExpression, IExpressionContainer, IDebugSession, IStackFrame, IExceptionBreakpoint, IBreakpoint, IFunctionBreakpoint, IDebugModel,
IThread, IRawModelUpdate, IScope, IRawStoppedDetails, IEnablement, IBreakpointData, IExceptionInfo, IBreakpointsChangeEvent, IBreakpointUpdateData, IBaseBreakpoint, State, IDataBreakpoint
} from 'vs/workbench/contrib/debug/common/debug';
import { Source, UNKNOWN_SOURCE_LABEL } from 'vs/workbench/contrib/debug/common/debugSource';
import { commonSuffixLength } from 'vs/base/common/strings';
@ -25,75 +24,13 @@ import { IEditorService } from 'vs/workbench/services/editor/common/editorServic
import { ITextFileService } from 'vs/workbench/services/textfile/common/textfiles';
import { ITextEditor } from 'vs/workbench/common/editor';
export class SimpleReplElement implements IReplElement {
constructor(
private id: string,
public value: string,
public severity: severity,
public sourceData?: IReplElementSource,
) { }
toString(): string {
return this.value;
}
getId(): string {
return this.id;
}
}
export class RawObjectReplElement implements IExpression {
private static readonly MAX_CHILDREN = 1000; // upper bound of children per value
constructor(private id: string, public name: string, public valueObj: any, public sourceData?: IReplElementSource, public annotation?: string) { }
getId(): string {
return this.id;
}
get value(): string {
if (this.valueObj === null) {
return 'null';
} else if (Array.isArray(this.valueObj)) {
return `Array[${this.valueObj.length}]`;
} else if (isObject(this.valueObj)) {
return 'Object';
} else if (isString(this.valueObj)) {
return `"${this.valueObj}"`;
}
return String(this.valueObj) || '';
}
get hasChildren(): boolean {
return (Array.isArray(this.valueObj) && this.valueObj.length > 0) || (isObject(this.valueObj) && Object.getOwnPropertyNames(this.valueObj).length > 0);
}
getChildren(): Promise<IExpression[]> {
let result: IExpression[] = [];
if (Array.isArray(this.valueObj)) {
result = (<any[]>this.valueObj).slice(0, RawObjectReplElement.MAX_CHILDREN)
.map((v, index) => new RawObjectReplElement(`${this.id}:${index}`, String(index), v));
} else if (isObject(this.valueObj)) {
result = Object.getOwnPropertyNames(this.valueObj).slice(0, RawObjectReplElement.MAX_CHILDREN)
.map((key, index) => new RawObjectReplElement(`${this.id}:${index}`, key, this.valueObj[key]));
}
return Promise.resolve(result);
}
toString(): string {
return `${this.name}\n${this.value}`;
}
}
export class ExpressionContainer implements IExpressionContainer {
public static allValues = new Map<string, string>();
// Use chunks to support variable paging #9537
private static readonly BASE_CHUNK_SIZE = 100;
public type: string | undefined;
public valueChanged = false;
private _value: string = '';
protected children?: Promise<IExpression[]>;
@ -195,13 +132,43 @@ export class ExpressionContainer implements IExpressionContainer {
toString(): string {
return this.value;
}
async evaluateExpression(
expression: string,
session: IDebugSession | undefined,
stackFrame: IStackFrame | undefined,
context: string): Promise<boolean> {
if (!session || (!stackFrame && context !== 'repl')) {
this.value = context === 'repl' ? nls.localize('startDebugFirst', "Please start a debug session to evaluate expressions") : Expression.DEFAULT_VALUE;
this.reference = 0;
return false;
}
this.session = session;
try {
const response = await session.evaluate(expression, stackFrame ? stackFrame.frameId : undefined, context);
if (response && response.body) {
this.value = response.body.result || '';
this.reference = response.body.variablesReference;
this.namedVariables = response.body.namedVariables;
this.indexedVariables = response.body.indexedVariables;
this.type = response.body.type || this.type;
return true;
}
return false;
} catch (e) {
this.value = e.message || '';
this.reference = 0;
return false;
}
}
}
export class Expression extends ExpressionContainer implements IExpression {
static DEFAULT_VALUE = nls.localize('notAvailable', "not available");
public available: boolean;
public type: string | undefined;
constructor(public name: string, id = generateUuid()) {
super(undefined, 0, id);
@ -214,30 +181,7 @@ export class Expression extends ExpressionContainer implements IExpression {
}
async evaluate(session: IDebugSession | undefined, stackFrame: IStackFrame | undefined, context: string): Promise<void> {
if (!session || (!stackFrame && context !== 'repl')) {
this.value = context === 'repl' ? nls.localize('startDebugFirst', "Please start a debug session to evaluate expressions") : Expression.DEFAULT_VALUE;
this.available = false;
this.reference = 0;
return Promise.resolve(undefined);
}
this.session = session;
try {
const response = await session.evaluate(this.name, stackFrame ? stackFrame.frameId : undefined, context);
this.available = !!(response && response.body);
if (response && response.body) {
this.value = response.body.result || '';
this.reference = response.body.variablesReference;
this.namedVariables = response.body.namedVariables;
this.indexedVariables = response.body.indexedVariables;
this.type = response.body.type || this.type;
}
} catch (e) {
this.value = e.message || '';
this.available = false;
this.reference = 0;
}
this.available = await this.evaluateExpression(this.name, session, stackFrame, context);
}
toString(): string {

View file

@ -6,15 +6,105 @@
import * as nls from 'vs/nls';
import severity from 'vs/base/common/severity';
import { IReplElement, IStackFrame, IExpression, IReplElementSource, IDebugSession } from 'vs/workbench/contrib/debug/common/debug';
import { Expression, SimpleReplElement, RawObjectReplElement } from 'vs/workbench/contrib/debug/common/debugModel';
import { isUndefinedOrNull, isObject } from 'vs/base/common/types';
import { ExpressionContainer } from 'vs/workbench/contrib/debug/common/debugModel';
import { isString, isUndefinedOrNull, isObject } from 'vs/base/common/types';
import { basenameOrAuthority } from 'vs/base/common/resources';
import { URI } from 'vs/base/common/uri';
import { endsWith } from 'vs/base/common/strings';
import { generateUuid } from 'vs/base/common/uuid';
const MAX_REPL_LENGTH = 10000;
let topReplElementCounter = 0;
export class SimpleReplElement implements IReplElement {
constructor(
private id: string,
public value: string,
public severity: severity,
public sourceData?: IReplElementSource,
) { }
toString(): string {
return this.value;
}
getId(): string {
return this.id;
}
}
export class RawObjectReplElement implements IExpression {
private static readonly MAX_CHILDREN = 1000; // upper bound of children per value
constructor(private id: string, public name: string, public valueObj: any, public sourceData?: IReplElementSource, public annotation?: string) { }
getId(): string {
return this.id;
}
get value(): string {
if (this.valueObj === null) {
return 'null';
} else if (Array.isArray(this.valueObj)) {
return `Array[${this.valueObj.length}]`;
} else if (isObject(this.valueObj)) {
return 'Object';
} else if (isString(this.valueObj)) {
return `"${this.valueObj}"`;
}
return String(this.valueObj) || '';
}
get hasChildren(): boolean {
return (Array.isArray(this.valueObj) && this.valueObj.length > 0) || (isObject(this.valueObj) && Object.getOwnPropertyNames(this.valueObj).length > 0);
}
getChildren(): Promise<IExpression[]> {
let result: IExpression[] = [];
if (Array.isArray(this.valueObj)) {
result = (<any[]>this.valueObj).slice(0, RawObjectReplElement.MAX_CHILDREN)
.map((v, index) => new RawObjectReplElement(`${this.id}:${index}`, String(index), v));
} else if (isObject(this.valueObj)) {
result = Object.getOwnPropertyNames(this.valueObj).slice(0, RawObjectReplElement.MAX_CHILDREN)
.map((key, index) => new RawObjectReplElement(`${this.id}:${index}`, key, this.valueObj[key]));
}
return Promise.resolve(result);
}
toString(): string {
return `${this.name}\n${this.value}`;
}
}
export class ReplEvaluationInput implements IReplElement {
private id: string;
constructor(public value: string) {
this.id = generateUuid();
}
toString(): string {
return this.value;
}
getId(): string {
return this.id;
}
}
export class ReplEvaluationResult extends ExpressionContainer implements IReplElement {
constructor() {
super(undefined, 0, generateUuid());
}
toString(): string {
return `${this.value}`;
}
}
export class ReplModel {
private replElements: IReplElement[] = [];
@ -24,10 +114,11 @@ export class ReplModel {
return this.replElements;
}
addReplExpression(stackFrame: IStackFrame | undefined, name: string): Promise<void> {
const expression = new Expression(name);
this.addReplElement(expression);
return expression.evaluate(this.session, stackFrame, 'repl');
async addReplExpression(stackFrame: IStackFrame | undefined, name: string): Promise<void> {
this.addReplElement(new ReplEvaluationInput(name));
const result = new ReplEvaluationResult();
await result.evaluateExpression(name, this.session, stackFrame, 'repl');
this.addReplElement(result);
}
appendToRepl(data: string | IExpression, sev: severity, source?: IReplElementSource): void {

View file

@ -6,12 +6,12 @@
import * as assert from 'assert';
import { URI as uri } from 'vs/base/common/uri';
import severity from 'vs/base/common/severity';
import { SimpleReplElement, DebugModel, Expression, RawObjectReplElement, StackFrame, Thread } from 'vs/workbench/contrib/debug/common/debugModel';
import { DebugModel, Expression, StackFrame, Thread } from 'vs/workbench/contrib/debug/common/debugModel';
import * as sinon from 'sinon';
import { MockRawSession } from 'vs/workbench/contrib/debug/test/common/mockDebug';
import { Source } from 'vs/workbench/contrib/debug/common/debugSource';
import { DebugSession } from 'vs/workbench/contrib/debug/browser/debugSession';
import { ReplModel } from 'vs/workbench/contrib/debug/common/replModel';
import { SimpleReplElement, RawObjectReplElement, ReplEvaluationInput, ReplModel } from 'vs/workbench/contrib/debug/common/replModel';
import { IBreakpointUpdateData } from 'vs/workbench/contrib/debug/common/debug';
import { NullOpenerService } from 'vs/platform/opener/common/opener';
@ -348,9 +348,7 @@ suite('Debug - Model', () => {
assert.equal(replModel.getReplElements().length, 3);
replModel.getReplElements().forEach(re => {
assert.equal((<Expression>re).available, false);
assert.equal((<Expression>re).name, 'myVariable');
assert.equal((<Expression>re).reference, 0);
assert.equal((<ReplEvaluationInput>re).value, 'myVariable');
});
replModel.removeReplExpressions();

View file

@ -28,7 +28,7 @@ const STACK_FRAME = `${VIEWLET} .monaco-list-row .stack-frame`;
const SPECIFIC_STACK_FRAME = filename => `${STACK_FRAME} .file[title*="${filename}"]`;
const VARIABLE = `${VIEWLET} .debug-variables .monaco-list-row .expression`;
const CONSOLE_OUTPUT = `.repl .output.expression .value`;
const CONSOLE_INPUT_OUTPUT = `.repl .input-output-pair .output.expression .value`;
const CONSOLE_EVALUATION_RESULT = `.repl .evaluation-result.expression .value`;
const REPL_FOCUSED = '.repl-input-wrapper .monaco-editor textarea';
@ -132,8 +132,8 @@ export class Debug extends Viewlet {
// Wait for the keys to be picked up by the editor model such that repl evalutes what just got typed
await this.editor.waitForEditorContents('debug:replinput', s => s.indexOf(text) >= 0);
await this.code.dispatchKeybinding('enter');
await this.code.waitForElement(CONSOLE_INPUT_OUTPUT);
await this.waitForOutput(output => accept(output[output.length - 1] || ''));
await this.code.waitForElements(CONSOLE_EVALUATION_RESULT, false,
elements => !!elements.length && accept(elements[elements.length - 1].textContent));
}
// Different node versions give different number of variables. As a workaround be more relaxed when checking for variable count